Open tttp opened 1 year ago
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.
cc @shadowspawn for your thoughts
--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
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.
@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)
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.
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 }
The "silent" part is what feels like a bug to me. It should either reject it or preserve "other" somewhere.
(Thinking about that...)
A somewhat related issue https://github.com/minimistjs/minimist/issues/2
it might be enough to offer an option mistyped = (key, value) => {} working similarly to unknown ?
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:
true
or false
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()
.
I like adding a strict mode, which in a future semver-major could be the default.
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
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.
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 https://github.com/pkgjs/parseargs/pull/116 for discussion leading to that interface. Just has none/any and not an expected count..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 })
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: https://github.com/minimistjs/minimist/issues/37#issuecomment-1530981069
Everything is a string unless otherwise specified - string isn't a special case, it's the primary/only case in a shell.
I am wondering about an
auto
property with list of options, to balance withstring
andboolean
.
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 }
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...
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"])
Yes, that's the point of "strict" - nothing's implicit, and numbers are only numbers if you specify them as such.
but how to specify that a param is a number? you can only define string and boolean as explicit params, not numbers, right?
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
Hi,
me again, in my quest to idiot proof the cli ;)
so I have two params, one foo and one bar
if I run it with
I get
as expected
however, if I run it with
I get
so a --bar=string is silently converted into --bar = true
is there an option to catch these conversions ?