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(parser): Extract Actions from Parser #3773

Merged
merged 19 commits into from May 31, 2022
Merged

Conversation

epage
Copy link
Member

@epage epage commented May 31, 2022

This is not publicly exposed yet and is a step towards #3405. Nearly all storing of data into ArgMatches is extracted into a react function that is based on an Action enum associated with the Arg. I said "nearly" because we can't quite do this for external subcommand values.

The parser iterates once per argv field. This presented a problem for flags with values and multi-valued positionals. We handle this by tracking "pending" values and resolving them before reacting to the current value. This comes at some behavior changes for ArgMatches::occurrences_of that could be fine for future versions of the actions though we should reconsider this before release.

epage added 19 commits May 27, 2022 15:51
There is a default_missing_vals case which is slightly different because
its not actually a default but closing out the remaining argument that
was started in last iteration.
When creating `PendingValues`, I can't have the lifetime.  I could make
it a `Cow` but decided to hold off instead since we don't need this
right now.  Maybe by the time we do need it, we'll have another way of
doing this.
This changes how occurrences and values are grouped for multiple values.
Today, it appears as a bug.  If we move forward with clap-rs#3772, then this
can make sense.
If we felt this was important long-term, we should fix this outside of
the Action.  Since we might be changing up occurrences (clap-rs#3772), we can
probably get away with a hack.
My hope is to add group actions as well, so we need to qualify what kind
of action this is.
This fixes a compatibility issue introduced in 9805fda
@epage epage merged commit 66567d1 into clap-rs:master May 31, 2022
@epage epage deleted the pending branch May 31, 2022 20:07
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