language-formatters-pre-commit-hooks
language-formatters-pre-commit-hooks copied to clipboard
autofix with exit code 0 (if successful)
I would personally favor this way thanks!
@brodybits can I ask you to provide context on why this change is reasonable?
The reason why the tool returns failures if autofix fixed the code is to still signal, to pre-commit hook, that the files needed handling. This way the author will need to re-accept/apply the provided changes in order to be able to commit
I found a couple of these tools (pretty-format-ini & pretty-format-toml) to be useful in a project that is NOT part of a GitHub build, in 2 different places:
- lint checking, where we check that the formatting is OK or not
- auto-formatting with the
--autofix
option, where we would want to continue formatting other files in case we DID have to reformat at one point
I think and hope this should be clear. Yes we can very easily do something like add | true
at the end to ignore the non-zero exit code when auto-formatting. But I think it would be nicer if we can get these updates.
Thanks for your consideration.
Hey is there anything we can do to help move this forward?
I really hope I won't have to fork this to get this resolved.
Thanks in adavance!
The main target for this tool is the integration in pre-commit. Having autofix return exit status 0, will imply that pre-commit will consider the file well formatted and as such the staged changes will be committed.
This will defeat the most important component of pre-commit. This because as after the commit, the format-fixes won't be committed but rather present as unstaged changes on the repository. Essentially, we committed not pretty file and then we should eventually recommit (or commit amend) the fix of format.
For this very reasons I would be opposed to the idea to have exit status 0 in case a fix was placed.
If we were to make this optional via a cli argument (but this needs to be done across all the available tools) then I might consider this, but changing the default is a no go.
Furthermore, you presented a case where you would like to run the tool within lints to format code... I would highly recommend that the developer need to be notified about "automated" changes happened to the files is required (remember automatic formatting should not change the semantic of the code, but bug might exists). (The comment above is very high level as I don't know your environment and as such it might also be not correct).