Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restrict the number of allowed options/strict mode #37

Open
tttp opened this issue May 1, 2023 · 9 comments
Open

restrict the number of allowed options/strict mode #37

tttp opened this issue May 1, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request help wanted pull requests welcome

Comments

@tttp
Copy link

tttp commented May 1, 2023

Hi,

I'm not sure what's the best place to discuss feature, feel free to close and direct me to a better place

Some users of our cli mistype an argument name (eg the cli expects --dry-run and they type --dryrun).

If would be useful to have a strict mode that stops the process if there is an unknown error

(eg. invalid parameter "dryrun")

I tried to implement it using unknown

unknown: (param) => {param[0] === '-' ? console.error("invalid parameter",param) || process.exit(1) : true},

however,

  1. it would be nice to have to have a second parameter "type" on unknown (ie "-" "--" "" if alias, normal, or argv.)
  2. I can't figure out how to handle valid params that are int (eg aren't listed into boolean or string), is there an "any" or "integer" option?

X+

@tttp tttp changed the title limit the number of options/strict mode restrict the number of allowed options/strict mode May 1, 2023
@ljharb
Copy link
Member

ljharb commented May 1, 2023

I'm not sure what you mean - the unknown function option should only be called with a truly unknown arg (like "dryrun") and you can throw an error there to handle it.

@tttp
Copy link
Author

tttp commented May 1, 2023

yes, but how do you tell minimist that

  • an integer param is "known" (the same way you can with the boolean option)
  • an "any" param is "known"
  • an boolean param is set but with a value
    (non exhaustive list)

so say if I have --min=12 --dry-run=dummy --mode --extra=bling

and options= {boolean:["dry-run"], string: ["mode"]}

if there an helper to make it easy to flag that:

  • dry-run is string but should be boolean
  • no alert for min as it's expected to be a number
  • mode should be a boolean
  • extra is unknown

right now, I don't know how to handle integer params (they will always be pushed to my unkown function), and if there is type error, it's silently dropped, isn't it?

Basically, minimist is awesome for quick prototyping, but once I'm clear about which parameters are expected, I'd like to "freeze" that list and display errors when the user puts unexpected parameters (eg either wrong types or unknown params)

@ljharb
Copy link
Member

ljharb commented May 1, 2023

Your unknown would return true for the unknown things you want, and everything else throws.

Since dry-run is marked as a boolean, I'd expect it would be pretty trivial to validate that for yourself after parsing is completed?

@shadowspawn
Copy link
Collaborator

shadowspawn commented May 2, 2023

Minimist does not have much in the way of "strict" support. The unknown function is specifically just called for arguments that are unknown. It does not support checking the option-value against the option "type". Likewise, the implicit conversion of an option value to integer is a bonus from the automatic loose processing and not a "type" as such.

One way of getting an integer option to at least be recognised is to give it an alias.

Given this code:

var argv = require('minimist')(process.argv.slice(2), {
    boolean:["dry-run"], 
    string: ["mode"],
    alias: {min: 'm'},
    unknown: (unknownParam) => {
        console.log({unknownParam});
    }
});
console.log(argv);

I see this output for your example line:

% node index.js --min=12 --dry-run=dummy --mode --extra=bling
{ unknownParam: '--extra=bling' }
{ _: [], 'dry-run': true, min: 12, m: 12, mode: '', extra: 'bling' }

@shadowspawn
Copy link
Collaborator

shadowspawn commented May 2, 2023

Basically, minimist is awesome for quick prototyping, but once I'm clear about which parameters are expected, I'd like to "freeze" that list and display errors when the user puts unexpected parameters (eg either wrong types or unknown params)

You might like to take a look at parseArgs which is available from Node.js 16. Both @ljharb and I helped with design and implementation, and it is strict by default so better addresses your goals in that regard.

https://nodejs.org/dist/latest-v18.x/docs/api/util.html#utilparseargsconfig

parseArgs does not support integer conversion, so that is left up to you. However, it would handle all four of your examples in strict mode with appropriate config:

  • dry-run is string but should be boolean: ERROR
  • no alert for min as it's expected to be a (number) string: OK
  • mode should be a boolean: OK
  • extra is unknown: ERROR

I can do an example program matching your example if you would like to see one.

@tttp
Copy link
Author

tttp commented May 2, 2023

nice trick to use alias!

this is what I have so far

const argv = require("minimist")(process.argv.slice(2), {
  boolean: ["help", "dry-run", "verbose"],
  string: ["to"],
  unknown: (d) => {
    const allowed = ["min"];
    if (d[0] !== "-") return true;
    if (allowed.includes(d.split("=")[0].slice(2))) return true;
    console.error("unknown param");
    help(1);
  },
});
if (argv._.length !== 1) {
  console.log(color.red("only one unamed param allowed"), argv._);
  help();
}

to make it more compact, ideally:

  • unknown would provide one other params: (unknownParam, name)
    where name is
  1. if named -> unknownParam.split("=")[0].slice(2)) // doesn't properly handle the alias format
  2. if unamed -> undefined

@tttp
Copy link
Author

tttp commented May 2, 2023

You might like to take a look at parseArgs

thanks for the pointer, I completely missed that one!

@tttp tttp closed this as completed Jun 8, 2023
@ljharb
Copy link
Member

ljharb commented Jun 8, 2023

parseArgs existence, however, doesn’t mean minimist can’t be improved :-) this is still worth adding.

@ljharb ljharb reopened this Jun 8, 2023
@ljharb ljharb added enhancement New feature or request help wanted pull requests welcome labels Jun 8, 2023
@shadowspawn
Copy link
Collaborator

I raised the idea of a strict mode at the end of #39 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted pull requests welcome
Projects
None yet
Development

No branches or pull requests

3 participants