internal: expose tf related options via init opts

[danishprakash]

Closes #240

[hashicorp-cla]

All committers have signed the CLA.

[radeksimko]

Looks like the tests are failing - can you take a look at the results and make these pass again?

You can also run tests locally via go test ./....

Thanks.

[danishprakash]

Sure. It looks like new contributors can't rerun tests on subsequent pushes, can you help out here? Thanks

[danishprakash]

@radeksimko does it make sense to abstract out this validation for paths to a separate package? something like fileutil or maybe rename the existing pathcmp package to path and have such utilities moved there?

[radeksimko]

This looks decent, thank you for the PR. Aside from my inline comments:

Do you mind also adding a note to the CLI option, so that users are aware that we are deprecating it in favour of terraformExecPath LSP config option?

I would add a brief note to the flag help itself here

terraform-ls/internal/cmd/serve_command.go

Line 45 in 9acdc67

fs.StringVar(&c.tfExecPath, "tf-exec", "", "path to Terraform binary")

and then also log a warning (just a message prepended with [WARN]) after here

terraform-ls/internal/cmd/serve_command.go

Line 142 in 9acdc67

logger.Printf("Terraform exec path set to %q", path)

---

Down the line, after this is released and clients had some chance to reflect this we can also start sending a warning message but I wouldn't do that yet. I'd just create a separate issue to revisit later.

[radeksimko]

LGTM. I will remove that redundant error check as mentioned in-line and make some minor adjustments of the comment phrasing before merging - I hope you don't mind.

[radeksimko]

Actually on a second look I'm afraid this won't work because all the methods (formatting, didSave, executeCommand) which are supposed to consume the LSP-configured exec path receive execOpts in their own context, but the initialize handler never writes into it. Instead they will just all always fall back to auto-discovered Terraform exec path - that is not desirable.

[radeksimko]

I'm afraid this is going to be much more involved than expected - we need the exec path pretty early in the lifecycle for walker and module manager, which are both currently created before the initialize handler is ever executed.

I will take this over.

[radeksimko]

Thanks for the PR 👍🏻 I wish I realized the complexity earlier, I did some refactoring to address the mentioned problem and also added some docs, so this is now ready to be merged.

[danishprakash]

Thanks for reviewing this PR, will go through the refactoring that you've done before making other changes.

[github-actions]

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 issues.
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.