-
Notifications
You must be signed in to change notification settings - Fork 32.6k
Test: Terminal Shell Environment Proposed API (Scoped-down) #247453
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
Comments
It worked well on Windows. In terms of speed, I tested on a Windows dev box, and while both the Insiders and Stable versions had a slight but noticeable lag when typing, their performance seemed about the same. |
@anthonykim1, I have confirmed that the "hardcoded variables" are working as expected on all the terminals that have been listed for macOS.
Read the TPI multiple times but it is not clear to me how the user would be able to specify which environment variables would be tracked. So with that I was not able to verify that things are working as expected with the user-defined environment variables. Defining an environment variable in a shell init script did not help as that did not show up in the object returned by the event. Is that expected or a bug? |
@anthonykim1 I tested this API because I believe it can be extremely useful for the Ruby LSP and other extensions, so I wanted to share some feedback. For context, our language server is written in Ruby itself, which is an interpreted language. One of the pain points for users is the integration with environment managers. We ended up rolling out a bunch of custom integrations for each tool, but it's not as robust as we would like. In addition to the brittleness, users often expect that their shell customizations for their manager will end up being applied in the extension, which of course doesn't happen because we're not really launching their shell. Questions/feedbackI understand VS Code is going with a callback approach for performance reasons. When do
The Ruby LSP has many integrations with the dependencies of the user's project. We would need to be able to fetch the values for variables, some with known names and others matching patterns. Some variables are always known, like GEM_HOME, GEM_PATH, BUNDLE_PATH. Others can be added via customization. This is often done for tokens that allow you to install private dependencies (e.g.: BUNDLE___SOME_TOKEN). To be able to switch to this API and get rid of our custom environment integrations, we would need to be able to read environment variables matching a pattern (or all of them) during boot - in addition to subscribing to changes. And this would be useful for any extension that has to launch an executable for an interpreted language. Some other Ruby examples are but maybe even the Python extension needs to find the interpreter too. cc @isidorn who pointed out this issue for us 🙏 |
This is correct/expected for now. But we do plan to let user or extension define which environment variable to track for in the future! |
It might be bit harder to tell on dev box, but I think you would notice difference when executing something into the prompt (such as How long it took you to "wait" until the next prompt show up would be the measurement point. |
@vinistock Hello and thanks for testing this.
Mainly when terminal shell integration is activated for user's given terminal. See here There are couple things that will fire this It is very possible you would get series of
I would say register a listener as soon as your extension activates, but I wouldn't block language server activation until this event gets fired. It may take a while for shell integration to be activated, and also for these env stuff to get propagated from shell integration scripts to VS Code side, and eventually to extensions, I would set it up more dynamically where if you get the You would also have to make sure
Yes! The idea is that we would let user/extension define the environment variables they are interested in. I'll make sure to ping you when we eventually allow this proper subscription. |
Worked well on Linux! I also ran into the issue of not being able to see user defined environment variables, but it sounds like that capability will be coming later based on comments in this thread. Re: performance. There was no difference in new terminal start up time that I could perceive between stable and insiders. There was maybeee a slight difference in speed until the next prompt showed up (insiders being slightly faster), but it was so so small I'm not 100% sure it was actually a thing or not. Note: I use an immutable linux distro which means my ability to install packages the "typical" way is a bit limited and I wasn't able to install / test powershell. |
The problem is that then we would still need to keep all of our custom integrations for the initial launch to work. My hopes were to get rid of all of that code and rely on this new API, both reducing our maintenance cost and improving reliability. Is there a way this API can be designed where we can get the shell environment for the current workspace upon extension activation with acceptable performance? Honestly, even if it takes a few extra seconds during activation, we can easily show a progress notification explaining why we're waiting. |
@vinistock I'm starting to wonder if you are really interested in shell environment for user's terminal instance or environment variables related to user's VS Code workspace. If it is the latter, I think you can just use Are
something that lives inside shell init files? (such as .zshrc, .bashrc, etc) |
Yeah, using This was a constant source of confusion for users until we implemented the environment manager integrations.
TL;DR: yes. Most version/env managers for Ruby use shell hooks to automatically activate environment variables for the Ruby version being used by the workspace. And there are several layers of configurations, customizations and slightly different ways in which each one of the tools work. It's usually a mix of dedicated configuration files (e.g.: |
Right, yeah. Aside from the API, were you able to get/see all the environment variables you need from the VS Code integrated terminal if you were to type If yes, I think this terminal shell env api could help you get contents of those env var accessible inside terminal shell, since it also covers things inside shell init files as well. It would also help for the configs with shell hooks, but I don't think it would be useful for ones without shell hooks + inside some language specific env config files. |
Yes, from inside the integrated terminal, all variables are available - regardless of whether you open the editor via icon or from the shell. And to make it work I tested it out for bash, zsh and fish.
The majority of environment managers for Ruby will read their specialized configuration files automatically in their shell hooks. If we have a way of reading what is the environment in the shell, I'm pretty confident that we'll have an accurate representation of the real environment the user has for their workspace. And by the way, the callback approach is still extremely useful for us. We had reports of people manually changing the Ruby version they use in their shells, but the Ruby LSP doesn't have a way to listen for those changes at the moment and auto restart. So the callback API is definitely useful. We just also need a way to know the environment on activation. Just throwing an idea in the bucket here, but to alleviate the performance issues, could VS Code automatically cache the previously detected shell environment for workspaces (not every FS path, just workspaces opened in the editor)? That way, we could pay the price once and the cache could update based on the new callback. |
Uh oh!
There was an error while loading. Please reload this page.
Refs: #227467
Complexity: 5
Create Issue
Thanks in advance for testing.
I've rated this as complexity 5 due to having to cover multiple shells, but should not be technically challenging to test.
Background Information
This is an updated version from: #241722.
We scoped down the shell env api for performance reason.
Instead of sending and caching environment information for all of the user's environment variable, we are going towards "subscription" way where user or extensions would be able to define few environment variable that we track.
For now, you will only see pre-defined hard-coded list of env var such as PATH, HOME, etc.
Setup
terminalShellEnv
on extension/code of your choiceterminal.shellIntegration
event. (onDidChangeTerminalShellIntegration)terminal.integrated.shellIntegration.enabled
Steps to Test:
For MacOS and Linux:
For Windows
window.onDidChangeTerminalShellIntegration
.terminal.shellIntegration.env
. Note there is two field forenv
, which are:terminal.shellIntegration.env.value
should get you all of the environment variablesterminal.shellIntegration.env.isTrusted
(This should be true for pwsh, bash, zsh, and false for fish)terminal.integrated.shellIntegration.environmentReporting
Try turning this off to see that you dont get theenv
object.echo hi
) on insider vs. stable (make sure you go set env reporting setting to true on both). Make sure its snappier/faster on insiders.The text was updated successfully, but these errors were encountered: