Terraform Version LanguageStatusItem #1325
Conversation
This uses a StaticFeature to provide a LanguageStatusItem that displays the version of terraform that terraform-ls knows about.
jpogran
force-pushed
the
terraform-version-status
branch
from
d643836
to
831ca5b
Compare
src/features/terraformVersion.ts
Outdated
|
|
||
| try { | ||
| const response = await terraform.terraformVersion(moduleDir.toString(), this.client, this.reporter); | ||
| this.terraformStatus.text = response.discovered_version; |
There was a problem hiding this comment.
What do we want to display if no version was discovered?
There was a problem hiding this comment.
Not sure here. If I don't set it, its an empty space. I could replace with N/A or Not found but it looks odd.
You can't hide a LanguageStatusItem that's been created like you can with a StatusBarItem, so you are left with it there after initialization. The alternative is to not create it unless a version comes back, but that does not account for the scenario when one directory has a terraform binary available to query, and another doesn't.
There was a problem hiding this comment.
I think the combination of what you're proposing may work best, i.e. not create the LanguageStatusItem at all if the version is unknown and display something like unknown if it changes to unknown as part of the refresh.
FWIW unlike the constraint, we don't really re-discover the installed version on LS side at the moment, so the scenario with unknown basically can not happen, for now. hashicorp/terraform-ls#69
There was a problem hiding this comment.
I'll leave review of the implementation details to Daniel, I have just two mostly UX related questions:
- Do we know if there's any convention on the location and content of such a "status item"? Reason I'm asking is because the official Go extension puts the Go version on the left side and it appears as one "item"
They have two additional items in the status bar on the right
I'm not sure if all these things are relevant for us, now or later, and I'm neither sure if other extensions follow what Go does too, just trying to get a sense of "what's common" in VS Code and what and where does the user will expect to see. - Are we intentionally leaving out the discovered Terraform version constraint? I thought in the initial issue that it would be useful to display, but I can see how limited space we're working with, so I'd be fine with leaving it out - just seeking ack that this was intentional rather than accidental. 😃
Yes, there are now two "status item" items: StatusBarItem and LanguageStatusItem. StatusBarItems can be placed anywhere in the status bar, and originally was the only way to indicate status. You could create as many as you wanted and put whatever text you wanted in it, but were encouraged to be a good extension author and not get carried away. LanguageStatusItem is the newer item, and it only appears next to the Language identifier and there is only "one" of them in the sense there is only one The Go extension uses StatusBarItems and chooses to put it to the left for versions and right for updating Go. The placement is a design decision on their part. Use of the left side is discouraged as it's seen as "core functionality", but there is no restriction on placement. I chose LanguageStatusItem because of the standardized and centralized location for many different pieces of information, with it's built in support for status indication and error notification. You can look at the TypeScript and JSON extensions for examples.
Not intentional. While it appears we don't have much "space", we have the ability to add a new LanguageStatusItem for each piece of information, which can show status (error, busy, etc) individually. This is what StatusBarItem can't do: keep multiple sets of information together in a concise and configurable form.
I can't simulate it in the gif, but imagine the author opening the file with the resolved and required versions pinned. The required version shows up, but the resolved version shows an error because the required version is incorrect. The |
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 uses a StaticFeature to provide a LanguageStatusItem that displays the version of terraform that terraform-ls knows about.