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

[bigfix] fix panics from clap internal type casting #1237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JackDyre
Copy link

@JackDyre JackDyre commented Feb 20, 2025

Resolves #1236

Any CLI flags that are constructed with the .value_parser(["slice", "of", "possible", "values"]) would cause clap to panic due to a type casting error whenever parsed into any type other than String, (even &str).

The best solution is to parse to a String and map it to parse it further. The value_parser!() macro is more concise and allows clap to parse to the type specified, but you lose the error message that tells the user the possible values to pass to the flag.

Tests pass

A follow-up PR that adds test coverage to the CLI flags might be a good idea so this doesn't happen again.

@JackDyre
Copy link
Author

JackDyre commented Feb 20, 2025

Oops I didn't notice until now that I did all of this just to be a duplicate of #1233.

That said I believe my PR is more robust as it handles panics in more than just the --num-format flag, and I believe the other PR has a logic error since it calls .ok() on the parse result and doesn't exit if it fails to parse the flag value

@JackDyre JackDyre changed the title fix panics from clap internal type casting [bigfix] fix panics from clap internal type casting Feb 20, 2025
@JackDyre
Copy link
Author

Oops I just noticed this PR is also a duplicate of #1209. Smh I need to do better investiagtion.

I still believe my implementation is better as per my comment about value_parser!() and fixing the other affect CLI flags

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.

panics when passed --num-format flag
1 participant