Skip to content

terraform-fmt updates files, optionally skipped via flags - #53

Open
tyron wants to merge 3 commits into
gruntwork-io:masterfrom
tyron:restore-fmt-behaviour
Open

terraform-fmt updates files, optionally skipped via flags#53
tyron wants to merge 3 commits into
gruntwork-io:masterfrom
tyron:restore-fmt-behaviour

Conversation

@tyron

@tyrontyron commented Aug 9, 2021

Copy link
Copy Markdown

Restore terraform-fmt behaviour from v0.1.11. Fixes#48.

By default, changes are written back to source files as usual for pre-commit hooks, but can be optionally disabled by providing a --no-autofix flag to terraform-fmt.

@tyrontyron changed the title Restore fmt behaviourterraform-fmt updates files, optionally skipped via flagsAug 9, 2021
for file in "$@"; do
terraform fmt -diff -check "$file" || FMT_ERROR=$?
for file in "$FILES"; do
file=$(dirname "$file")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required to be able to run the hook outside the repo folder. For example, pre-commit try-repo /path/to/local/hook terraform-fmt would invoke terraform fmt with files relative to that /path/to/local/hook. By providing the entire dirname, we can use the pre-commit tool for local development easily.

@tyron

Copy link
Copy Markdown
Author

@brikis98 , @yorinasub17 please let me know if this is OK to be reviewed, or if you'd prefer that we don't revert the default behaviour -- rather just expose the flag to switch to the old one.

write_changes=true
FILES=()

parse_arguments() {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspired on shellcheck.sh's version:

parse_arguments() {

@joshmyers

Copy link
Copy Markdown

Any movement on this? Seems a few folks would like old behavior

@mleuthold

Copy link
Copy Markdown

Just my 2cents:

I thought with 1.1.17 this would have been fixed by now. Because it is not, I will still continue using 0.1.11 for auto format files.

@ysawa0

Copy link
Copy Markdown

+1 on this change. Printing the diff is not as useful as auto-correcting the styling.

@ad-m-ss

Copy link
Copy Markdown

Is there anything what I can do to make that change happen?

@ad-m-ss

Copy link
Copy Markdown

If anyone interested that change can be easily tested eg. via:

- repo: https://github.com/gruntwork-io/pre-commit
rev: 837cfa83f27ec2dd4a4c6860b2a6ca9392642fe0
hooks:
- id: terraform-fmt
- id: terraform-validate
exclude: terragrunt/modules

@sanguis

Copy link
Copy Markdown

while this is not complete its better then the current fuctionality

@ayk33

Copy link
Copy Markdown

Can we get some traction on this. The previous functionality is much nicer.

@Fez29

Fez29 commented Jan 4, 2023

Copy link
Copy Markdown

Also waiting on this functionality

@Tlunch

Copy link
Copy Markdown

Would be good to see this functionality

@jannikmi

Copy link
Copy Markdown

I 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_fmt

@dbowling

Copy link
Copy Markdown

Looks like this also fixes #138

As far as I can see, we're just missing code review. Can any maintainer help?

I might suggest that the default be evaluated as it is a change in behavior. Though I personally prefer the default being to auto-fix.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

terraform-fmt hook behaviour change

11 participants

@tyron@joshmyers@mleuthold@ysawa0@ad-m-ss@sanguis@ayk33@Fez29@Tlunch@jannikmi@dbowling