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

Format drops comments between items in a table #818

Open
charliermarsh opened this issue Dec 10, 2024 · 1 comment
Open

Format drops comments between items in a table #818

charliermarsh opened this issue Dec 10, 2024 · 1 comment

Comments

@charliermarsh
Copy link

Below, # one and # two are dropped by calling .fmt():

use toml_edit::DocumentMut;

fn main() {
    let toml = r#"
[items]
# one
foo = [1, 2, 3]
# two
bar = [4, 5, 6]
    "#;
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["items"].as_table_mut().map(|t| t.fmt());
    let expected = r#"
[items]
foo = [1, 2, 3]
bar = [4, 5, 6]
    "#;
    assert_eq!(doc.to_string(), expected);
}
@epage
Copy link
Member

epage commented Dec 10, 2024

Yes, fmt is fairly naive in what it does. I'm a bit hesitant about making it much smarter

  • There is a long tail of what that even means and can depend on style guides
  • There is ambiguity about what what a comment is associated with and there is likely no right answer we can provide

Overall, this seems better suited for a heavier weight manifest formatting tool, with related configuration, rather than expecting the TOML parser / formatter to do it all.

Within Cargo, the approach we've taken is

  • If we think its reasonable to sort something and we detect its currently sorted, keep it sorted
  • Insert new content with "good enough" defaults
  • Leave everything else to cargo fmt to eventually handle

The last part is modeled off of cargo fix which applies rustc's suggestions naively and you have to choose to format your code, rather than rustc being aware of your rustfmt configuration or cargo guessing you want rustfmt run.

Even when there isn't a manifest formatter yet, like in Cargo's case, I think its still helpful to be naive about formatting so you don't throwaway formatting from the user which is especially important for users to see the diff of what the machine generated change was.

charliermarsh added a commit to astral-sh/uv that referenced this issue Dec 10, 2024
## Summary

If you look at Ed's reply
[here](toml-rs/toml#818 (comment)),
it sounds like we're being too heavy-handed in applying `.fmt()`. I
think I added this to handle an issue with inline tables whereby we were
inserting a space after a trailing comma? So now I'm just applying
`.fmt()` to inline tables, which don't allow comments between elements
anyway.

Closes #9758.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants