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

refactor(validator): Decouple parser #3645

Merged
merged 8 commits into from Apr 21, 2022
Merged

Conversation

epage
Copy link
Member

@epage epage commented Apr 21, 2022

This both makes intent clearer and will make it easier in the future to allow users to customize the parts

On the way to getting there, some of the refactors ended up fixing a couple of bugs

  • We had two different implementations for detecting conflicts with one being a subset of the other. This brings the subset to be in line with the superset
  • We inconsistently treated overrides as conflicts. Now we do so

We had two different implementations of conflicts
- Validating conflicts
- Allowing conflicts to override `required`
  - Missing members of a group conflicting with each other
  - Missing symmetric conflicts (A conflicts with B so B conflicts with
    A)

This consolidates their implementations which should fix overriding of
`required`.
Before, if two arguments were required *and* overrode each other, then
`cmd --opt=1 --other=2` succeded but `cmd --other=2` failed despite
ignoring `--opt=1`.  Requiring `--opt=1` to be present but unavailable
doesn't help anyone and makes the behavior less predictable.

Now both commands will have the same behavior.
This isn't a parser policy but command-level policy.
@epage epage merged commit 8505c47 into clap-rs:master Apr 21, 2022
@epage epage deleted the override_conflicts branch April 21, 2022 23:14
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

Successfully merging this pull request may close these issues.

None yet

1 participant