search

macisamuele / language-formatters-pre-commit-hooks

Collection of custom pre-commit hooks.
Apache License 2.0
115 stars 58 forks source link

"Orphan" comments are deleted during TOML formatting #161

Closed maresb closed 1 year ago

maresb commented 1 year ago

The toml-sort library sometimes deletes comments. Specifically, toml-sort has the notion of an "orphan" comment where there are empty newlines between the comment and the subsequent section.

I usually trust pre-commit hooks to not do anything destructive, so this feels like a fairly serious issue. Thus I asked in https://github.com/pappasam/toml-sort/issues/59 if functionality could be added which might prevent this.

macisamuele commented 1 year ago

Closing the issue as the underlying issue would be solved by the toml sort library.

In the meantime, pre-commit should not perform destructive operations and comments are relevant for humans and not altering the machine readable content of the config (which would make me even more concerned).

Don't take me wrong, I'm not suggesting that comments are not important but rather that developers during commit/code-review/pull-request would see the change and have it added back.

tkukushkin commented 10 months ago

@macisamuele What do you think about adding a configuration option that controls whether orphan comments should be deleted or whether sorting should fail with an appropriate error message? In such case, toml-sort wouldn't delete any comments silently and it would be always safe to use it in precommit without reviewing changes by human.

maresb commented 10 months ago

@tkukushkin, there seems to be support from @macisamuele, but the problem is in toml-sort. Someone needs to open a PR there in order to move this forward.

tkukushkin commented 10 months ago

@maresb I accidentally got the repository wrong)