-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
#2548: cargo run --example should print available examples #5062
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matklad (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Thanks for this! I wonder if we could perhaps refactor this slightly to be a bit more extensible? For example this doesn't cover flags like |
Thank you for the review @alexcrichton ! Currently I've been focusing on Am I correct in assuming the Finally, I'm struggling to find cargo documentation on the I'll have a go at implementing your suggestion. |
Ah they're all pretty similar and accepted by a number of subcommands, but they're related to the targets (like entries in |
Hi Alex, I'm not feeling confident in how to implement a general fix yet. So I'm putting this off until I understand how to use those tools better. I'm also going to explore docopt more, because the fix may be better off there? Otherwise we're intercepting strings. I feel like I'd need to add tests to make sure the string intercepting doesn't fail. I'm sorry about my slow turnarounds. |
Ah no worries! In the meantime though this may actually want to hold off on more investigation until we sort out #5152 as we may be switching to clap anyway! I do agree though that exploring a better option here may necessitate changes in the command line parser itself, although I suspect that may be a bit easier in clap! |
☔ The latest upstream changes (presumably #5152) made this pull request unmergeable. Please resolve the merge conflicts. |
@SpyR1014 so we've successfully migrated to clap, so I think it should be easier right now to give an error when Currently, the cargo/src/bin/command_prelude.rs Line 59 in 8cc8f3b
And it is processed over here: cargo/src/bin/command_prelude.rs Line 261 in 8cc8f3b
To fix this issue, I think first of all we should tell clap that it's okay to actually supply zero arguments to it. I think it could be done with The same could probably work for |
@matklad Thank you for the help, and congratulations on the migration! |
Closing as I am unable to make progress. |
--{example,bin,bench,test} with no argument now lists all available targets ``` cargo run --bin error: "--bin" takes one argument. Available binaries: cargo error: process didn't exit successfully: `target\debug\cargo.exe run --bin` (exit code: 101) ``` Previous PR: #5062 Closes #2548 Notes: - `optional_opt` is a weird name, can someone come up with a better one? - Should I call clap's `usage()` as well when printing the error message?
Partial fix for this issue.
Handles the more common case. Everything else falls through.
It would be interesting to consider a way of catching
docopt
errors to improve future error messages in cargo's arguments and flags.