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

Weird allowUnknownOption behavior #609

Closed
kevinrenskers opened this issue Mar 8, 2017 · 11 comments
Closed

Weird allowUnknownOption behavior #609

kevinrenskers opened this issue Mar 8, 2017 · 11 comments

Comments

@kevinrenskers
Copy link

I'd like to allow any and all options, because my application has plugins that can support their own options that my code itself doesn't know about. I just want to pass on all options as given to plugins, and they pick the ones they know about (hope this makes sense so far).

Sadly allowUnknownOption doesn't work as I expected, and I was wondering if this is a bug, or am I missing something?

const program = require('commander');

program
  .version('1.0.0')
  .allowUnknownOption()
  .option('-i, --input [input]', 'input file')
  .option('-o, --output [output]', 'output file')
  .parse(process.argv);

console.log(program);

When I run this with node test.js --foo bar then I would expect that program.args would contain the foo option with the bar value, but instead program.args is empty.

Only when I run the script like node test.js foo bar, then program.args contains two values. So it seems that unknown options can't be supplied with the -- prefix, which means they don't fit well with the "regular" options that my application itself supports, and that plugins can't really expect to get key/value pairs?

At the moment it seems like plugins would need to re-process the program.rawArg themselves using Commander with their own explicitly configured options, which is a shame to explain to my plugin authors.

@wis3guy
Copy link

wis3guy commented Mar 8, 2017

@kevinrenskers I am no nodejs developer, but if i look at these test cases in the commander repo (https://github.com/tj/commander.js/blob/master/test/test.command.allowUnknownOption.js) my impression is that you misinterpret the meaning of . allowUnknownOption().

You seem to think/hope that it means that all and any parameter will be parsed. The test cases tell me that including that method call will instead only prevent commander from generating errors in case it encounters arguments that were not configured and thus expected.

@kevinrenskers
Copy link
Author

That is indeed what I am hoping for :) Also what the answer to #426 seemed to imply.

@wis3guy
Copy link

wis3guy commented Mar 8, 2017

I may be way out of my league here, but i wonder if the path you intend to go down truly meets your use case. Do you really want people to specify theme specific options on the command line, along with the options/arguments that drive the core conversion? They feel like two different concerns to me.

Personally i would be more comfortable with a single, optional argument that allows me to point to a config file. Something like:

raml2html --theme slate --theme-config slate_cfg.json -i example.raml -o example.html

That way the command line remains concise whilst a theme designer can go wild with theme driving settings. I could imagine a config file to look like:

{
   "logo_url" : "http://www.example.com/img/foo.png",
   "colors: {
      "text" : "#c0c0c0",
      "background" : "#ff00ff"
   }
}

This also spares you from having to figure out how to properly encode the command line arguments in case they contain fancy characters and complex objects.

@kevinrenskers
Copy link
Author

Not a bad idea, but I feel like that just puts the burden on the end user of the tool. Instead of just quickly firing off a command they'd now have to create a config file.

It would be awesome if Commander would work as I described in the issue above, I feel that it does make sense that unknown options are still parsed into program.args. Also for example adding the param foo works, but --foo doesn't - but that's also an unknown param right? And according to the docs, it's supposed to end up in program.args.

So at least partly it feels like a bug, and partly like a feature request.

@danilopolani
Copy link

+1, is there a way to retrieve the unknown options?

@kevinrenskers
Copy link
Author

In the end I switched to yargs which works great for me.

@danilopolani
Copy link

Thank you for the tip, unfortunately I'm editing a file for a PR so I can't remove their usage of Commander... I think I would parse with a regex the --flags

@shadowspawn
Copy link
Collaborator

shadowspawn commented Nov 12, 2017

There is a caller work-around. I had tried a pattern I had seen in another program of using bare "--" to stop processing options and leave them for subcommand. I found it worked in commander too, and am using this technique.

See comments in #216

program
  .command("test <command> [args...]")
  .option("--expected")
  .action((command, args, options) => {
    console.log(`expected is ${options.expected}`);
    console.log(`command is ${command}`);
    console.log(`args is ${args}`);

example test --expected -- a b c --unexpected
produces

expected is true
command is a
args is b,c,--unexpected

@jrgtt
Copy link

jrgtt commented Nov 8, 2018

Been using commander for a cli project and it's nice project, but possibly will drop it due to what @kevinrenskers described here. The allowUnkownOption seems counterintuitive, or more or less useless, I would like it to be able to get options that I don't expect myself and would be nice to be able to break the script execution when setting it false.

@shadowspawn
Copy link
Collaborator

This issue has not had any activity in over six months. It isn't likely to get acted on due to this report.

The current allowUnknownOption is very limited, it simple suppresses the error message without offering any support for identifying or further processing of the unknown options. Still open issues covering this are: #802 #903

Feel free to open a new issue if it comes up again, with new information and renewed interest.

Thank you for your contributions.

@shadowspawn
Copy link
Collaborator

Note: from Commander 5 unknown options are passed through as command arguments, as per the original comment here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants