Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use terminal shell env when resolving commands in path #237588

Draft
wants to merge 54 commits into
base: main
Choose a base branch
from
Draft

Use terminal shell env when resolving commands in path #237588

wants to merge 54 commits into from

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Jan 9, 2025 •
edited
Loading

Blocked on #237415

Fixes #237587

Tested on pwsh/Windows by checking command suggestions:

image

Changing env to a single path and checking again (see vsn.exe):

image

anthonykim1 and others added 30 commits September 16, 2024 22:20
@anthonykim1
start terminal shell env proposed
714bb39
@anthonykim1
fix typo
3c412ff
@anthonykim1
progress on shellEnvDetectionCapability, mainThreadTerminalSI
f214756
@anthonykim1
update IShellEnvDetectionCapability interface
3923373
@anthonykim1
touch up on $shellEnvChange
0465588
@anthonykim1
adjust IShellEnvDetectionCapability
e1d611a
@anthonykim1
properly listen to envChangeEvent
98131e7 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1
Serialize env map, expose on exthost
e37f119
@anthonykim1
Merge remote-tracking branch 'upstream/main' into shellEnvAPI
a0602bc
@anthonykim1
start adding to zsh script
94c6574
@anthonykim1
receive environment variable in extension host, properly escape "
e4ab9e8 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1
clean up
e2b4ca1
@anthonykim1 @Tyriar
Add TODO: properly escape double quotes, figure out why JSON parse fa…
1cf51a5 
…ils for bash

Co-authored-by: Daniel Imms <[email protected]>
@Tyriar
Fix nonce check, ignore PS1 for now in bash
f0b1578
@Tyriar
Add some simple PS1 string tests to deserializeMessage
1b2eb1f
@Tyriar
New approach of sending env entries separately
0956b86
@anthonykim1
be able to get EnvSingleVar
bb735f8
@anthonykim1
few comments
a1503da
@anthonykim1
add function signature for start, set, end environment var
61653d5
@anthonykim1
implement EnvStart, EnvEntry, EnvEnd for single env entry
db981ae
@anthonykim1
deserialize env value for EnvEntry
a25a9a9
@anthonykim1
Remove unncessary comments
7fe7c26
@anthonykim1
only leave pwsh in this PR and exclude other shells
338514c
@anthonykim1
keep exlcuding other shell env - only pwsh should remain
9b97e1b
@anthonykim1 @Tyriar
Update src/vs/workbench/api/common/extHostTerminalShellIntegration.ts
fc9f3f5 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vscode-dts/vscode.proposed.terminalShellEnv.d.ts
bb93fdd 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/contrib/terminal/common/scripts/shellIntegrat…
0ca8a75 
…ion.ps1

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/contrib/terminal/common/scripts/shellIntegrat…
935f14a 
…ion-rc.zsh

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/contrib/terminal/common/scripts/shellIntegrat…
2f7b969 
…ion-rc.zsh

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/api/common/extHostTerminalShellIntegration.ts
cb997d5 
Co-authored-by: Daniel Imms <[email protected]>
anthonykim1 and others added 24 commits January 7, 2025 22:39
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/capabilities/shellEnvDetection…
b6b673f 
…Capability.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/api/common/extHost.protocol.ts
0ab7322 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/api/browser/mainThreadTerminalShellIntegratio…
fe93145 
…n.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/capabilities/shellEnvDetection…
badfb37 
…Capability.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/capabilities/shellEnvDetection…
3f156f9 
…Capability.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/capabilities/shellEnvDetection…
4e8c2cb 
…Capability.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/capabilities/capabilities.ts
333c8d5 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/workbench/api/browser/mainThreadTerminalShellIntegratio…
c68c58a 
…n.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts
944ce9a 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts
b2afe32 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts
dbbe2df 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1
add comment for ShellEnvDetection
0b248e2
@anthonykim1
change envs in shellEnvDetectionCapability to env
d246ba7
@anthonykim1
Mention escaping character for EnvJSON similar to commandLine
6b081da
@anthonykim1
Do not fire env event if env has not changed
d555813
@anthonykim1
add link to CommandLine
3591fa4
@anthonykim1
follow main branch format so I avoid merge conflict
143a4cc
@anthonykim1
Merge branch 'main' into shellEnvPwshOnly
6509fa1
@anthonykim1
remove resolved TODO
03fe326
@anthonykim1 @Tyriar
Update src/vs/workbench/api/browser/mainThreadTerminalShellIntegratio…
d501cee 
…n.ts

Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1 @Tyriar
Update src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts
e94cf4d 
Co-authored-by: Daniel Imms <[email protected]>
@anthonykim1
use vscode object equals
0b831a5
@Tyriar
Merge branch 'main' into shellEnvPwshOnly
5f9f2db
@Tyriar @anthonykim1
Use terminal shell env when resolving commands in path
4cbdc0a 
Fixes #237587

Co-authored-by: Anthony Kim <[email protected]>
@Tyriar Tyriar added this to the January 2025 milestone Jan 9, 2025
@Tyriar Tyriar requested a review from meganrogge January 9, 2025 16:44
@Tyriar
Copy link
Member Author

Tyriar commented Jan 9, 2025

@meganrogge you can see 4cbdc0a for the change this will add

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meganrogge meganrogge meganrogge approved these changes

Assignees

@Tyriar Tyriar

@anthonykim1 anthonykim1

Labels
None yet
Projects
None yet
Milestone
January 2025
Development

Successfully merging this pull request may close these issues.

Terminal Suggest: Use new shell env API to use actual shell PATH variable
3 participants
@Tyriar @meganrogge @anthonykim1