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

feat(clap_derive): Add default_values_t and default_values_os_t for Vec field types #3933

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

emersonford
Copy link
Contributor

Implements #3932

@emersonford
Copy link
Contributor Author

hm as I coded this up, I realized I could apply this for all Vec<impl Display>, not just a Vec<impl ValueEnum>.

this would be out of scope of #3932, but I'll push another commit with this change as I feel might as well handle it in the same PR? can always reset back to the single commit and submit another PR otherwise :)

@emersonford emersonford force-pushed the vec-value-enum-default-value-t branch 2 times, most recently from 536df7d to 8cb18d8 Compare July 14, 2022 04:34
@emersonford emersonford changed the title feat(clap_derive): Add default_value_t support for Vec of ValueEnum feat(clap_derive): Add default_value_t support for all Vec types Jul 14, 2022
@emersonford emersonford changed the title feat(clap_derive): Add default_value_t support for all Vec types feat(clap_derive): Add default_value_t and default_value_os_t support for all Vec types Jul 14, 2022
clap_derive/src/attrs.rs Outdated Show resolved Hide resolved
clap_derive/src/attrs.rs Outdated Show resolved Hide resolved
clap_derive/src/attrs.rs Outdated Show resolved Hide resolved
clap_derive/src/attrs.rs Outdated Show resolved Hide resolved
@emersonford emersonford force-pushed the vec-value-enum-default-value-t branch from 8cb18d8 to daccb6d Compare July 24, 2022 23:10
@emersonford emersonford changed the title feat(clap_derive): Add default_value_t and default_value_os_t support for all Vec types feat(clap_derive): Add default_values_t and default_values_os_t for Vec field types Jul 24, 2022
@emersonford emersonford force-pushed the vec-value-enum-default-value-t branch from daccb6d to 373905c Compare July 24, 2022 23:15
@emersonford
Copy link
Contributor Author

@epage just pushed the changes to address all of your comments (aside from the swapping multi_value ones as those are no longer relevant when using default_values_t)

Copy link
Member

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the ui tests are passing, I'll merge this.

master is now for clap v4. Once this is merged, if you want you can then cherry-pick this back to v3-master branch and submit a new PR.

@emersonford emersonford force-pushed the vec-value-enum-default-value-t branch 7 times, most recently from c967596 to 62c73ad Compare July 25, 2022 17:13
@emersonford emersonford force-pushed the vec-value-enum-default-value-t branch from 62c73ad to 04e0ed7 Compare July 25, 2022 17:14
@emersonford
Copy link
Contributor Author

that was a bit annoying, my local cargo test output differs from the Github actions cargo test output, so took a minute to resolve. but looks like the test pass now @epage, will submit a PR to cherry pick back to v3 shortly. :)

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

2 participants