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

Support using a function to get default value for option and arg #1036

Closed
ngocdaothanh opened this issue Aug 31, 2019 · 11 comments
Closed

Support using a function to get default value for option and arg #1036

ngocdaothanh opened this issue Aug 31, 2019 · 11 comments

Comments

@ngocdaothanh
Copy link

ngocdaothanh commented Aug 31, 2019

I have a use case like this:
If a required option or arg is not given, fall back to using Inquirer.js to prompt users to input.

The current signature of #option:

Command.prototype.option = function(flags, description, validationFn, defaultValue)

One solution to my use case is to change it to this:

Command.prototype.option = function(flags, description, validationFn, defaultValueOrFn)

When defaultValueOrFn is a function, it will return a Promise containing the default value.

Similarly for #parseExpectedArgs (and related methods).

Is there a workaround, without having to change Commander.js?
If there's no workaround, I'll create a PR.

@shadowspawn
Copy link
Collaborator

Work-arounds:

For options, you could prompt for missing values after the parse.

For non-option arguments, a missing required argument is intended to generate an error (although there are some holes in the error detection!). For some cases you might be able to mark the argument as optional rather than required and prompt for missing values after the parse.

(There is a long thread discussing mandatory arguments including options in #230.)

@shadowspawn
Copy link
Collaborator

One solution to my use case is to change it to this:
Command.prototype.option = function(flags, description, validationFn, defaultValueOrFn)

That won't work with the existing call patterns. The default value is used as a starting value for custom option processing:
https://github.com/tj/commander.js#custom-option-processing

@ngocdaothanh
Copy link
Author

ngocdaothanh commented Sep 1, 2019

The default value is used as a starting value for custom option processing:
https://github.com/tj/commander.js#custom-option-processing

I think there's a bug (if it's not a bug, then it's perfect for my proposed solution about defaultValueOrFn):

  • Both custom option processing function and default value are set. When running, users do not provide value for the option.
  • Actual behavior: the function is not called.
  • Expected behavior: the function is always called.

Example:

const program = require('commander');

function myParseInt(value, dummyPrevious) {
  console.log('Inside myParseInt', value, dummyPrevious);
  return parseInt(value);
}

program.option('-i, --integer <number>', 'integer argument', myParseInt, 123);

program.parse(process.argv);
console.log(program.integer);
$ node myprogram.js 
123

$ node myprogram.js -i 456
Inside myParseInt 456 123
456

@ngocdaothanh
Copy link
Author

For some cases you might be able to mark the argument as optional rather than required and prompt for missing values after the parse.

Please let me explain the motivation behind this feature request:

For a good user experience, when we design a command like this:

mycommand <arg1> [arg2] --option1 <something> --option2 [something]

We want to communicate with users that:
Logically (business logic), for this command to work, users need to specify arg1 and option1. They are required for the business logic.

When users do not specify arg1 and option1, displaying prompts (using Inquirer) is just a fallback so that users have a better user experience. It still communicates well with users that they need to specify arg1 and option1.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 1, 2019

Both custom option processing function and default value are set. When running, users do not provide value for the option.
Actual behavior: the function is not called.
Expected behavior: the function is always called.

This is as intended The custom option processing function is only called when the option is specified on the command line.

@shadowspawn
Copy link
Collaborator

Please let me explain the motivation behind this feature request:

Thank you for the nice explanation.

(My suggested work-around was not a solution, just an idea for offering prompt without changing Commander.)

@ngocdaothanh
Copy link
Author

I'm thinking about a workaround by wrapping Command like this:

Use myOption and myAction to wrap Command#option and Command#action.

When

myOption('-s -long <required>')

is called, call

option('-s -long [required]')

to parse the option (required is not actually required)

Then override

cmd.options[cmd.options.length - 1].flags = '-s -long <required>'

so that when help for the option is displayed, users will see

-s -long <required>

Also set

cmd.options[cmd.options.length - 1].requiresFallback = true

When myAction(actionFn) is called, check requiresFallback and display Inquirer prompt if the option value is not specified.

Inside myAction I have to manually call the custom option transformation function.

For command args, do something similarly.

This workaround is very ugly, so it's best to enhance Commander.js to support using a function to get default value for option and arg.


Command.prototype.option = function(flags, description, transformFn, defaultValueOrFn)
That won't work with the existing call patterns. The default value is used as a starting value for custom option processing

We tune the design like this:
When defaultValueOrFn is a function, when calling it, we pass user input value to it (or undefined if user didn't input anything). Based on that information, the function can return proper default value.

What do you think?

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 6, 2019

We tune the design like this:
When defaultValueOrFn is a function, when calling it, we pass user input value to it (or undefined if user didn't input anything). Based on that information, the function can return proper default value.

What do you think?

I think it may be too complicated to add this behaviour for a special case. I am painfully aware that took years to catch up with adding a parameter to turn the coercion function into a more general purpose custom option processing. This feels a bit similar.

More specifically though, I want to add support for mandatory options before trying to add support for prompting for their value. (#230)

@shadowspawn
Copy link
Collaborator

Early similar issue: #4

@shadowspawn
Copy link
Collaborator

On a related note, I noticed yargs has a concept of middleware for additional processing of arguments before they are processed, with an example of reading option values from a file if not specified:
https://github.com/yargs/yargs/blob/master/docs/advanced.md

@shadowspawn
Copy link
Collaborator

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

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

Thank you for your contributions.

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

No branches or pull requests

2 participants