feat(flags,command): improve support for negatable options (#103)

command/README.md

@@ -279,39 +279,35 @@ Missing required option: --cheese
 
 ### Negatable options
 
-You can call the long name from an option with a boolean or an optional value (declared using square brackets) with a leading `--no-` to set the option value to false when used.
+You can specify a boolean option long name with a leading `no-` to set the option value to false when used.
+Defined alone this also makes the option true by default.
 
-You can specify a default value for the flag and it can be overridden on command line.
+If you define `--foo`, adding `--no-foo` does not change the default value from what it would otherwise be.
+
+You can specify a default value for a flag and it can be overridden on command line.
 
 ```typescript
 import { Command } from "https://deno.land/x/cliffy/command/mod.ts";
 
 const { options } = await new Command()
-  .option("--sauce [sauce:boolean]", "Remove sauce", {
-    default: true,
-  })
-  .option("--cheese [flavour:string]", "cheese flavour", {
-    default: "mozzarella",
-  })
+  // default value will be automatically set to true if no --check option exists
+  .option("--no-check", "No check.")
+  .option("--color <color:string>", "Color name.", { default: "yellow" })
+  .option("--no-color", "No color.")
+  // no default value
+  .option("--remote <url:string>", "Remote url.")
+  .option("--no-remote", "No remote.")
   .parse(Deno.args);
 
-const sauceStr = options.sauce ? "sauce" : "no sauce";
-const cheeseStr = options.cheese === false
-  ? "no cheese"
-  : `${options.cheese} cheese`;
-
-console.log(`You ordered a pizza with ${sauceStr} and ${cheeseStr}`);
+console.log(options);
 ```
 
 ```
 $ deno run https://deno.land/x/cliffy/examples/command/negatable_options.ts
-You ordered a pizza with sauce and mozzarella cheese
-
-$ deno run https://deno.land/x/cliffy/examples/command/negatable_options.ts --no-sauce --no-cheese
-You ordered a pizza with no sauce and no cheese
+{ check: true, color: "yellow" }
 
-$ deno run https://deno.land/x/cliffy/examples/command/negatable_options.ts --sauce --cheese parmesan
-You ordered a pizza with sauce and parmesan cheese
+$ deno run https://deno.land/x/cliffy/examples/command/negatable_options.ts --no-check --no-color --no-remote
+{ check: false, color: false, remote: false }
 ```
 
 ### Global options

command/test/option/duplicate_test.ts

@@ -4,6 +4,7 @@ import { Command } from "../../command.ts";
 const cmd = new Command()
   .throwErrors()
   .option("-f, --flag [value:boolean]", "description ...")
+  .option("--no-flag", "description ...")
   .action(() => {});
 
 Deno.test("command optionDuplicate flag", async () => {

command/test/option/negatable_test.ts

@@ -0,0 +1,74 @@
+import { assertEquals, assertThrowsAsync } from "../../../dev_deps.ts";
+import { Command } from "../../command.ts";
+
+function command(): Command {
+  return new Command()
+    .throwErrors()
+    .allowEmpty()
+    .option("--no-check", "No check.")
+    .option("--color <color:string>", "Color name.", { default: "yellow" })
+    .option("--no-color", "No color.")
+    .option("--remote <url:string>", "Remote url.", { depends: ["color"] })
+    .option("--no-remote", "No remote.");
+}
+
+Deno.test("negatable options with no arguments", async () => {
+  const { options, args, literal } = await command().parse([]);
+
+  assertEquals(options, {
+    check: true,
+    color: "yellow",
+  });
+  assertEquals(args, []);
+  assertEquals(literal, []);
+});
+
+Deno.test("negatable options with arguments", async () => {
+  const { options, args, literal } = await command().parse(
+    ["--color", "blue", "--remote", "foo"],
+  );
+
+  assertEquals(options, {
+    check: true,
+    color: "blue",
+    remote: "foo",
+  });
+  assertEquals(args, []);
+  assertEquals(literal, []);
+});
+
+Deno.test("negatable flag --no-remote should not depend on --color", async () => {
+  const { options, args, literal } = await command().parse(["--no-remote"]);
+
+  assertEquals(options, {
+    check: true,
+    color: "yellow",
+    remote: false,
+  });
+  assertEquals(args, []);
+  assertEquals(literal, []);
+});
+
+Deno.test("negatable flags should negate value", async () => {
+  const { options, args, literal } = await command().parse(
+    ["--no-check", "--no-color", "--no-remote"],
+  );
+
+  assertEquals(options, {
+    color: false,
+    check: false,
+    remote: false,
+  });
+  assertEquals(args, []);
+  assertEquals(literal, []);
+});
+
+Deno.test("negatable options should not be combinable with positive options", async () => {
+  await assertThrowsAsync(
+    async () => {
+      await command().parse(["--color", "--no-color", "--no-check"]);
+    },
+    Error,
+    "Duplicate option: --no-color",
+  );
+});

command/test/type/boolean_test.ts

@@ -5,6 +5,7 @@ const cmd = new Command()
   .throwErrors()
   .name("test-command")
   .option("-f, --flag [value:boolean]", "description ...")
+  .option("--no-flag", "description ...")
   .action(() => {});
 
 Deno.test("command typeString flag", async () => {

command/test/type/number_test.ts

@@ -3,7 +3,9 @@ import { Command } from "../../command.ts";
 
 const cmd = new Command()
   .throwErrors()
-  .option("-f, --flag [value:number]", "description ...").action(() => {});
+  .option("-f, --flag [value:number]", "description ...")
+  .option("--no-flag", "description ...")
+  .action(() => {});
 
 Deno.test("command typeString flag", async () => {
   const { options, args } = await cmd.parse(["-f"]);

command/test/type/string_test.ts

@@ -3,7 +3,9 @@ import { Command } from "../../command.ts";
 
 const cmd = new Command()
   .throwErrors()
-  .option("-f, --flag [value:string]", "description ...").action(() => {});
+  .option("-f, --flag [value:string]", "description ...")
+  .option("--no-flag", "description ...")
+  .action(() => {});
 
 Deno.test("command typeString flag", async () => {
   const { options, args } = await cmd.parse(["-f"]);

examples/command/negatable_options.ts

@@ -3,17 +3,13 @@
 import { Command } from "../../command/command.ts";
 
 const { options } = await new Command()
-  .option("--sauce [sauce:boolean]", "Remove sauce", {
-    default: true,
-  })
-  .option("--cheese [flavour:string]", "cheese flavour", {
-    default: "mozzarella",
-  })
+  // default value will be automatically set to true if no --check option exists
+  .option("--no-check", "No check.")
+  .option("--color <color:string>", "Color name.", { default: "yellow" })
+  .option("--no-color", "No color.")
+  // no default value
+  .option("--remote <url:string>", "Remote url.")
+  .option("--no-remote", "No remote.")
   .parse(Deno.args);
 
-const sauceStr = options.sauce ? "sauce" : "no sauce";
-const cheeseStr = options.cheese === false
-  ? "no cheese"
-  : `${options.cheese} cheese`;
-
-console.log(`You ordered a pizza with ${sauceStr} and ${cheeseStr}`);
+console.log(options);

flags/flags.ts

@@ -38,6 +38,7 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
   let negate = false;
 
   const flags: Record<string, unknown> = {};
+  const optionNames: Record<string, string> = {};
   const literal: string[] = [];
   const unknown: string[] = [];
   let stopEarly = false;
@@ -79,19 +80,21 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
         throw new Error(`Invalid flag name: ${current}`);
       }
 
-      negate = current.indexOf("--no-") === 0;
+      negate = current.startsWith("--no-");
 
-      const name = current.replace(/^-+(no-)?/, "");
-
-      option = getOption(opts.flags, name);
+      option = getOption(opts.flags, current);
+      // if (option && negate) {
+      //   const positiveName: string = current.replace(/^-+(no-)?/, "");
+      //   option = getOption(opts.flags, positiveName) ?? option;
+      // }
 
       if (!option) {
         if (opts.flags.length) {
           throw new Error(`Unknown option: ${current}`);
         }
 
         option = {
-          name,
+          name: current.replace(/^-+/, ""),
           optionalValue: true,
           type: OptionType.STRING,
         };
@@ -101,9 +104,10 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
         throw new Error(`Missing name for option: ${current}`);
       }
 
-      const friendlyName: string = paramCaseToCamelCase(option.name);
+      const positiveName: string = option.name.replace(/^no-?/, "");
+      const propName: string = paramCaseToCamelCase(positiveName);
 
-      if (typeof flags[friendlyName] !== "undefined" && !option.collect) {
+      if (typeof flags[propName] !== "undefined" && !option.collect) {
         throw new Error(`Duplicate option: ${current}`);
       }
 
@@ -118,30 +122,32 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
 
       let argIndex = 0;
       let inOptionalArg = false;
-      const previous = flags[friendlyName];
+      const previous = flags[propName];
 
       parseNext(option, args);
 
-      if (typeof flags[friendlyName] === "undefined") {
+      if (typeof flags[propName] === "undefined") {
         if (typeof option.default !== "undefined") {
-          flags[friendlyName] = typeof option.default === "function"
+          flags[propName] = typeof option.default === "function"
             ? option.default()
             : option.default;
         } else if (args[argIndex].requiredValue) {
           throw new Error(`Missing value for option: --${option.name}`);
         } else {
-          flags[friendlyName] = true;
+          flags[propName] = true;
         }
       }
 
       if (typeof option.value !== "undefined") {
-        flags[friendlyName] = option.value(flags[friendlyName], previous);
+        flags[propName] = option.value(flags[propName], previous);
       } else if (option.collect) {
         const value: unknown[] = Array.isArray(previous) ? previous : [];
-        value.push(flags[friendlyName]);
-        flags[friendlyName] = value;
+        value.push(flags[propName]);
+        flags[propName] = value;
       }
 
+      optionNames[propName] = option.name;
+
       /** Parse next argument for current option. */
       // deno-lint-ignore no-inner-declarations
       function parseNext(option: IFlagOptions, args: IFlagArgument[]): void {
@@ -187,12 +193,7 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
         }
 
         if (negate) {
-          if (arg.type !== OptionType.BOOLEAN && !arg.optionalValue) {
-            throw new Error(
-              `Negate not supported by --${option.name}. Only optional option or options of type boolean can be negated.`,
-            );
-          }
-          flags[friendlyName] = false;
+          flags[propName] = false;
           return;
         }
 
@@ -238,17 +239,17 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
         if (
           typeof result !== "undefined" && ((args.length > 1) || arg.variadic)
         ) {
-          if (!flags[friendlyName]) {
-            flags[friendlyName] = [];
+          if (!flags[propName]) {
+            flags[propName] = [];
           }
 
-          (flags[friendlyName] as Array<unknown>).push(result);
+          (flags[propName] as Array<unknown>).push(result);
 
           if (hasNext(arg)) {
             parseNext(option, args);
           }
         } else {
-          flags[friendlyName] = result;
+          flags[propName] = result;
         }
 
         /** Check if current option should have an argument. */
@@ -295,7 +296,13 @@ export function parseFlags<O extends Record<string, any> = Record<string, any>>(
   }
 
   if (opts.flags && opts.flags.length) {
-    validateFlags(opts.flags, flags, opts.knownFlaks, opts.allowEmpty);
+    validateFlags(
+      opts.flags,
+      flags,
+      opts.knownFlaks,
+      opts.allowEmpty,
+      optionNames,
+    );
   }
 
   return { flags: flags as O, unknown, literal };

flags/test/option/collect_test.ts

@@ -10,6 +10,8 @@ const options = <IParseOptions> {
     aliases: ["f"],
     type: OptionType.STRING,
     optionalValue: true,
+  }, {
+    name: "no-flag",
   }, {
     name: "string",
     aliases: ["s"],
@@ -61,14 +63,6 @@ Deno.test("flags optionCollect flagTrueNoFlag", () => {
   );
 });
 
-Deno.test("flags optionCollect flagTrueNoFlagTrue", () => {
-  assertThrows(
-    () => parseFlags(["-f", "true", "--no-flag", "true"], options),
-    Error,
-    "Duplicate option: --no-flag",
-  );
-});
-
 Deno.test("flags optionCollect boolean", () => {
   const { flags, unknown, literal } = parseFlags(
     ["-b", "1", "--boolean", "0"],

flags/test/option/depends_test.ts

@@ -111,6 +111,10 @@ const options2 = {
     optionalValue: true,
     depends: ["flag1"],
     default: false,
+  }, {
+    name: "no-flag1",
+  }, {
+    name: "no-flag2",
   }],
 };
 
@@ -130,9 +134,9 @@ Deno.test("flags depends: should accept --standalone", () => {
   assertEquals(literal, []);
 });
 
-Deno.test("flags depends: should not accept --no-flag2", () => {
+Deno.test("flags depends: should not accept --flag2", () => {
   assertThrows(
-    () => parseFlags(["--no-flag2"], options2),
+    () => parseFlags(["--flag2"], options2),
     Error,
     "Option --flag2 depends on option: --flag1",
   );