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(usage): Remove redundant duplicate parameters when there is a conflict error #3688

Closed
wants to merge 1 commit into from

Conversation

leolovenet
Copy link

@leolovenet leolovenet commented May 4, 2022

I compared other code that uses the create_usage_with_title function and found that part of the filter was missing, so I added.

fix: #3665 and #3556

@epage
Copy link
Member

epage commented May 4, 2022

Curious about your thoughts on this approach vs #3689?

If we leave it to the caller, I suspect we should add a debug_assert that makes sure that the required arguments don't overlap with included arguments

@leolovenet
Copy link
Author

I totally agree with you and should not leave it to the caller.

The only question is whether is_hide_set should be checked in the build_conflict_err_usage function, because I looked at all functions that call create_usage_with_title without the &[] argument, and only build_conflict_err_usage doesn't check is_hide_set

Anyway, I'm still a newbie to using clap and not very proficient with the whole framework

I will close this comment, Thanks for your reply

@leolovenet leolovenet closed this May 5, 2022
@epage
Copy link
Member

epage commented May 5, 2022

I've updated my PR with is_hide_set. Good call!

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.

Duplicate output in USAGE message if called with conflict argument
2 participants