-
Notifications
You must be signed in to change notification settings - Fork 144
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
vscode/asdf: support configuration environment variables #2029
Conversation
This adds support for detecting asdf configuration via its environment variables: * ASDF_DIR: installation location * ASDF_DATA_DIR: data directory
I have signed the CLA! |
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.
Thank you for the contribution!
This approach would only work if the user opens VS Code through the terminal so that it can inherit the environment of the current shell.
Otherwise, VS Code's environment wouldn't include these environment variables - especially if they are set using shell scripts (which are not sourced automatically by the VS Code process, they are only source by the terminal).
If ASDF_DIR
and ASDF_DATA_DIR
can be override and that's a common thing to do, then the answer is to provide new settings so that users can configure that themselves.
Here's an example of how to do it #1976.
@vinistock Thanks for the feedback. FYI, a recent update broke this customization of the I presume that it was this commit: 30bc0001. I'm simply comparing the timeframe of the commit and the functionality change. (I use Ruby LSP every day.) |
@vinistock Shouldn't the shell be able to determine the values of the |
This removes the detection of the asdf configuration directories for setting the associated environment variables during activation: * ASDF_DIR: install location * ASDF_DATA_DIR: data directory After the activation script is run, the resulting environment variables are inspected for existence of those above. If not set, the directories are detected automatically.
We have decoupled all version manager integrations from shells. I explained in more detail here. The summary is that trying to automatically discover the user's shell and source their configurations has lead to several problems and it's generally a brittle approach. The NodeJS process where extensions run is not the same as your shell/terminal. Nothing you configure in shell scripts is available there. We're trying to push forward version manager integrations that do not connect to the shell in any way, so that we can achieve more reliable environment activation. Compared to other version managers, ASDF is proving to be extremely coupled with shells (multiple users have raised issues). It seems that when we source the If you have better ideas on how to make the integration smoother, please do tell us. |
@vinistock I pushed an update with an alternative approach. I'm not sure this will work either based on the complexities you mentioned, but I'm curious on your insights. |
I created an alternative and simpler update which simply includes the XDG base data directory as it is a common pattern: #2045. |
Issue fixed in #2006 |
This adds support for detecting asdf configuration via its environment variables:
Motivation
Fixes #2023
Implementation
If the environment variable is set, include it as the first option for where to search to validate the value.
Automated Tests
Manual Tests