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

fix(parser): Resolve problems around allow_hyphen_values #4187

Merged
merged 12 commits into from
Sep 7, 2022

Conversation

epage
Copy link
Member

@epage epage commented Sep 7, 2022

The main focus is on making Arg::allow_hyphen_values behave like Command::allow_hyphen_values and removing the latter. Some debug asserts and documentation got improved along the way.

This supersedes #4039

Fixes #3880
Fixes #1538
Fixes #3450

epage and others added 9 commits September 6, 2022 14:34
This has been a bit out of place being on the command.  Now its clearer
what the user intends to be the trailing var arg and it is more likely
to be discovered.
Now that we have it on `Arg`, we don't need it on `Command`
…first pos

This makes it match up with `Command::allow_hyphen_values` which was the
guiding factor for what the behavior should be.

This supersedes clap-rs#4039

Fixes clap-rs#3880
Fixes clap-rs#1538
CHANGELOG.md Show resolved Hide resolved
@epage epage merged commit f97670a into clap-rs:master Sep 7, 2022
@epage epage deleted the exp branch September 7, 2022 12:39
epage added a commit to epage/cargo that referenced this pull request Oct 7, 2022
When working on clap v4, it appeared that `last` and `trailing_var_arg`
are mutually exclusive, so I called that out in the debug asserts in
clap-rs/clap#4187.  Unfortunately, I didn't document my research on this
as my focus was elsewhere.

When updating cargo to use clap v4 in rust-lang#11159, I found `last` and
`trailing_var_arg` were used together.  I figured this was what I was
trying to catch with above and I went off of my intuitive sense of these
commands and assumed `trailing_var_arg` was intended, not knowing about
the `testname` positional that is mostly just a forward to `libtest`,
there for documentation purposes, except for a small optimization.

This restores us to behavior from 531ce13 which is what I originally
wrote the tests against.
epage added a commit to epage/cargo that referenced this pull request Oct 7, 2022
When working on clap v4, it appeared that `last` and `trailing_var_arg`
are mutually exclusive, so I called that out in the debug asserts in
clap-rs/clap#4187.  Unfortunately, I didn't document my research on this
as my focus was elsewhere.

When updating cargo to use clap v4 in rust-lang#11159, I found `last` and
`trailing_var_arg` were used together.  I figured this was what I was
trying to catch with above and I went off of my intuitive sense of these
commands and assumed `trailing_var_arg` was intended, not knowing about
the `testname` positional that is mostly just a forward to `libtest`,
there for documentation purposes, except for a small optimization.

So it looks like we just need the `last` call and not the
`trailing_var_arg` call.

This restores us to behavior from 531ce13 which is what I originally
wrote the tests against.
wmmc88 referenced this pull request in rust-lang/cargo Nov 2, 2023
This mirrors some of the categories from `cargo help` (the man pages).
There are fewer categories to avoid extra vertical space.  Instead, they
are left int the `Options` category but put first.
eatradish added a commit to AOSC-Tracking/choose that referenced this pull request Mar 30, 2024
structopt has been replaced by clap,
whose dependency libc does not build on newer architectures (e.g., loongarch64).
My previous attempts to port choose to clap v4 resulted in an incompatible change in allow_hyphen_values's behavior - I have reported this issue to the upstream:

- clap-rs/clap#5434
- clap-rs/clap#4187

To simplify things, I wrote my own argument parser for clap..

This change passes `cargo test` .
eatradish added a commit to AOSC-Tracking/choose that referenced this pull request Mar 30, 2024
structopt has been replaced by clap,
whose dependency libc does not build on newer architectures (e.g., loongarch64).
My previous attempts to port choose to clap v4 resulted in an incompatible change in allow_hyphen_values's behavior - I have reported this issue to the upstream:

- clap-rs/clap#5434
- clap-rs/clap#4187

To simplify things, I wrote my own argument parser for clap..

This change passes `cargo test` .
eatradish added a commit to AOSC-Tracking/choose that referenced this pull request Mar 30, 2024
structopt has been replaced by clap,
whose dependency libc does not build on newer architectures (e.g., loongarch64).
My previous attempts to port choose to clap v4 resulted in an incompatible change in allow_hyphen_values's behavior - I have reported this issue to the upstream:

- clap-rs/clap#5434
- clap-rs/clap#4187

To simplify things, I wrote my own argument parser for clap..

This change passes `cargo test` .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants