-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added logic to preserve carriage returns when updating the manifest file #14983
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[workspace] | ||
|
||
[package] | ||
name = "cargo-list-test-fixture" | ||
version = "0.0.0" | ||
edition = "2015" |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may be auto-converted, if Slightly related, in contributing doc we recommend splitting new tests into its own commit, and it still passes CI. Then followed by a fix and a test update. The test change in the second commit tell that that the bug is actually there, and the patch really fixes it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
use cargo_test_support::compare::assert_ui; | ||
use cargo_test_support::current_dir; | ||
use cargo_test_support::file; | ||
use cargo_test_support::prelude::*; | ||
use cargo_test_support::str; | ||
use cargo_test_support::Project; | ||
|
||
#[cargo_test] | ||
fn case() { | ||
cargo_test_support::registry::init(); | ||
cargo_test_support::registry::Package::new("my-package", "0.1.0").publish(); | ||
|
||
let project = Project::from_template(current_dir!().join("in")); | ||
let project_root = project.root(); | ||
let cwd = &project_root; | ||
|
||
snapbox::cmd::Command::cargo_ui() | ||
.arg("add") | ||
.arg_line("my-package") | ||
.current_dir(cwd) | ||
.assert() | ||
.success() | ||
.stdout_eq(str![""]) | ||
.stderr_eq(file!["stderr.term.svg"]); | ||
|
||
// Verify the content matches first (for nicer error output) | ||
assert_ui().subset_matches(current_dir!().join("out"), &project_root); | ||
|
||
// Snapbox normalizes lines so we also need to do a string comparision to verify line endings | ||
let expected = current_dir!().join("out/Cargo.toml"); | ||
let actual = project_root.join("Cargo.toml"); | ||
assert_eq!( | ||
std::fs::read_to_string(expected).unwrap(), | ||
std::fs::read_to_string(actual).unwrap() | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
[workspace] | ||
|
||
[package] | ||
name = "cargo-list-test-fixture" | ||
version = "0.0.0" | ||
edition = "2015" | ||
|
||
[dependencies] | ||
my-package = "0.1.0" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
[workspace] | ||
|
||
[package] | ||
name = "cargo-list-test-fixture" | ||
version = "0.0.0" | ||
edition = "2015" | ||
|
||
[dependencies] | ||
my-package = "0.1.0" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
use cargo_test_support::compare::assert_ui; | ||
use cargo_test_support::current_dir; | ||
use cargo_test_support::file; | ||
use cargo_test_support::prelude::*; | ||
use cargo_test_support::str; | ||
use cargo_test_support::Project; | ||
|
||
#[cargo_test] | ||
fn case() { | ||
cargo_test_support::registry::init(); | ||
cargo_test_support::registry::Package::new("my-package", "0.1.0").publish(); | ||
|
||
let project = Project::from_template(current_dir!().join("in")); | ||
let project_root = project.root(); | ||
let cwd = &project_root; | ||
|
||
snapbox::cmd::Command::cargo_ui() | ||
.arg("remove") | ||
.arg_line("my-package") | ||
.current_dir(cwd) | ||
.assert() | ||
.success() | ||
.stdout_eq(str![""]) | ||
.stderr_eq(file!["stderr.term.svg"]); | ||
|
||
// Verify the content matches first (for nicer error output) | ||
assert_ui().subset_matches(current_dir!().join("out"), &project_root); | ||
|
||
// Snapbox normalizes lines so we also need to do a string comparision to verify line endings | ||
let expected = current_dir!().join("out/Cargo.toml"); | ||
let actual = project_root.join("Cargo.toml"); | ||
assert_eq!( | ||
std::fs::read_to_string(expected).unwrap(), | ||
std::fs::read_to_string(actual).unwrap() | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[workspace] | ||
|
||
[package] | ||
name = "cargo-list-test-fixture" | ||
version = "0.0.0" | ||
edition = "2015" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this does what we want. However, I am not sure if this should be in toml_edit or in Cargo. Found one relevant comment from an issue: toml-rs/toml#752 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#14863 isn't marked as
S-accepted
. I would generally recommend discussing there, and leave this for implementation only dicussion.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah my bad 😅 I was looking through
C-bug
issues for small bugs to work on to learn/get some experience with the Cargo codebase. I'll be sure to stick to S-accepted or at least discuss further on the issue before starting/opening a PR.