pretty-format-toml removes trailing comments in list

[l0b0]

When reformatting a list setting like

load-plugins = [
  "pylint.extensions.bad_builtin",
  # "pylint.extensions.docparams",
  "pylint.extensions.set_membership"
  # "pylint.extensions.typing"
]

, the first comment is retained, but the second one is removed. Both comments should be retained. Not sure whether this is an issue with TomlSort or this repo.

[maresb]

This looks like a duplicate of #161

[maresb]

I feel like this TOML pre-commit hook needs a big red warning that it's unsafe since it will unceremoniously delete comments.

[Avasam]

I find it'll do so even outside lists. And I have no workaround, making this hook pretty unusable.
It also removes newlines between blocks of comments

[corneliusroemer]

Yes, TOML needs to be fixed or marked unsafe as it removes comments - don't use!

[maresb]

@corneliusroemer I don't see a corresponding PR for the commit linked above. Is one on the way, or did I miss it? Thanks for following through with that!

[corneliusroemer]

@maresb That is for reminding me! I must have forgotten to make a PR. Done now! #242

[maresb]

By the way, if people need a safe TOML pre-commit hook, this just became available a few days ago and it seems to work very well. (The previous version used a cumbersome Docker contaier.):

https://github.com/ComPWA/mirrors-taplo

[macisamuele]

Unfortunately, the hook is not unsafe.
For a formatter unsafe is when the generated output is semantically different than the input.

In the issue a few points were mentioned:

• removal of trailing comma: the semantic of the content is not altered, hence the difference is about rendering
• removal of comment not bound to any item: the semantic of the content is not altered

I could understand the argument that there might be better base library to use, and I would be welcoming PRs in this direction.

[l0b0]

• removal of comment not bound to any item: the semantic of the content is not altered

Semantics isn't only about computers, though. Removing a comment definitely changes the semantics for any human readers.

[Avasam]

If it accidentally dropped comments whilst applying a fix it'd be one thing, since you can add them back and the formatting should be stable after that. But it currently removes certain comments without applying any real changes.

Semantics of "unsafe" aside, the end result is I can't use this hook because it can't keep some comments.

Concerning possible alternatives, as a formatter backend there's https://taplo.tamasfe.dev/ which is what backs the Even Better TOML VSCode extension. They don't have an official pre-commit.ci hook or GitHub action yet (tamasfe/taplo#535, tamasfe/taplo#470 & tamasfe/taplo#326)

Edit: Actually, there's https://github.com/ComPWA/taplo-pre-commit, and for use in GitHub action in a python project, taplo is bundled as a wheel https://pypi.org/project/taplo/

[maresb]

@macisamuele, the issue of toml-sort deleting comments definitely isn't your fault. It's an upstream issue pappasam/toml-sort#59 so I hope you are not taking this personally.

On the other hand, many people (including me) store critical contextual information in comments. I am unfamiliar with any popular formatter (other than toml-sort or minifiers) that deletes comments in its default configuration, although I'm happy to be corrected on this point. Therefore, I hope that you would agree that this hook is "unexpectedly destructive" in the most literal sense possible. It is unexpected in the sense that it is extremely uncommon for a formatter to delete comments and has caught me and all the above commenters offguard. It is destructive in that I often put significant work and information into my comments, they often contain information and TODOs, and the disappearance of this information would eventually likely result in breakage.

As such, I have opened #249 as a follow-up to #242 as neutral statement of facts.