-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Expose shell's environment - bash #237602
Conversation
Co-authored-by: Daniel Imms <Tyriar@users.noreply.github.com>
Co-authored-by: Daniel Imms <Tyriar@users.noreply.github.com>
…ils for bash Co-authored-by: Daniel Imms <Tyriar@users.noreply.github.com>
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
…ion.ps1 Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
…ion-rc.zsh Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
…ion-rc.zsh Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
extensions/vscode-api-tests/src/singlefolder-tests/terminal.shellIntegration.test.ts
Outdated
Show resolved
Hide resolved
src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts
Outdated
Show resolved
Hide resolved
@anthonykim1 updating branch should hopefully simplify the commit list since the pwsh PR was merged. |
if (!isTrusted) { | ||
return; | ||
} | ||
this._env.clear(); |
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.
Don't need to clear anymore
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.
👍
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.
I removed them here:
but would we ever have to worry about a case where we start startEnvironmentSingleVar
but User requests to access terminal.shellIntegration.env
before we go into setEnvironmentSingleVar
and endEnvironmentSingleVar
?
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.
If that happens I would expect the old state, rather than a partially complete new state
src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts
Outdated
Show resolved
Hide resolved
src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts
Outdated
Show resolved
Hide resolved
…Capability.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
…Capability.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts
Outdated
Show resolved
Hide resolved
This PR originates from a fork. If the changes appear safe, you can trigger the pipeline by commenting |
Hi, the recents builds are failing in the integration tests. I traced the failure to this PR. I will revert this PR so we can release Insiders today, but feel free to rework the PR and merge it again once the issue is resolved. |
This reverts commit e6805d7.
@anthonykim1 see #237784 for the build that failed. |
Trying to break #228791 down smaller so that it is easier to review.
Part of #227467