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.

Sign up for GitHub

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

Jump to bottom

Port clap_derive to ValueParser #3734

Closed
5 tasks done
epage opened this issue May 18, 2022 · 0 comments
Closed
5 tasks done

Port clap_derive to ValueParser #3734

epage opened this issue May 18, 2022 · 0 comments
Labels
A-derive Area: #[derive]` macro API C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot
Milestone
3.x

Comments

@epage
Copy link
Member

epage commented May 18, 2022 •
edited
Loading

Adjusting the derive API to #3732 was proving to be difficult because of the current traits take &ArgMatches and because FromArgMatches::update_from_arg_matches derive for subcommands assumes that ArgMatches remains untouched.

Implementing the derive will automatically address

It will also let us deprecate

  • Deprecate possible_values in favor of value_parser
  • Deprecate allow_invalid_utf8 in favor of ValueParser
  • Deprecate arg value validators in favor of ValueParser
  • Deprecate forbid_empty_values
  • Deprecate value_* in favor of get_*
@epage epage added C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot A-derive Area: #[derive]` macro API labels May 18, 2022
@epage epage mentioned this issue May 18, 2022
Merged
@epage epage added this to the 3.x milestone May 18, 2022
@epage epage closed this as completed in b52c7f1 May 23, 2022
epage added a commit to epage/clap that referenced this issue May 24, 2022
@epage
fix(parser): Clean up remove types
ed45de2 
The remove functions no longer return `Arc` but the core type, at the
cost of requiring `Clone`.  I originally held off on this
in clap-rs#3732 in the hope of gracefully transition the derive and requiring
`Clone` would have been a breaking change but when it came to clap-rs#3734, I didn't
find a way to make it work without a breaking change, so I made it
opt-in.  This means I can force the `Clone` requirement now.

I added the requirement for `Clone` everywhere else in the hopes that in
the future, we can drop the `Arc` without a breaking change.
This was referenced May 24, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-derive Area: #[derive]` macro API C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot
Projects
None yet
Milestone
3.x
Development

No branches or pull requests
1 participant
@epage