boolean silently converts string to true

[tttp]

Hi,

me again, in my quest to idiot proof the cli ;)

so I have two params, one foo and one bar

const argv = require("minimist")(process.argv.slice(2), {
  string: ["foo"],
  boolean: ["bar"]
});

if I run it with

--foo alice --bar bob

I get

• argv.foo = alice
• argv.bar=true
• argv._ = ["bob"]

as expected

however, if I run it with

--foo=alice --bar=bob

I get

• argv.foo = alice
• argv.bar=true
• argv._ = []

so a --bar=string is silently converted into --bar = true

is there an option to catch these conversions ?

[ljharb]

That's surprising; both forms (equals sign, or space) are standard for double-dashed args, and I'd strongly expect and prefer always using the = to avoid ambiguity.

In other words, both of those should be indistinguishable imo, and it's a bug that they aren't.

[ljharb]

cc @shadowspawn for your thoughts

[tttp]

--silent A B is IMO correctly doing argv.silent=true (if defined boolean)and argv._ = ["A","B"]
I understand where --silent=A being converted as argv.silent=true (if defined as boolean) comes from, but IMO making it surprising

[ljharb]

if --silent is defined as a string, then --silent A should be the same as --silent=A imo - only if silent is defined as a boolean should it not take a value.

[tttp]

@ljharb, exactly, when set as boolean, --x A and --x=A behave differently.

I understand that if set as boolean it can't take a value, what is non-intuitive is that it does silently discard the string if --x=A but process it as a separate args._. if --x A

and what is the most difficult for me is that minimist allows me to identify and raise an alert if --x A, but doesn't allow me to identify that a user wrongly wrote --x=A (where x being boolean)

[ljharb]

ahh, i see what you mean.

I would expect --x A is two args when x is a boolean, and --x=A would eventually lead to a validation error, as you say.

[shadowspawn]

me again, in my quest to idiot proof the cli ;)

Minimist is not the right tool for that job. But it is fun exploring anyway! 😆

is there an option to catch these conversions ?

Short version: no.

In more common cases, the behaviour with boolean options is fairly self-consistent. Assigning a value to a boolean option on the command-line means Minimist silently processes it.

var parse = require('minimist');

const argv = parse(process.argv.slice(2), { 
    boolean: 'flag',
    string: 'w'
});

console.log(argv);

% node index.js             
{ _: [], flag: false }
% node index.js --flag
{ _: [], flag: true }
% node index.js --flag true
{ _: [], flag: true }
% node index.js --flag false
{ _: [], flag: false }
% node index.js --flag other
{ _: [ 'other' ], flag: true }
% node index.js --flag=true 
{ _: [], flag: true }
% node index.js --flag=false
{ _: [], flag: false }
% node index.js --flag=other
{ _: [], flag: true }

[ljharb]

The "silent" part is what feels like a bug to me. It should either reject it or preserve "other" somewhere.

[shadowspawn]

(Thinking about that...)

[tttp]

A somewhat related issue
#2

it might be enough to offer an option mistyped = (key, value) => {} working similarly to unknown ?

[shadowspawn]

Thinking about example --bar=bob where an option argument is explicitly given to an option declared as boolean.

Possible approaches, and my reservations, leading to a more radical suggestion.

1. reject (say throw): Minimist current accepts anything, so certainly a breaking change.

2. preserve: the value could be preserved directly like { bar: 'bob' } so the issue could be detected by the author. It isn't likely to be a common error. Amusingly, a naive "truthy" test of bar would return true anyway!

However, this would be a change in existing behaviour to make it possible to detect a special case, without handling the special case. So cost/benefit ratio is dubious.

3. mistyped(): I don't think this is common enough to justify an opt-in author supplied handler.

4. status quo: not satisfying, but likely a rare problem in real world use!

5. add .strict(). This would be opt-in so does not break existing users. Strict mode throws for:

• boolean option used with an explicit option argument other than true or false
• string option used without an option argument
• unknown options, unless there is an unknown handler (in which case left to author)

I feel this might address the underlying problem more directly, which is validating the user supplied arguments with the author supplied configuration.

Edit: thinking about API, would be strict: true rather than .strict().

[ljharb]

I like adding a strict mode, which in a future semver-major could be the default.

[shadowspawn]

strict:

unknown options, unless there is an unknown handler (in which case left to author)

correction: throw for unknown options, which includes when unknown() handler returns false

[tttp]

I like it,

going back to my experiments, the last bit that isn't yet covered is the number of argv._ arguments

  if (argv._.length !== 2 ) {
    console.error("missing parameter(s). usage: node index.js foo bar");
    process.exit(1);
  }

What about an optional parameter to .strict(_length: Integer) ?

and throw an Error if argv._.length !== _length

OTH, it'd be saving 4 lines, it probably fits in the cosmetic category.

[shadowspawn]

number of argv._ arguments

Good coverage of possible validations! I think past diminishing returns for now and easy for author to check, both against what is expected and to describe what is missing. (e.g. "please specify one or more filenames"). Could revisit after solidify what strict does.

For interest:

• parseArgs has allowPositionals which is false by default in strict mode. See Make positionals opt-in when strict:true pkgjs/parseargs#116 for discussion leading to that interface. Just has none/any and not an expected count.
• Commander knows what positionals are expected. Error for missing positionals, have to opt-in to get an error for excess positionals.
• Yargs know what positionals are expected, and in strict mode has an error for missing or excess positionals.

.strict(_length: Integer)

It is reasonably common for there to be a variadic positional, whether 0-or-more or 1-or-more, so a single length is not enough. For clarity and future expansion I suggest if there are any options, they should be named like say: .strict({ minPositionals: 1 })

[shadowspawn]

Thinking about strict mode. I think an error for unknown options leads to wanting a more convenient way to mark options as known, but not string or boolean which are special cases. I am wondering about an auto property with list of options, to balance with string and boolean. Automatic options get the normal automatic behaviours: number conversion, and optional option-argument. (Haven't tried code and client examples yet, just thinking!)

Touched on indirectly in: #37 (comment)

[ljharb]

Everything is a string unless otherwise specified - string isn't a special case, it's the primary/only case in a shell.

[shadowspawn]

I am wondering about an auto property with list of options, to balance with string and boolean.

Or perhaps known to use in strict mode for the options which are not covered by opts.string or opts.boolean or opts.alias.

const parse = require('minimist');
try {
    const result = parse(
        process.argv.slice(2), { 
            strict: true,
            string: ['str'],
            boolean: ['bool'],
            known: ['num'],
        }
    );
    console.log(result);
} catch (err) {
    console.error(err.message);
}

% node known.js --oops
Unknown option "oops"
% node known.js --str VALUE --bool --num 123  
{ _: [], bool: true, str: 'VALUE', num: 123 }

[shadowspawn]

Regarding adding auto/known: now thinking number might be more consistent to solve the problem of identifying the known options. Going to try that out in code...

[tttp]

Everything is a string unless otherwise specified

Unless I'm missing something, there is the case of --answer = 42

ie. if a param isn't specified as a string or boolean and that it converts to a number, it's parsed as int.

My understanding is that it wouldn't be possible in the strict mode (at least without introducing a "number":["answer"])

[ljharb]

Yes, that's the point of "strict" - nothing's implicit, and numbers are only numbers if you specify them as such.

[tttp]

but how to specify that a param is a number? you can only define string and boolean as explicit params, not numbers, right?

[shadowspawn]

Yes indeed. To support a transition from existing non-strict usage expecting number conversions, I am experimenting with adding opts.number to parallel opts.string and opts.boolean in #41