Skip to content
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

Comments aren't preserved when modifying an existing stanrdard toml table with cargo add #10850

Closed
epage opened this issue Jul 11, 2022 · 2 comments · Fixed by #12838
Closed
Labels

Comments

@epage
Copy link
Contributor

epage commented Jul 11, 2022

Problem

cargo add can modify existing entries, like adding more features. When using a standard table (as opposed to an inline table, see #10849), comments get deleted.

Steps

Note: steps are using this is a test case in cargo and these crates don't normally exist

With Cargo.toml:

[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

[dependencies.your-face]
version="99999.0.0"  # Hello world
features=["eyes"]  # Goodbye moon

run

$ cargo add your-face -F features nose

and you'll get

[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

[dependencies.your-face]
version = "99999.0.0"
features = ["eyes"]

Possible Solution(s)

There are two problems

  • We drop the existing formatting
  • We explicitly call table.fmt() which deletes all decor

The reason for the latter is so the trailing space in inline tables is removed when an item is no longer the last.

Notes

No response

Version

No response

@marcospb19
Copy link

marcospb19 commented Jul 15, 2023

@epage would the solution be to just remove the table.fmt() call and manually traverse toml_edit items to remove trailing spaces?

@weihanglo
Copy link
Member

@marcospb19 I am not sure if this is that easy but definitely something people can experiment

epage added a commit to epage/cargo that referenced this issue Oct 17, 2023
Huh, overlooked the cargo side when I fixed this in killercup/cargo-edit#725

Fixes rust-lang#10850
epage added a commit to epage/cargo that referenced this issue Oct 18, 2023
Huh, overlooked the cargo side when I fixed this in killercup/cargo-edit#725

Fixes rust-lang#10850
bors added a commit that referenced this issue Oct 19, 2023
fix(add): Preserve more comments

### What does this PR try to resolve?

This was fixed in killercup/cargo-edit#725 and I forgot to carry it over to here.

I kept in some auto-formatting because there is little to preserve until the TOML 1.1 spec is out which will allow mult-line inline-tables which means there might be comments interspersed.  We'll see which comes first, `cargo fmt` support for `Cargo.toml` or the 1.1 spec.

Fixes #10850

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

First commit adds a test demonstrating the problem.  The second makes a lot of noise that the third cleans up, so its a mixed bag as to which commit to look at.

### Additional information
@bors bors closed this as completed in 99e4847 Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants