Expose Terraform-related options via native LSP "InitializationOptions"

[radeksimko]

Current Version

0.5.3

Use-cases

Currently we only expose rootModulePaths in "LSP native" configuration:

terraform-ls/internal/settings/settings.go

Lines 11 to 13 in a1ff20f

	type Options struct {
	// RootModulePaths describes a list of absolute paths to root modules
	RootModulePaths []string `mapstructure:"rootModulePaths"`

Some runtime settings which we don't strictly need to know before the server is actually initialized is still exposed via CLI flags only, e.g. all Terraform-related options:

terraform-ls/commands/serve_command.go

Lines 43 to 47 in a1ff20f

	fs.StringVar(&c.tfExecPath, "tf-exec", "", "path to Terraform binary")
	fs.StringVar(&c.tfExecTimeout, "tf-exec-timeout", "", "Overrides Terraform execution timeout (e.g. 30s)")
	fs.StringVar(&c.tfExecLogPath, "tf-log-file", "", "path to a file for Terraform executions"+
	" to be logged into with support for variables (e.g. Timestamp, Pid, Ppid) via Go template"+
	" syntax {{.VarName}}")

We execute Terraform after the initialization, so these settings would be better suited for the LSP native configuration, which in the long run could also be modified at runtime via workspace/didChangeConfiguration.

Attempted Solutions

Keep using flags

Proposal

We can expose the configuration as terraformExecPath, terraformExecTimeout and terraformLogPath.

terraform-ls/internal/settings/settings.go

Lines 15 to 18 in a1ff20f

	// TODO: Need to check for conflict with CLI flags
	// TerraformExecPath string
	// TerraformExecTimeout time.Duration
	// TerraformLogFilePath string

I wouldn't go as far as removing the existing flags in favour of LSP settings yet, but I would document that as the preferred way of configuring these options and I would only allow one of these ways of configuring any option at a time. i.e. I would error out the initialization if the same settings is provided via flag and LSP.

[danishprakash]

@radeksimko is this open for external first time contributors to this project? I'd like to work on this.

[radeksimko]

Hi @danishprakash
Yes, this one is a good issue for any first time contributor. Feel free to ask questions and/or raise a PR.

[danishprakash]

@radeksimko thanks for the info. I've implemented the changes for TerraformExecPath here. If that approach looks good, I can add TerraformExecTimeout and then raise a PR. Thanks.

PS. the relevant client changes required to test these changes end to end are here.

[radeksimko]

It looks reasonable at first sight. We usually prefer to just raise the PR straight away (at this stage) as we can discuss on the relevant LOC there instead of abstractly pointing to it from here.

[radeksimko]

Reopening until remaining 2 options are added.

[radeksimko]

And I take back what I said about tfExecLogPath - we shouldn't need to run Terraform before initialize request and the recent refactoring done in #588 confirms this, so both options should now be eligible for exposure via LSP.

[danishprakash]

Reopening until remaining 2 options are added.

Sure, will look into those. Thanks for the help!

[github-actions]

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