feat(add/install): check if given crate argument would be valid with inserted @ symbol #15441
Conversation
rustbot
added
A-cli
There was a problem hiding this comment.
Not a blocker. Would you mind following the atomic commit pattern described here. That is,
As part of your atomic commits, prefer adding tests as their own commit before any functionality changes. The tests should pass in each commit, demonstrating the behavior before your change and how each commit affects behavior.
Oops, sorry. Should have seen that. |
|
As a suggestion to clarify what is being asked by the link to the contributor guide, including the tip. The commits could be organized as:
We're fine with your rewriting your commits during the review process. |
… symbol is missing for the version
…bol is missing for the version
…rted @ symbol, suggest fixed argument
dawedawe
force-pushed
the
suggest_crate_name_fix
branch
2 times, most recently
from
3180ca9 to
c46ba9c
Compare
… @ symbol, suggest fixed argument
dawedawe
force-pushed
the
suggest_crate_name_fix
branch
from
c46ba9c to
8c9b373
Compare
|
Okay, I hope the history is acceptable now. |
| @@ -148,7 +148,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Could you fix this in the relevant commit?
| suggested_crate_name.insert_str(idx, "@"); | ||
| if let Ok((_, Some(_))) = parse_crate(&suggested_crate_name.as_str()) { | ||
| return Err(anyhow!( | ||
| "possible missing `@` detected in package name `{crate_name}`. Try `{suggested_crate_name}` instead." |
There was a problem hiding this comment.
This does not read as an error message to me (saying what is wrong) but instead as a suggestion of what could be wrong and how that could be fixed which we usually put in help: sections of an error message.
I'd keep the original package_name.unwrap_err() and append something like this (note: I've not thought too deeply on this wording):
help: if this is meant to be a package name followed by a version, insert a `@` like `{suggested_crate_name}`
Thanks, this helps a lot! |
Suggest to user to use a crate name with an inserted @ before the first invalid package name character
Fixes #15318