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

Options with optional option argument seem too greedy. A way to change it is needed #1899

Closed
aweebit opened this issue Jul 1, 2023 · 7 comments

Comments

@aweebit
Copy link
Contributor

aweebit commented Jul 1, 2023

I've read into the documentation explaining how to deal with parsing ambiguities and am not entirely satisfied with the suggested solutions. Below is a simple use case scenario where one required argument is expected and there is one option with an optional option argument.

// test.js
import { Command } from 'commander';

const program = new Command()
  .argument('<filename>')
  .option('-p, --preprocess [program]', 'Program to run before processing the file. Default preprocessor is used if no option argument is provided')
  .exitOverride();

try {
  program.parse();
} catch {} finally {
  console.log(program.opts().preprocess);
  console.log(program.args);
}

I run following tests:

# Test 1
node test.js -p file

# Test 2
node test.js --preprocess file

# Test 3
node test.js -pfile

# Test 4
node test.js --preprocess=file

# Test 5
node test.js file -p

# Test 6
node test.js file --preprocess

The expected behavior is that file be treated as the filename argument value in tests 1 and 2, but it was only treated as such in tests 5 and 6, when placed before the option.

So my question is, would it be possible to add a way to make an option with optional option argument lazy, so that it would only grab the immediately following argument value if there was no other interpretation for it? (In my example, this would be the case if a second argument was provided, e.g. node test.js -p program file.)

Maybe something like the following:

program.addOption(new Option('-p, --preprocess [program]').lazy());

Or is there any particular reason why this would not work well?

@aweebit aweebit changed the title Options Options with optional option argument seem too greedy. A way to change it is needed Jul 1, 2023
@shadowspawn
Copy link
Collaborator

For interest, the POSIX behaviour for an option taking an optional argument is that it never takes the following space-separated argument. The option argument can only be specified directly with the option:

node test.js -pfile
node test.js --preprocess=file

Not many argument parsers support this mode, but I do like that it avoids the parsing ambiguity with optional arguments.

@shadowspawn
Copy link
Collaborator

So my question is, would it be possible to add a way to make an option with optional option argument lazy, so that it would only grab the immediately following argument value if there was no other interpretation for it?

It is technically possible, but...

  1. Harder to implement. The current implementation looks at one argument at a time to see if it is a an option or command, and for options taking optional values then looks ahead one argument at a time to see if the argument starts with a dash.

  2. I think this is an uncommon approach. Are you aware of any utilities or libraries that work this way?

@aweebit
Copy link
Contributor Author

aweebit commented Jul 2, 2023

2. I think this is an uncommon approach. Are you aware of any utilities or libraries that work this way?

Unfortunately, I could not find any. There is only this 13 year old Python issue dealing with a similar problem in the argparse module that still has not been resolved: python/cpython#53584.

It is the first time I am creating a command line interface and this seemed to me like the most logical approach, so I am surprised to see there are no libraries that take it.

@shadowspawn
Copy link
Collaborator

For interest, I did some research when working on parseArgs into some widely used implementations and found what I called "greedy" vs "choosy" vs "embedded":

@aweebit
Copy link
Contributor Author

aweebit commented Jul 2, 2023

That is interesting indeed :) Well, if my suggestion does not seem feasible, maybe adding an option to make an option-argument "embedded" (and thus conforming to POSIX) should be considered.

@shadowspawn
Copy link
Collaborator

I have nearly written up POSIX-style a couple of times, done now: #1901

@shadowspawn
Copy link
Collaborator

No further activity in a month. Closing this in favour of #1901.

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

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

2 participants