-
Notifications
You must be signed in to change notification settings - Fork 180
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 Terraform version info #1190
Conversation
f9a1cdc
to
763cdcd
Compare
This commit adds a LanguageStatusItem that shows the discovered Terraform version in the current workspace.
763cdcd
to
8b62630
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 👍
I've a few questions and minor suggestions.
terraformStatus.command = { | ||
command: 'terraform.commands', | ||
title: 'Terraform Commands', | ||
tooltip: 'foo', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
foo
?
ExecuteCommandParams, | ||
ExecuteCommandRequest, | ||
LanguageClient, | ||
WorkDoneProgress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WorkDoneProgress, |
} | ||
|
||
public dispose(): void { | ||
this.disposables.forEach((d: vscode.Disposable) => d.dispose()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to remove the disposable after disposing it?
terraformStatus.name = 'Terraform'; | ||
terraformStatus.detail = 'Terraform'; | ||
terraformStatus.command = { | ||
command: 'terraform.commands', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this command do? Do we need to register it?
const moduleDir = Utils.dirname(moduleUri); | ||
|
||
const response = await terraformVersion(moduleDir.toString(), client, reporter); | ||
tfStatus.setTerraformVersion(response.discovered_version); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is the place where I would expect the modification of the status bar item. Most of the other functions in this file make an API call and return the result.
Maybe this line belongs in the TerraformVersionFeature
?
This was implemented as part of #1325 |
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 LanguageStatusItem that shows the discovered Terraform version in the current workspace. This utilizes the LSP request mechanism to automatically show the version when it is discovered for the currently open document.
This requires an open document and does not attempt to query for a folder because LanguageStatusItems only show for open documents.
Requires hashicorp/terraform-ls#1016