Skip to content

Consolidate ps1 shell integration vars into __VSCodeState #251114

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 2 commits into from
Jun 11, 2025

Conversation

anthonykim1
Copy link
Contributor

@anthonykim1 anthonykim1 commented Jun 10, 2025

Resolves: #250000
/cc @mattcargile

@anthonykim1 anthonykim1 self-assigned this Jun 10, 2025
@anthonykim1 anthonykim1 added bug Issue identified by VS Code Team member as probable bug terminal-shell-pwsh An issue in the terminal specific to PowerShell, including shell integration labels Jun 10, 2025
@anthonykim1 anthonykim1 requested review from Tyriar and meganrogge June 10, 2025 19:32
@anthonykim1
Copy link
Contributor Author

Can you guys give this a try?

I've tried on pwsh Mac, and shell integration seem fine. (Tried with starship):
Screenshot 2025-06-10 at 12 27 44 PM

This is the difference between the old way (with many variables vieweable when typing Get-Variable) in pwsh vs. the improved version:

Screenshot 2025-06-10 at 12 34 59 PM

I think we can ignore PSScriptRoot and PSCommandPath. After I modify shellIntegration.ps1 script, it just seem to point at my script for some reason.

@anthonykim1 anthonykim1 marked this pull request as ready for review June 10, 2025 19:38
@vs-code-engineering vs-code-engineering bot added this to the June 2025 milestone Jun 10, 2025
Copy link
Contributor

@meganrogge meganrogge left a comment

Choose a reason for hiding this comment

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

LGTM and works on mac. I think we should test on Windows before merging.

@anthonykim1
Copy link
Contributor Author

Just tried on parallels, and I think we are fine.
Screenshot 2025-06-11 at 12 48 59 PM

@anthonykim1 anthonykim1 merged commit 768477e into main Jun 11, 2025
8 checks passed
@anthonykim1 anthonykim1 deleted the anthonykim1/shellIntGlobalLeak branch June 11, 2025 19:50
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jul 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug terminal-shell-pwsh An issue in the terminal specific to PowerShell, including shell integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shellIntegration.ps1 Leaks Variables To Global Scope
2 participants