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 overrides_with("itself") #976

Closed
Tracked by #1037
ericbn opened this issue May 29, 2017 · 4 comments
Closed
Tracked by #1037

Support overrides_with("itself") #976

ericbn opened this issue May 29, 2017 · 4 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies
Milestone

Comments

@ericbn
Copy link

ericbn commented May 29, 2017

Suppose I have defined these:

.arg(flag("foo").overrides_with("foo"))
.arg(flag("bar").overrides_with("bar").takes_value(true).possible_values(&["0", "1", "2"]))

I'd like to allow the options to be able to be defined multiple times, the last one overriding previous ones.

So a call with options --foo --foo --foo --bar=0 --bar=1 --bar=2 will not fail with error: The argument '--foo' was provided more than once, but cannot be used multiple times (or the same for --bar), and both foo, and bar with value 2 would be yield (resulting in the same as calling with only --foo --bar=2).

Or even better, maybe a .setting(AppSettings::POSIX) defined once would enable the previous behavior. Why would defining the same option twice or more yield an error? I just played with some command line tools here (git status --short --short --short, ls -l -l -l, vi -R -R -R README.md) and none complains of options provided multiple times...

@kbknapp kbknapp added A-parsing Area: Parser's logic and needs it changed somehow. D: intermediate C-bug Category: Updating dependencies labels May 30, 2017
@kbknapp
Copy link
Member

kbknapp commented May 30, 2017

Thanks! I like the AppSettings idea too as defining all args as overridable by themselves would be handy.

@ericbn
Copy link
Author

ericbn commented May 30, 2017

Cool. Actually the POSIX specifications don't state nothing about overriding arguments. But the way the POSIX implementation usually involves a loop through the arguments, same arguments just get a count increment or override the previous assigned one.

Maybe the AppSettings could be something more explicit than just AppSettings::POSIX...

BurntSushi pushed a commit to BurntSushi/ripgrep that referenced this issue Jun 12, 2017
to better organize options. These are the changes:
- color will have default value of "never" if --vimgrep is given,
  and only if no --color option is given
- last overrides previous: --line-number and --no-line-number, --heading
  and --no-heading, --with-filename and --no-filename, and --vimgrep and
  --count
- no heading will be show if --vimgrep is defined. This worked inside
  vim actually because heading is also only shown if tty is stdout
  (which is not the case when rg is called from vim).

Unfortunately, clap does not behave like a usual GNU/POSIX in some
cases, as reported in clap-rs/clap#970
and clap-rs/clap#976 (having all the bells
and whistles, on the other hand). So we still have issues like rg
failing when same argument is given more than once (unless for the few
ones marked with `multiple(true)`), or having unintuitive precedence
rules (and probably non-intentional, just there because of clap's
limitations) like:
- --no-filename over --vimgrep
- --no-line-number over --column, --pretty or --vimgrep
- --no-heading over --pretty
regardless of the order in which options where given, where the desired
behavior would be that the last option would override the previous ones
given.
@lilyball
Copy link

lilyball commented Aug 4, 2017

There definitely should be an AppSetting for this (POSIX isn't a good name for this, maybe AppSetting::AllowDuplicateFlags?). And I suppose a per-arg setting as well, in case you want to e.g. allow it in general but disallow it on certain flags that take arguments (for example, you'd probably want to allow it on flags that take arguments specifying config values, e.g. sort options, but disallow it on flags that take arguments specifying input files).

exa just ran into this issue with its argument parsing (which was getopts), and it looks like their solution is to implement a custom argument parser.

And it looks like ripgrep also hit this limitation.

@kbknapp
Copy link
Member

kbknapp commented Feb 3, 2018

This tweet got me thinking about this issue some more, and I'm thinking that because of the new lazy validation I'm doing this could actually be an easy add, and easily forward portable to the v3-master branch. I'm going to take a look at this tomorrow see and if it's as easy as I think it'll be.

@kbknapp kbknapp added this to the 2.29.3 milestone Feb 3, 2018
kbknapp added a commit that referenced this issue Feb 5, 2018
…elves

This allows properly supporting shell aliases and config files.

Before, if an option only accepted a single value and that option was
set inside an alias, the CLI could never override that value.

For instance:

```
$ alias prog='prog --option=value'
$ prog --option=other
```

This would cause an error. Now clap gracefully accepts the new value to replace the old value.

Closes #976
kbknapp added a commit that referenced this issue Feb 5, 2018
kbknapp added a commit that referenced this issue Feb 5, 2018
…elves

This allows properly supporting shell aliases and config files.

Before, if an option only accepted a single value and that option was
set inside an alias, the CLI could never override that value.

For instance:

```
$ alias prog='prog --option=value'
$ prog --option=other
```

This would cause an error. Now clap gracefully accepts the new value to replace the old value.

Closes #976
kbknapp added a commit that referenced this issue Feb 5, 2018
@kbknapp kbknapp mentioned this issue Feb 5, 2018
kbknapp added a commit that referenced this issue Feb 5, 2018
@kbknapp kbknapp closed this as completed in 70a4718 Feb 5, 2018
@kbknapp kbknapp mentioned this issue Feb 5, 2018
87 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

3 participants