Add terraform version command #1016
Conversation
This commit adds a new `module.terraform` command which returns the required and discovered version of Terraform in the currrent workspace.
jpogran
force-pushed
the
terraform_version_command
branch
from
58c564e to
754eafb
Compare
There was a problem hiding this comment.
Aside from my in-line comments, do you mind adding a test for this new command?
Feel free to look into https://github.com/hashicorp/terraform-ls/blob/main/internal/langserver/handlers/execute_command_module_providers_test.go for some inspiration.
This ticket is targeting hashicorp/vscode-terraform#697, which points to #69 for detecting and tracking when the version changes. Should we tackle both work items in this PR, or just the original ticket? |
|
Good question! So I think for the Terraform version in particular, until #69 is addressed, it won't change, but the version requirements which are parsed from the configuration may change at any time - so for those I think we should send the updates. |
| response.RequiredVersion = mod.Meta.CoreRequirements.String() | ||
| } | ||
|
|
||
| progress.Report(ctx, "Sending response ...") |
There was a problem hiding this comment.
Since this is getting the data from memory, it should be pretty quick (definitely sub-second, low milliseconds in most cases), so I'm not sure there's much value in the progress reporting for this particular command?
There was a problem hiding this comment.
The method is a no-op if a token isn't provided in the request, so this technically doesn't do anything yet.
My thinking along this lines is that we enable progress information everywhere as we go, instead of having to add when there's a problem. Then we can dial back if it's too much.
|
Thank you for adding the test - that looks good - we could possibly deal with the data updates in a separate PR, if you wish. One thing we should still do however is to document this new command here https://github.com/hashicorp/terraform-ls/blob/main/docs/commands.md |
|
Version change notification added and docs updated |
There was a problem hiding this comment.
I'm half wondering if the refresh command should be called refreshModuleTerraform, to indicate that it's related to the module.terraform, which in turn exposes more than just a Terraform version, however I'm also happy to keep it as is while we figure out what exact data and in what exact shape do we want to expose. 😃
👍🏻
| @@ -463,6 +463,10 @@ func (svc *service) configureSessionDependencies(ctx context.Context, cfgOpts *s | |||
| moduleHooks = append(moduleHooks, callRefreshClientCommand(svc.server, commandId)) | |||
| } | |||
|
|
|||
| if commandId, ok := lsp.ExperimentalClientCapabilities(cc.Experimental).RefreshTerraformVersionCommandId(); ok { | |||
| moduleHooks = append(moduleHooks, callRefreshClientCommand(svc.server, commandId)) | |||
| } | |||
There was a problem hiding this comment.
I think in the long run we'll probably want to create a separate hook which only fires when the data exposed through each command changes (not just any data), but this is 🆗 for now, since the other two refresh commands above have the exact same problem.
Co-authored-by: Radek Simko <radek.simko@gmail.com>
|
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. |
This commit adds a new
module.terraformcommand which returns the required and discovered version of Terraform in the currrent workspace.