test(add): Ensure comments are preserved #10849
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @ehuss (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
3 tasks
|
It looks like this is missing from https://github.com/rust-lang/cargo/blob/master/tests/testsuite/cargo_add/mod.rs? |
A comment on killercup/cargo-edit#15 had me worried that `cargo add` was deleting comments now. It appears that isn't the case for inline tables. Standard tables however do delete comments. The work to make sure they don't conflicts with another need. When changing the source, we delete the old source fields and append the new which can cause some formatting to be carried over unnecessarily. For example, what would normally look like ```toml cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" } ``` When fixed to preserve comments with my naive solution looks like ```toml cargo-list-test-fixture-dependency = { optional = true , path = "../dependency", version = "0.0.0" } ``` Note that `optional = true` used to be last, so space separating it and `}` was kept, now separating it and `,`. More work will be needed to get this into an ideal state but we can at least have confidence with inline tables for now.
epage
force-pushed
the
inline
branch
2 times, most recently
from
664c203
to
b78f918
Compare
epage
added a commit
to epage/cargo
that referenced
this pull request
Jul 13, 2022
|
Thanks! Missed that when splitting this out of another branch . Its fixed now. |
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 14, 2022
Update cargo 7 commits in b1dd22e668af5279e13a071ad4b17435bd6bfa4c..8827baaa781b37872134c1ba692a6f0aeb37890e 2022-07-09 14:48:50 +0000 to 2022-07-14 02:56:51 +0000 - Add a test for regressions in selecting the correct workspace root (rust-lang/cargo#10862) - clarify profile used for 'cargo install --debug' (rust-lang/cargo#10861) - servers should use 404 (rust-lang/cargo#10860) - test(add): Ensure comments are preserved (rust-lang/cargo#10849) - Fix nested workspace resolution (rust-lang/cargo#10846) - Small tweaks to the future-incompat docs. (rust-lang/cargo#10856) - Fixed extra period typo (rust-lang/cargo#10847)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A comment on killercup/cargo-edit#15 had me worried that
cargo addwasdeleting comments now. It appears that isn't the case for inline
tables.
Standard tables however do delete comments. The work to make sure they
don't conflicts with another need. When changing the source, we delete
the old source fields and append the new which can cause some formatting
to be carried over unnecessarily.
For example, what would normally look like
When fixed to preserve comments with my naive solution looks like
Note that
optional = trueused to be last, so space separating it and}was kept, now separating it and,.More work will be needed to get this into an ideal state but we can at
least have confidence with inline tables for now.