init: improve error message and add tests#16643
Conversation
rustbot
added
Command-new
S-waiting-on-review
|
r? @epage rustbot has assigned @epage. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
Note that we prefer atomic commit patterns.
init: improve error message and add tests
The commit summary contains "and" indicating that it might be able to split into multiple. And I think reserved_name_core test case has nothing to do we the the cargo init improvement hence splitting is better.
There was a problem hiding this comment.
My bad. I've split the changes into two atomic commits now
|
|
||
| if path.join("Cargo.toml").exists() { | ||
| anyhow::bail!("`cargo init` cannot be run on existing Cargo packages") | ||
| anyhow::bail!( |
There was a problem hiding this comment.
We are migrating to annotate-snippets #15944. While this case we can defer the migration, we also start following rustc diagnostics style guide closer recently: https://rustc-dev-guide.rust-lang.org/diagnostics.html?highlight=diag#diagnostic-output-style-guide. That means this should have help: and be more direct to action and sometimes more succinct.
There was a problem hiding this comment.
Updated the message to follow the rustc diagnostic style
src/cargo/ops/cargo_new.rs
Outdated
| anyhow::bail!("`cargo init` cannot be run on existing Cargo packages") | ||
| anyhow::bail!( | ||
| "`cargo init` cannot be run on existing Cargo packages\n\ | ||
| If you want to create a new package here, please remove `Cargo.toml` first, \ |
There was a problem hiding this comment.
I am not sure if this is a good suggestion. Removing Cargo.toml just fixes the cargo init validation logic, but other files may still linger. I don't think we should give that suggestion. We could probably give some for certain circumstances though. What is in your mind that is reasonable suggestion?
There was a problem hiding this comment.
Agreed. I've removed the 'remove Cargo.toml' suggestion and replaced it with a more helpful pointer to 'cargo new'
raushan728
force-pushed
the
feature/init-diagnostic-fix
branch
from
8beff23 to
1b71665
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Hi @weihanglo i have updated the err msg style and split the changes into two atomic commits as suggested also cleaned up the merge history. Plz take a look |
rustbot
removed
the
S-waiting-on-review
4 participants
Improves the error message when Cargo.toml exists and adds tests for reserved names