Repository navigation
terraform-fmt hook behaviour change #48
Description
Activity
A PR to support some sort of flag that enables the old behavior (overwriting the files) is welcome!
@brikis98 do you think the following can solve the problem rather than a flag:
terraform-fmt.sh can run terraform fmt
terraform-diff.sh can run terraform fmt -diff -checkReacted by Karthic Hariharan, Simon Hulme, Josh Myers and JulesClauI would agree the changes to format in v0.1.12 don't make sense.
If I wanted to know all the errors I would have run lint or validate on the files first. IMHO running format should just auto-correcting issues as the formatter deems fit. It should fail on invalid syntax or logic issues immediately so they can be fixed and need to be re-run, hence why you should validate first if you need to. I don't care about issues it finds regarding spacing, etc that is why I am using the auto-formatter.
Also the diff may matter from the perspective of debugging a CI or a dry-run perspective, but for the default it doesn't. Changes should happen immediately on disk.
I would propose the default behaviour be restored and if you don't want changes made on disk a
--dry-runflag be added to the arguments for the hook.Reacted by Simon Hulme, CaseyLabs, Christian Saiki, Matti Bragge, Ritesh Kumar and lewis-jackson-bots+1 for restoring previous functionality
Reacted by CaseyLabs, Christian Saiki, Martin Leuthold, Tyron, mog, Topher Brown, Josh Myers, Matti Bragge, Ahmed Kamel, Sam Adams and 2 moreYeah at the moment the way to go is to use v0.1.11 in the meantime.
Maybe in the future I can try to implement @skoblenick approach if you guys agree.@brikis98 I filed a PR that allows for both options via a flag (
--no-autofix). I reverted the default behaviour to what used to be in place in v0.1.11 as I thought this is more in-line with other pre-commit hooks (e.g. from pre-commit/pre-commit-hooks:trailing-whitespace,sort-simple-yaml,end-of-file-fixer).Obviously we can invert the logic pretty easily if needed, but I'd like to hear your thoughts first!
- added 2 commits that reference this issue
on Jan 27, 2022 is this going to be released any time soon? 0.1.17 still does not actually modify the files?
Reacted by Tobiasz Kędzierski, Alessandro Cunsolo, Leonid Zaikin, Gareth Fraser, Hossein Rasi, Thomas Bergmann, Roman, Sanjeev kumar Badrinath, Sam Adams, Rodolphe and 8 moreIs there any further thoughts on this behavior being added even as an optional argument?
Reacted by Calum Lynch, Peter Naugle Sherman and Carol NaranjoI found this functionality in another repository:
https://github.com/antonbabenko/pre-commit-terraform?tab=readme-ov-file#terraform_fmt- repo: https://github.com/antonbabenko/pre-commit-terraform rev: v1.92.2 hooks: - id: terraform_fmtReacted by Diogo Diasridiculous negligence on gruntwork's part ¯_(ツ)_/¯
Reacted by lewis-jackson-bots
Previously the terraform-fmt hook made changes to the files, whereas now it only shows the difference and errors out. I did some digging and found that it was an intentional change as part of this PR: #46
It would be nice to have the old behaviour as an option. Generally my terraform files are syntactically valid, but incorrectly formatted, the previous behaviour took away the pain point of needing to run
terraform fmtafter every change. The differences don't need to be analysed, so there's no need for the--diff --check.Thanks for providing these hooks, they've been super useful!