Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Packer #4192

Merged
merged 6 commits into from
May 16, 2022
Merged

Add support for Packer #4192

merged 6 commits into from
May 16, 2022

Conversation

wzyboy
Copy link
Contributor

@wzyboy wzyboy commented May 10, 2022

This PR adds basic fixer support for Packer.

Terraform and Packer are both developed by HashiCorp and share the same HCL syntax. However, terraform fmt and packer fmt cannot be used inter-changably.

I have no experience with VimL, so I just copied the terraform fixer (and its test) into packer variant and it seems to work fine and all tests passed.

Please feel free to edit the PR if I made n00b mistakes on VimL.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Looks good but needs to also update documentation:

  1. Add an entry to doc/ale-hcl.txt for packer fixer.
  2. Add an entry to supported-tools.md file.
  3. Add an entry to ale-supported-languages-and-tools.txt
  4. Add an entry to section 7.6 in ale.txt

Note that in doc/ale-hcl.txt the terraform-fmt entry is just a link to the doc/ale-terraform.txt entry. For Packer you do not want a link, instead you want to add the documentation itself.

Also ensure the entries on all these files are alphabetically ordered. packer-fmt comes before terraform-fmt on all the documents.

@wzyboy wzyboy requested a review from hsanson May 11, 2022 17:52
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Sorry, I believe my last comment should have been more details. My understanding is that Packer is a tool to format HCL andthat HCL is the file type. If this is the case then:

  1. The doc/ale-packer.txt file in unnecessary. The contents you added in there should be directly inside the doc/ale-hcl.txt file. These doc/ale-xxx.txt files are per file type (e..g HCL) not per tool. Simply remove the doc/ale-packer.txt file and add the contents to the ale-hcl-packer-fmt section in doc/ale-hcl.txt
  2. Update the index files by moving the packer entry belo the hcl entry and point them to ale-hcl-packer-fmt.

@wzyboy
Copy link
Contributor Author

wzyboy commented May 15, 2022

@hsanson Hi, the relation between HCL and Packer is as follows:

  1. Terraform uses HCL with file extensions .tf, .tfvars, and .hcl;
  2. Packer uses HCL with file extensions .pkr.hcl and .pkrvars.hcl;
  3. The terraform fmt and packer fmt commands can only work on their own files.

IMHO Packer deserves its own format entry because only packer fmt can format Packer's HCL. You cannot format Packer's HCL with terraform fmt, and vice versa. On the other hand, Terraform and Packer both call their DSL "HCL" so it makes sense to mention the two tools in ale/doc-hcl.txt.

@wzyboy wzyboy requested a review from hsanson May 15, 2022 02:27
@hsanson
Copy link
Contributor

hsanson commented May 15, 2022

I see, thanks for the explanation. If packer is its own file type then documentation is ok as it is and I would recommend to add "packer" in the suggested fixer registry and remove "hcl':

'suggested_filetypes': ['packer'],

This way the fixer wont trigger for all hcl files, only for packer files. I would also remove hcl from the suggested_filetypes for terraform. If not the terraform firxer can be triggered for packer files with hcl extension.

Also please add this to you ale_linters configuration so Vim warns you about linter issues that are failing now:

let g:ale_linters = {
  \   'vim': ['vint', 'ale_custom_linting_rules'],
\ }

@wzyboy
Copy link
Contributor Author

wzyboy commented May 15, 2022

I agree with you that packer ft should be in the suggested_filetypes of packer tool. However, I believe suggesting both packer and terraform tools for hcl filetype makes sense, because:

  1. As I mentioned above, the two tools share the HCL syntax, for some files (e.g. variables ), the formats are identical. It would be hard (if not impossible) to tell them apart. It's just like for ft=yaml, ALE also suggests circleci.
  2. Exsting tools already associate *.hcl files with ft=hcl. So I won't remove hcl from the suggested file types of terraform tool.

As of the linting errors, I cannot determine which part is wrong. I already have vint in my $PATH and also tried :ALEFix align_help_tags before submitting the PR. Could you help me determine why CI is failing?

@hsanson
Copy link
Contributor

hsanson commented May 15, 2022

You need to add packer-fmt under the HCL section on all index files to make the linter happy:

diff --git a/doc/ale-supported-languages-and-tools.txt b/doc/ale-supported-languages-and-tools.txt
index 6852e80a..047020bf 100644
--- a/doc/ale-supported-languages-and-tools.txt
+++ b/doc/ale-supported-languages-and-tools.txt
@@ -239,6 +239,7 @@ Notes:
   * `stack-ghc`
   * `stylish-haskell`
 * HCL
+  * `packer-fmt`
   * `terraform-fmt`
 * HTML
   * `VSCode HTML language server`
diff --git a/doc/ale.txt b/doc/ale.txt
index 8e73ac2b..db646377 100644
--- a/doc/ale.txt
+++ b/doc/ale.txt
@@ -2909,6 +2909,7 @@ documented in additional help files.
     hie...................................|ale-haskell-hie|
     ormolu................................|ale-haskell-ormolu|
   hcl.....................................|ale-hcl-options|
+    packer-fmt............................|ale-hcl-packer-fmt|
     terraform-fmt.........................|ale-hcl-terraform-fmt|
   help....................................|ale-help-options|
     cspell................................|ale-help-cspell|
diff --git a/supported-tools.md b/supported-tools.md
index cc30fc61..f3f3a117 100644
--- a/supported-tools.md
+++ b/supported-tools.md
@@ -248,6 +248,7 @@ formatting.
   * [stack-ghc](https://haskellstack.org/)
   * [stylish-haskell](https://github.com/jaspervdj/stylish-haskell)
 * HCL
+  * [packer-fmt](https://github.com/hashicorp/packer)
   * [terraform-fmt](https://github.com/hashicorp/terraform)
 * HTML
   * [VSCode HTML language server](https://github.com/hrsh7th/vscode-langservers-extracted)

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Great, thanks for all the changes. Looking good.

@hsanson hsanson merged commit 429f5a1 into dense-analysis:master May 16, 2022
wzyboy added a commit to wzyboy/vimrc that referenced this pull request May 17, 2022
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add support for HashiCorp Packer

* Add test for packer fmt

* Add doc for HCL/Packer

* Add link to Packer doc

* Also suggest packer fix for packer ft

* Add more links to TOC
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add support for HashiCorp Packer

* Add test for packer fmt

* Add doc for HCL/Packer

* Add link to Packer doc

* Also suggest packer fix for packer ft

* Add more links to TOC
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.

None yet

2 participants