Skip to content

Scope down shell env api #245899

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

Merged
merged 13 commits into from
Apr 14, 2025
Merged

Scope down shell env api #245899

merged 13 commits into from
Apr 14, 2025

Conversation

anthonykim1
Copy link
Contributor

@anthonykim1 anthonykim1 commented Apr 7, 2025

Resolves: #245695, #244140

@anthonykim1 anthonykim1 added this to the April 2025 milestone Apr 7, 2025
@anthonykim1 anthonykim1 self-assigned this Apr 7, 2025
@anthonykim1 anthonykim1 requested a review from meganrogge April 10, 2025 22:52
@anthonykim1
Copy link
Contributor Author

For reference, things are 50% faster (both for initial boot start-up of terminal and per each prompt) being scoped down.
Future plan is to have extensions, user specify which env vars they want to subscribe to and keep track of, but for now we have a list of defined env vars for immediate use.

Bash:
bashWhole
vs.
bashPart

Pwsh:
PWSHwhole1
vs.

PWSHpart

Zsh(This is going to be the slowest since parameter expansion in zsh is turning out to be lot more expensive than in other shell!) and yes this is the native parameter expansion from zsh, which are supposed to be lot faster for us than using env or printenv, etc.. :
zshWhole
vs.
zshPart

if (shellLaunchConfig.shellIntegrationEnvironmentReporting) {
// TODO: (Perf) Temporarily pass lists of hardcoded env vars for shell env api
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this describing the change here? Should TODO be removed and should the comment be moved to be above scopedDownShellEnvs?

Also nit, but list vs lists 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally! valid 100%
I took the feedback here => 0a6d37b

@meganrogge
Copy link
Contributor

I see env for all shells except for zsh with these changes.

Screenshot 2025-04-14 at 12 50 28 PM

@meganrogge
Copy link
Contributor

I see that become defined when I hit enter and @anthonykim1 has told me it's expected that it could initially be undefined.

@anthonykim1 anthonykim1 marked this pull request as ready for review April 14, 2025 17:55
@anthonykim1 anthonykim1 merged commit 74b46b9 into main Apr 14, 2025
8 checks passed
@anthonykim1 anthonykim1 deleted the anthonykim1/scopeDownShellEnvApi branch April 14, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Temporarily scope down environment reporting
2 participants