Skip to content

Commit

Permalink
Auto merge of #12191 - epage:mut, r=weihanglo
Browse files Browse the repository at this point in the history
fix(add): Reduce the chance we re-format the user's `[features]` table

### What does this PR try to resolve?

#11743 pointed out that we re-format the users `[features]` table when running `cargo add`  which was a bug introduced in #11099.

This reduces the chance people will run into this problem
- Reducing the scope of the `fmt` call
- Preserving formatting in a simple case

Actually removing the `fmt` case can make some common formatting cases more complex to do "right", so I'm punting on that for now.

### How should we test and review this PR?

Look at the individual commits as I show how each change improves the behavior of `cargo add`.
  • Loading branch information
bors committed May 27, 2023
2 parents 0425b11 + dbf9134 commit 8f49b55
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 3 deletions.
11 changes: 8 additions & 3 deletions src/cargo/util/toml_mut/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,11 @@ fn fix_feature_activations(
for idx in remove_list.iter().rev() {
feature_values.remove(*idx);
}
if !remove_list.is_empty() {
// HACK: Instead of cleaning up the users formatting from having removed a feature, we just
// re-format the whole feature list
feature_values.fmt();
}

if status == DependencyStatus::Required {
for value in feature_values.iter_mut() {
Expand All @@ -511,13 +516,13 @@ fn fix_feature_activations(
} = parsed_value
{
if dep_name == dep_key && weak {
*value = format!("{dep_name}/{dep_feature}").into();
let mut new_value = toml_edit::Value::from(format!("{dep_name}/{dep_feature}"));
*new_value.decor_mut() = value.decor().clone();
*value = new_value;
}
}
}
}

feature_values.fmt();
}

pub fn str_or_1_len_table(item: &toml_edit::Item) -> bool {
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ mod path_dev;
mod path_inferred_name;
mod path_inferred_name_conflicts_full_feature;
mod path_normalized_name;
mod preserve_features_table;
mod preserve_sorted;
mod preserve_unsorted;
mod quiet;
Expand Down
21 changes: 21 additions & 0 deletions tests/testsuite/cargo_add/preserve_features_table/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "xxx"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
your-face = { version = "99999.0.0", optional = true }

[features]
default = [
"a",
"b",
"c",
]
a = [
"your-face?/nose", # but not the mouth and nose
]
b = []
c = []
Empty file.
31 changes: 31 additions & 0 deletions tests/testsuite/cargo_add/preserve_features_table/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

use cargo_test_support::curr_dir;

#[cargo_test]
fn case() {
cargo_test_support::registry::init();
cargo_test_support::registry::Package::new("your-face", "99999.0.0+my-package")
.feature("nose", &[])
.feature("mouth", &[])
.feature("eyes", &[])
.feature("ears", &[])
.publish();

let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("your-face --no-optional")
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
21 changes: 21 additions & 0 deletions tests/testsuite/cargo_add/preserve_features_table/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "xxx"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
your-face = { version = "99999.0.0" }

[features]
default = [
"a",
"b",
"c",
]
a = [
"your-face/nose", # but not the mouth and nose
]
b = []
c = []
7 changes: 7 additions & 0 deletions tests/testsuite/cargo_add/preserve_features_table/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Updating `dummy-registry` index
Adding your-face v99999.0.0 to dependencies.
Features:
- ears
- eyes
- mouth
- nose
Empty file.

0 comments on commit 8f49b55

Please sign in to comment.