From bfeb1410c12a2b722aa6529dec57ea0348787913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9la=20Lakatos?= Date: Thu, 9 Jul 2020 11:35:58 +0200 Subject: [PATCH 1/3] change: modified boolean option parsing, don't parse the next value for boolean options. Prevent the annoying prevous behavior when parsing a boolean option followed by variadic arguments parsed the first followed as value for option instead of true and parst the variadic arguments from the second element only. Like `--verbose first second` was parsed where the `verbose` value was `first` instead `true` and the followed array is only `[second]` instead `[first, second]`. In order to achieve this the `shouldTakeNextAsValue` function extracted from `handleConcatenatedOpts` and use it in `handleOptWithoutValue` as well. Resolves #187 --- src/command/__tests__/command.spec.ts | 70 +++++++++++++++++++++++++++ src/parser/__tests__/parser.spec.ts | 59 +++++++++++++++++----- src/parser/index.ts | 17 ++++--- 3 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/command/__tests__/command.spec.ts b/src/command/__tests__/command.spec.ts index 0404da3..0c906ab 100644 --- a/src/command/__tests__/command.spec.ts +++ b/src/command/__tests__/command.spec.ts @@ -253,6 +253,76 @@ describe("Command", () => { ) }) + describe("with boolean option before arguments", () => { + it("command should be able to handle variadic arguments with boolean option alias", async () => { + const action = jest.fn().mockReturnValue("OK!") + prog + .command("order", "Order something") + .argument("", "Pizza types") + .option("-v, --verbose", "Detailed log") + .action(action) + const args = ["order", "-v", "pepperoni", "regina"] + await prog.run(args) + expect(action).toHaveBeenCalledWith( + expect.objectContaining({ + args: { types: ["pepperoni", "regina"] }, + options: { v: true, verbose: true }, + }), + ) + }) + + it("command should be able to handle variadic arguments with boolean option", async () => { + const action = jest.fn().mockReturnValue("OK!") + prog + .command("order", "Order something") + .argument("", "Pizza types") + .option("-v, --verbose", "Detailed log") + .action(action) + const args = ["order", "--verbose", "pepperoni", "regina"] + await prog.run(args) + expect(action).toHaveBeenCalledWith( + expect.objectContaining({ + args: { types: ["pepperoni", "regina"] }, + options: { v: true, verbose: true }, + }), + ) + }) + + it("command should be able to handle variadic arguments with boolean option and explicit value", async () => { + const action = jest.fn().mockReturnValue("OK!") + prog + .command("order", "Order something") + .argument("", "Pizza types") + .option("-v, --verbose", "Detailed log") + .action(action) + const args = ["order", "--verbose=true", "pepperoni", "regina"] + await prog.run(args) + expect(action).toHaveBeenCalledWith( + expect.objectContaining({ + args: { types: ["pepperoni", "regina"] }, + options: { v: true, verbose: true }, + }), + ) + }) + + it("command should be able to handle variadic arguments with negative boolean option", async () => { + const action = jest.fn().mockReturnValue("OK!") + prog + .command("order", "Order something") + .argument("", "Pizza types") + .option("-v, --verbose", "Detailed log") + .action(action) + const args = ["order", "--no-verbose", "pepperoni", "regina"] + await prog.run(args) + expect(action).toHaveBeenCalledWith( + expect.objectContaining({ + args: { types: ["pepperoni", "regina"] }, + options: { v: false, verbose: false }, + }), + ) + }) + }) + it("command should check arguments range (variable)", async () => { const action = jest.fn().mockReturnValue("hey!") const cmd = prog diff --git a/src/parser/__tests__/parser.spec.ts b/src/parser/__tests__/parser.spec.ts index 5e4c755..7a3d131 100644 --- a/src/parser/__tests__/parser.spec.ts +++ b/src/parser/__tests__/parser.spec.ts @@ -88,20 +88,53 @@ describe("Parser", () => { expect(result.args).toEqual(["my-arg1", "my-arg2"]) }) - it("should handle 'boolean' option", () => { - const line = - "--my-opt true --my-bool 1 --my-false=0 --another=yes --not-now=no -y=yes --not-forced=yes" - const result = parseLine(line, { - boolean: ["myOpt", "myBool", "myFalse", "another", "notNow", "y"], + describe("boolean options", () => { + it("should handle 'boolean' option", () => { + const line = + "--my-opt --my-bool --my-false=0 --another=yes --not-now=no -y=yes --not-forced=yes --no-negative" + const result = parseLine(line, { + boolean: ["myOpt", "myBool", "myFalse", "another", "notNow", "y", "negative"], + }) + expect(result.options).toEqual({ + myOpt: true, + myBool: true, + myFalse: false, + another: true, + notNow: false, + y: true, + notForced: "yes", + negative: false, + }) }) - expect(result.options).toEqual({ - myOpt: true, - myBool: true, - myFalse: false, - another: true, - notNow: false, - y: true, - notForced: "yes", + + it("should handle flag followed by arguments", async () => { + const line = "-v arg1 arg2" + const result = parseLine(line, { + boolean: ["v"], + }) + expect(result.options).toEqual({ + v: true, + }) + }) + + it("should handle long option followed by arguments", async () => { + const line = "--verbose arg1 arg2" + const result = parseLine(line, { + boolean: ["verbose"], + }) + expect(result.options).toEqual({ + verbose: true, + }) + }) + + it("should handle negative option followed by arguments", async () => { + const line = "--no-verbose arg1 arg2" + const result = parseLine(line, { + boolean: ["verbose"], + }) + expect(result.options).toEqual({ + verbose: false, + }) }) }) diff --git a/src/parser/index.ts b/src/parser/index.ts index e1ae9cf..1249e62 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -250,12 +250,13 @@ class OptionParser { handleOptWithoutValue(name: string, tree: Tree): void { const next = tree.next() - const nextIsOptOrUndef = isOptionStr(next) || isDdash(next) || next === undefined + const cleanName = formatOptName(name) + const shouldTakeNextAsVal = this.shouldTakeNextAsValue(cleanName, next) this.compute( name, - cast(name, nextIsOptOrUndef ? true : (next as string), this.config), + cast(name, shouldTakeNextAsVal ? (next as string) : true, this.config), ) - if (!nextIsOptOrUndef) { + if (shouldTakeNextAsVal) { tree.forward() } } @@ -265,10 +266,7 @@ class OptionParser { val = true const next = tree.next() const last = names[names.length - 1] - const alias = this.config.alias[last] - const shouldTakeNextAsVal = - next && !isOptionStr(next) && !isDdash(next) && !this.isBoolean(last, alias) - if (shouldTakeNextAsVal) { + if (this.shouldTakeNextAsValue(last, next)) { tree.forward() val = next as string } @@ -276,6 +274,11 @@ class OptionParser { this.computeMulti(names, val) } + shouldTakeNextAsValue(cleaned: string, next: string | undefined): boolean { + const nextIsOptOrUndef = isOptionStr(next) || isDdash(next) || next === undefined + return !nextIsOptOrUndef && !this.isBoolean(cleaned, this.config.alias[cleaned]) + } + visit(tree: Tree): boolean { // only handle options /* istanbul ignore if */ From e2143325925bffec35209b9a215b5b6f6c7c2ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9la=20Lakatos?= Date: Thu, 9 Jul 2020 13:07:20 +0200 Subject: [PATCH 2/3] fix: keep original values in `rawOptions` for negative flags instead the inserted one --- src/parser/__tests__/parser.spec.ts | 13 +++++++++++++ src/parser/index.ts | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/parser/__tests__/parser.spec.ts b/src/parser/__tests__/parser.spec.ts index 7a3d131..3dce744 100644 --- a/src/parser/__tests__/parser.spec.ts +++ b/src/parser/__tests__/parser.spec.ts @@ -136,6 +136,19 @@ describe("Parser", () => { verbose: false, }) }) + + it("should handle negative option correcly in rawOptions", async () => { + const line = "--no-verbose" + const result = parseLine(line, { + boolean: ["verbose"], + }) + expect(result.options).toEqual({ + verbose: false, + }) + expect(result.rawOptions).toEqual({ + "--no-verbose": true, + }) + }) }) it("should handle 'string' option", () => { diff --git a/src/parser/index.ts b/src/parser/index.ts index 1249e62..3715106 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -316,7 +316,8 @@ class OptionParser { : [prop] ).concat(val) } else { - this.rawOptions[name] = this.options[cleanName] = no ? !val : val + this.options[cleanName] = no ? !val : val + this.rawOptions[name] = val } if (alias) { this.options[alias] = this.options[cleanName] From 8da978461d657a9aba40c241444dfece184eab10 Mon Sep 17 00:00:00 2001 From: Bela Lakatos Date: Fri, 4 Dec 2020 11:34:50 +0100 Subject: [PATCH 3/3] trigger GitHub actions