Skip to content

Better error when a ValueValidation error occurs #3310

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

Merged
merged 3 commits into from
Apr 5, 2023

Conversation

0xPoe
Copy link
Member

@0xPoe 0xPoe commented Apr 4, 2023

@0xPoe 0xPoe marked this pull request as draft April 4, 2023 02:59
@0xPoe 0xPoe changed the title Print help when a ValueValidation error occurs [WIP] Print help when a ValueValidation error occurs Apr 4, 2023
@rbtcollins
Copy link
Contributor

The test shows nice behaviour. It seems like clap has some way to go to make this have a really nice interface.

@0xPoe 0xPoe force-pushed the rustin-patch-help branch 2 times, most recently from fafbbd6 to 5e5b53e Compare April 4, 2023 16:09
@0xPoe 0xPoe changed the title [WIP] Print help when a ValueValidation error occurs Better error when a ValueValidation error occurs Apr 4, 2023
@0xPoe 0xPoe force-pushed the rustin-patch-help branch from 5e5b53e to a6a8587 Compare April 4, 2023 16:12
@0xPoe 0xPoe requested a review from rbtcollins April 4, 2023 16:12
@0xPoe 0xPoe marked this pull request as ready for review April 4, 2023 16:12
@0xPoe
Copy link
Member Author

0xPoe commented Apr 4, 2023

The test shows nice behaviour. It seems like clap has some way to go to make this have a really nice interface.

I changed it again. Because the help text in this commit is still not very user-friendly. Because it does not show the value which the user passed to the rustup.

Also, I think print help text for users is not consistent with other errors. So I prefer to just tell the users why we print the +toolchain error. Instead of printing the whole help text(it still does not explain why we treat it as a toolchain name.)

Signed-off-by: hi-rustin <[email protected]>
@0xPoe 0xPoe merged commit e724717 into rust-lang:master Apr 5, 2023
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.

2 participants