terraform icon indicating copy to clipboard operation
terraform copied to clipboard

command/fmt: support formatting multiple files

Open zimbatm opened this issue 4 years ago • 14 comments

All the code infrastructure was there to support formatting multiple files already.

This makes terraform fmt more flexible and also compliant with the treefmt formatted spec

Fixes https://github.com/numtide/treefmt/issues/97

zimbatm avatar Mar 24 '21 20:03 zimbatm

Codecov Report

Merging #28191 (a648ade) into main (bc38623) will decrease coverage by 1.29%. The diff coverage is 16.66%.

Impacted Files Coverage Δ
command/fmt.go 58.85% <16.66%> (-14.80%) :arrow_down:
terraform/eval_error.go 33.33% <0.00%> (-16.67%) :arrow_down:
terraform/transform_vertex.go 45.45% <0.00%> (-11.69%) :arrow_down:
terraform/transform_local.go 63.63% <0.00%> (-11.37%) :arrow_down:
terraform/node_provisioner.go 71.42% <0.00%> (-10.39%) :arrow_down:
states/statefile/version0.go 33.33% <0.00%> (-9.53%) :arrow_down:
helper/hashcode/hashcode.go 66.66% <0.00%> (-8.34%) :arrow_down:
backend/remote/colorize.go 55.55% <0.00%> (-8.09%) :arrow_down:
plans/plan.go 40.00% <0.00%> (-8.00%) :arrow_down:
registry/response/terraform_provider.go 37.50% <0.00%> (-7.96%) :arrow_down:
... and 654 more

codecov[bot] avatar Mar 24 '21 20:03 codecov[bot]

codecov is complaining because the added doc is untested :)

zimbatm avatar Mar 24 '21 21:03 zimbatm

ping?

zimbatm avatar Apr 10 '21 09:04 zimbatm

Some more context and justification for this PR from my perspective: treefmt allows dramatically speeding up repository formatting by only attempting to format files that have changed. The current interface to terraform fmt, while working great standalone, is too restrictive to support this use-case. This seems like a shame!

mloughran avatar May 07 '21 19:05 mloughran

rebased to fix the merge conflict

zimbatm avatar May 07 '21 20:05 zimbatm

rebased to fix the merge conflicts

zimbatm avatar Jun 24 '21 13:06 zimbatm

Ping @jbardin. It'd be really nice if this could go in!

flokli avatar Nov 08 '21 13:11 flokli

/cc @teamterraform

zimbatm avatar Dec 12 '21 16:12 zimbatm

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Mar 12 '22 17:03 hashicorp-cla

This feature is useful to have!

aldoborrero avatar Mar 17 '22 09:03 aldoborrero

Some more justification for this PR: Currently the only way to run terraform via treefmt is it to use a wrapper script (e.g. see https://github.com/numtide/treefmt/blob/master/treefmt.toml#L77)

Unfortunately, the startup speed of terraform fmt means that running it separately for each file is very slow. On my project (64 terraform files) this is already 23s. Output from treefmt:

[INFO]: #terraform: 64 files processed in 23.24s

Whereas

terraform fmt -recursive  0.48s user 0.90s system 83% cpu 1.659 total

This makes treefmt + terraform just about acceptable for local use cases (hopefully the files don't change much, and treefmt only runs on changed files), but it's a deal-breaker for use cases such as checking formatting in CI.

mloughran avatar Jul 22 '22 11:07 mloughran

Hi @zimbatm, apologies for the late reply. We should be able to review this within a few weeks. Thanks for your patience.

crw avatar Jul 29 '22 21:07 crw

Codecov Report

Merging #28191 (b070abd) into main (21228e1) will increase coverage by 1.46%. The diff coverage is 16.66%.

:exclamation: Current head b070abd differs from pull request most recent head 7a9ccc0. Consider uploading reports for the commit 7a9ccc0 to get more accurate results

Impacted Files Coverage Δ
command/fmt.go 59.78% <16.66%> (-16.61%) :arrow_down:
terraform/graph_builder_eval.go 0.00% <0.00%> (-100.00%) :arrow_down:
terraform/node_provider_eval.go 0.00% <0.00%> (-100.00%) :arrow_down:
terraform/ui_output_callback.go 0.00% <0.00%> (-100.00%) :arrow_down:
repl/format.go 11.42% <0.00%> (-87.63%) :arrow_down:
configs/provider_meta.go 0.00% <0.00%> (-83.34%) :arrow_down:
command/version.go 0.00% <0.00%> (-71.24%) :arrow_down:
internal/getproviders/http_mirror_source.go 0.00% <0.00%> (-68.93%) :arrow_down:
internal/providercache/installer_events.go 42.85% <0.00%> (-57.15%) :arrow_down:
command/console_interactive.go 0.00% <0.00%> (-48.15%) :arrow_down:
... and 776 more

codecov-commenter avatar Aug 07 '22 12:08 codecov-commenter

rebased!

  • The code didn't change
  • I re-did the doc, copying some of the text from the website
  • Renamed PATH... to [target...] to better follow the other commands.
  • Updated the cmd and website descriptions to be more in sync.

zimbatm avatar Aug 07 '22 13:08 zimbatm

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

github-actions[bot] avatar Aug 12 '22 17:08 github-actions[bot]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Sep 13 '22 02:09 github-actions[bot]