Skip to content

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

Closed
3 tasks done
anthonykim1 opened this issue Apr 25, 2025 · 12 comments
Closed
3 tasks done

Test: Terminal Shell Environment Proposed API (Scoped-down) #247453

anthonykim1 opened this issue Apr 25, 2025 · 12 comments

Comments

@anthonykim1
Copy link
Contributor

anthonykim1 commented Apr 25, 2025

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

  1. Make sure to enable proposed api terminalShellEnv on extension/code of your choice
  2. Bring/Create extension of your choice where you can subscribe for terminal.shellIntegration event. (onDidChangeTerminalShellIntegration)
  3. Make sure you have shell integration setting enabled: terminal.integrated.shellIntegration.enabled

Steps to Test:

For MacOS and Linux:

  • Test for: pwsh, zsh, bash, fish

For Windows

  • Test for: pwsh, gitbash.
  1. Make sure you are listening to window.onDidChangeTerminalShellIntegration.
  2. The new proposed API and env (environment variables object) should be available via terminal.shellIntegration.env. Note there is two field for env, which are:
    • terminal.shellIntegration.env.value should get you all of the environment variables
    • terminal.shellIntegration.env.isTrusted(This should be true for pwsh, bash, zsh, and false for fish)
  3. Create terminal, watch environment variables. Try putting some env vars in your shell init scripts and see if they show up in the list of environment variables.
  4. Try updating some of the environment variable (pre-defined, hardcoded) listed, such as HOME, or PATH, and see if you get the updated env information.
  5. This API is enabled by true in default in insiders via setting terminal.integrated.shellIntegration.environmentReporting Try turning this off to see that you dont get the env object.
  6. IMPORTANT STEP: Compare the terminal startup & prompt speediness performance by comparing VS Code Stable with environment reporting true with latest insider with environment reporting true.
  7. Make sure terminal startup and prompt speediness (how fast it takes for next prompt to show up when you type something like 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.
@bhavyaus
Copy link
Collaborator

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.

@lszomoru
Copy link
Member

@anthonykim1, I have confirmed that the "hardcoded variables" are working as expected on all the terminals that have been listed for macOS.

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.

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?

@lszomoru lszomoru removed their assignment Apr 29, 2025
@vinistock
Copy link
Contributor

@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/feedback

I understand VS Code is going with a callback approach for performance reasons. When do window.onDidChangeTerminalShellIntegration callbacks get triggered exactly?

  1. Is it possible to receive one of these upon initialization of the extension so that we can read the environment ahead of booting the language server?

For now, you will only see pre-defined hard-coded list of env var such as PATH, HOME, etc.

  1. Are you planning on allowing for other keys?

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 🙏

@anthonykim1
Copy link
Contributor Author

So with that I was not able to verify that things are working as expected with the user-defined environment variables.

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!

@anthonykim1
Copy link
Contributor Author

anthonykim1 commented Apr 29, 2025

, and while both the Insiders and Stable versions had a slight but noticeable lag when typing, their performance seemed about the same.

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 echo hi then waiting for next prompt to show up in the terminal.)

How long it took you to "wait" until the next prompt show up would be the measurement point.
Also how long it takes to open a fresh new terminal with the environment reporting setting.

@anthonykim1
Copy link
Contributor Author

anthonykim1 commented Apr 29, 2025

@vinistock Hello and thanks for testing this.

When do window.onDidChangeTerminalShellIntegration callbacks get triggered exactly?

Mainly when terminal shell integration is activated for user's given terminal. See here

There are couple things that will fire this onDidChangeTerminalShellIntegration event such as when cwd change, and ofc the shell environment stuff that is mentioned here.

It is very possible you would get series of onDidChangeTerminalShellIntegration event as soon as you launch a new VS Code terminal (with shell integration on), and not have the env values show up until the later of those series of event.

Is it possible to receive one of these upon initialization of the extension so that we can read the environment ahead of booting the language server?

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 env.value from the listener, you could potentially reboot language server or part of it with the context you got from user's shell env information.

You would also have to make sureonDidChangeTerminalShellIntegration was fired with shell env event by checking the terminal.shellIntegration.env.value because there are multiple things that can fire onDidChangeTerminalShellIntegration

Are you planning on allowing for other keys?

Yes! The idea is that we would let user/extension define the environment variables they are interested in.
It's great that you brought this up, for Python it was VIRTUAL_ENV, and I'm sure other extensions would have growing lists of env vars they are interested in.

I'll make sure to ping you when we eventually allow this proper subscription.

@hawkticehurst
Copy link
Member

hawkticehurst commented Apr 29, 2025

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.

@hawkticehurst hawkticehurst removed their assignment Apr 29, 2025
@vinistock
Copy link
Contributor

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.

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.

@anthonykim1
Copy link
Contributor Author

anthonykim1 commented Apr 30, 2025

@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 process.env from node itself. This way you won't need to wait for all the terminal shell integration stuff.

Are

GEM_HOME, GEM_PATH, BUNDLE_PATH.

something that lives inside shell init files? (such as .zshrc, .bashrc, etc)

@vinistock
Copy link
Contributor

Yeah, using process.env only works if you launch VS Code from the terminal. If you launch it from an icon or from a search (like spotlight on MacOS), then those variables won't be set because they're all set in the shell only.

This was a constant source of confusion for users until we implemented the environment manager integrations.

something that lives inside shell init files? (such as .zshrc, .bashrc, etc)

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.: .ruby-version, .mise.toml, ~/.bundle/config) and shell scripts that live in ~/.zshrc, ~/.bashrc, ~/.config/fish/config.fish.

@anthonykim1
Copy link
Contributor Author

anthonykim1 commented Apr 30, 2025

Yeah, using process.env only works if you launch VS Code from the terminal. If you launch it from an icon or from a search (like spotlight on MacOS), then those variables won't be set because they're all set in the shell only.

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 env (assuming you are using zsh for instance)?
Particularly for when you would launch from icon or search, rather than starting VS Code from external terminal.

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.

@vinistock
Copy link
Contributor

vinistock commented May 1, 2025

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 env (assuming you are using zsh for instance)?

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.

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants