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 linterOptions to settings #1738

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wata727
Copy link

@wata727 wata727 commented Jun 23, 2024

Part of #334
Fixes #855

This PR adds linterOptions to the language server settings. This includes the following TFLint configs, which I plan to use in subsequent PRs:

  • options.linters.tflint.path
  • options.linters.tflint.configPath
  • options.linters.tflint.lintOnSave
  • options.linters.tflint.timeout

Please note that this change differs from that proposed in #855 in the following points:

  • The binaryPath has been changed to path to match options.terraform.path.
  • The binaryFlags is not supported. TFLint has multiple flags, and I'm not yet sure if they can be safely passed through terraform-ls.
    • However, since --config is likely to be needed at an early stage, only configPath is supported.
  • The timeout is added. Because TFLint rules can be extended, it is necessary to set a timeout to ensure completion.
  • Just like Terraform, binary path is only validated to make sure it is accessible, not executable.
  • If validation fails, it returns an error instead of silently failing, which is probably better if you set the path explicitly.

Whether these changes are appropriate is open to debate. I would be happy to discuss it here.
If there are any other matters that need to be discussed in advance regarding this issue, please feel free to let me know. Thank you.

@wata727 wata727 requested a review from a team as a code owner June 23, 2024 13:23
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.

Add linter configuration to initialization options
1 participant