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

@tyron tyron 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.

@tyron tyron changed the title Restore fmt behaviour terraform-fmt updates files, optionally skipped via flags Aug 9, 2021
Comment thread hooks/terraform-fmt.sh
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

tyron commented Aug 15, 2021

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.

Comment thread hooks/terraform-fmt.sh
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

ysawa0 commented Mar 22, 2022

Copy link
Copy Markdown

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

@ad-m-ss

ad-m-ss commented Mar 31, 2022

Copy link
Copy Markdown

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

@ad-m-ss

ad-m-ss commented Mar 31, 2022

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

sanguis commented Jun 14, 2022

Copy link
Copy Markdown

while this is not complete its better then the current fuctionality

@ayk33

ayk33 commented Sep 26, 2022

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

Tlunch commented Feb 26, 2024

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 free to 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