Fix cargo add behaving different when translating package name #13765
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
Command-add
S-waiting-on-review
dohse
force-pushed
the
fix-13702
branch
2 times, most recently
from
b7d4b79
to
8340710
Compare
tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/mod.rs
Outdated
Show resolved
Hide resolved
dohse
force-pushed
the
fix-13702
branch
2 times, most recently
from
44abbff
to
d24d92e
Compare
src/cargo/ops/cargo_add/mod.rs
Outdated
| @@ -364,7 +364,24 @@ fn resolve_dependency( | |||
| }; | |||
| selected_dep = populate_dependency(selected_dep, arg); | |||
|
|
|||
| let old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?; | |||
| let mut old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?; | |||
| if old_dep.is_none() && selected_dep.source().is_none() && selected_dep.rename().is_none() { | |||
There was a problem hiding this comment.
selected_dep.source().is_none() to try the mutations only on when looking for registry dependencies and selected_dep.rename().is_none() to not try permutations when adding a renamed dependency.
src/cargo/ops/cargo_add/mod.rs
Outdated
| let mut old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?; | ||
| if old_dep.is_none() && selected_dep.source().is_none() && selected_dep.rename().is_none() { | ||
| for name_permutation in [ | ||
| selected_dep.name.replace('-', "_"), | ||
| selected_dep.name.replace('_', "-"), | ||
| ] { |
There was a problem hiding this comment.
Instead of looking up old_dep and then looking it up again, could we include the user-selected name in the list of permutations?
There was a problem hiding this comment.
I will try that, but I guess I have handle the rename case then separately.
src/cargo/ops/cargo_add/mod.rs
Outdated
| for name_permutation in [ | ||
| selected_dep.name.replace('-', "_"), | ||
| selected_dep.name.replace('_', "-"), | ||
| ] { |
There was a problem hiding this comment.
Should we break this out into a function to be reused between this and the call to find_workspace_dep?
There was a problem hiding this comment.
I will try that. Ideally package and workspace dependency lookup have the same logic.
There was a problem hiding this comment.
I did this and pushed a new version with that
Fixes #13702
Fixes #10680
TODOs
cargo removeas well