deno_std icon indicating copy to clipboard operation
deno_std copied to clipboard

bug(flags): boolean flag doesn't equal assigned value

Open timreichen opened this issue 2 years ago • 3 comments

Describe the bug A flag which is set as a boolean shouldn't be parsed as a boolean if it has a value assigned with the equals sign as described:

options.boolean - a boolean, string or array of strings to always treat as booleans. if true will treat all double hyphenated arguments without equal signs as boolean (e.g. affects --foo, not -f or --foo=bar).

Steps to Reproduce

import { parse } from "https://deno.land/[email protected]/flags/mod.ts";
const args = parse(["--reload=value"], { boolean: ["reload"] });

parse returns { _:[], reload: true }

Expected behavior parse should return { _:[], reload: "value" }

Environment

  • OS: MacOS 12.4
  • deno version: 1.22.0
  • std version: 0.143.0

timreichen avatar Jun 13 '22 20:06 timreichen

Maybe this check https://github.com/denoland/deno_std/blob/main/flags/mod.ts#L310 should just be removed and always run setArg(key, value, arg) instead?

sigmaSd avatar Jun 13 '22 20:06 sigmaSd

I think the returned result is correct here because --reload=value is only parsed as string if boolean is set to true, but in your example it is set to ["reload"], which explicitly marks reload as a boolean argument.

a boolean, string or array of strings to always treat as booleans.

if true will treat all double hyphenated arguments without equal signs as boolean (e.g. affects --foo, not -f or --foo=bar).

But i noticed the values for short flags are wrong:

import { parse } from "https://deno.land/[email protected]/flags/mod.ts";

const args1 = parse(["-r=value"], { boolean: ["r"] });

const args2 = parse(["-r=value"], {
  boolean: ["reload"],
  alias: { reload: ["r"] },
});

args1 returns { _:[], reload: "value" } args2 returns { _: [], r: "value", reload: "value" }

Expected behavior args1 should return { _:[], reload: true } args2 should return { _: [], r: true, reload: true }

c4spar avatar Jun 14 '22 20:06 c4spar

I think the returned result is correct here because --reload=value is only parsed as string if boolean is set to true, but in your example it is set to ["reload"], which explicitly marks reload as a boolean argument.

Oh, that is kinda strange. What if you want some flags to behave like you set { boolean: true } but not others? I think this that boolean property is not very intuitive.

What if we redesigned the parse options to something more intuitive and with a better overview with objects that represent flags? For example:

parse(args, {
  flags: [
    {
      name: "reload",
      alias: "r",
      converter(value: string) { return value == null || value }
    },
    {
      name: "some-list",
      default: "a,b,c",
      args: [{ name: "list" }],
      converter(value: string) { return value.split(",") }
    },
    {
      name: "log-level",
      description: `Set log level [possible values: debug, info]`,
      alias: "L",
      args: [{ name: "log-level" }],
    }
  ]
})

timreichen avatar Jun 14 '22 20:06 timreichen