Pass the path to `shellIntegration.ps1` directly to the server #4959

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

Jump to bottom

Merged

andyleejordan merged 1 commit into main from shell-integration

Apr 3, 2024

Merged

Pass the path to `shellIntegration.ps1` directly to the server #4959

andyleejordan merged 1 commit into main from shell-integration

Apr 3, 2024

Conversation

andyleejordan

Copy link

Member

@andyleejordan andyleejordan commented Apr 3, 2024 •

edited

Loading

The server can't use code --locate-shell-integration-path pwsh because it doesn't know if it's code or code-insiders etc. Even when given the direct path to the client's host process, it can't reliably use that API either. Mostly this is due to Windows using Code.cmd in the background (which is not Code.exe, the host process) but it's also slower anyway as it requires another launch of Node.js.

We'll have to rely on the path to the script not changing relative to vscode.env.appRoot, but at least that part is a public API.

Required by PowerShell/PowerShellEditorServices#2156.

@andyleejordan andyleejordan requested a review from a team

April 3, 2024 16:54

@andyleejordan andyleejordan added the Issue-Enhancement label

Apr 3, 2024

@andyleejordan andyleejordan requested a review from SeeminglyScience

April 3, 2024 16:54

@andyleejordan


 Pass the path to shellIntegration.ps1 directly to the server

The server can't use `code --locate-shell-integration-path pwsh`
because it doesn't know if it's `code` or `code-insiders` etc.
Even when given the direct path to the client's host process, it can't
reliably use that API either. Mostly this is due to Windows using
`Code.cmd` in the background (which is not `Code.exe`, the host process)
but it's also slower anyway as it requires another launch of Node.js.
We'll have to rely on the path to the script not changing relative to
`vscode.env.appRoot`, but at least that part is a public API.

@andyleejordan andyleejordan force-pushed the shell-integration branch from 5f20f63 to 7703924 Compare

April 3, 2024 18:09

@andyleejordan andyleejordan changed the title ~~(削除) Pass Code's executable path to PowerShell Editor Services (削除ここまで)~~ (追記) Pass the path to shellIntegration.ps1 directly to the server (追記ここまで)

Apr 3, 2024

andyleejordan

andyleejordan commented

Apr 3, 2024

View reviewed changes

src/session.ts

// When Terminal Shell Integration is enabled, we pass the path to the script that the server should execute.

// Passing an empty string implies integration is disabled.

const shellIntegrationEnabled = vscode.workspace.getConfiguration("terminal.integrated.shellIntegration").get<boolean>("enabled");

const shellIntegrationScript = path.join(vscode.env.appRoot, "out", "vs", "workbench", "contrib", "terminal", "browser", "media", "shellIntegration.ps1");

Copy link

Member Author

@andyleejordan andyleejordan Apr 3, 2024

Choose a reason for hiding this comment

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

@Tyriar I'm taking a hard dependency on this relative-to-app-root path. You all have too, but just internally as far as I can tell. Trust me, it was no good going down the road of passing process.argv0 and trying to use code --locate-blah-blah.