Skip to content

Commit

Permalink
src/goEnvironmentStatus: fix PATH mutation logic in osx
Browse files Browse the repository at this point in the history
I misuderstood the comment in
microsoft/vscode#99878 (comment)
which resulted in that the PATH change does not apply
with the default shell args settings in OS X.

As noted in #544 (comment)
VS Code sets `-l` as the default shell arg.

I also attempted to run updateIntegratedTerminal after
running environmentVariableCollection.prepend. But it seems like
the onDidOpenTerminal handler is not invoked when VS Code relaunches
the terminals to apply the environmentVariableCollection changes.

Tested manually with
  "terminal.integrated.shellArgs.osx": []
and without the setting.

Fixes #544

Change-Id: Icbaaa5131893038f85e322bad1491a99f26378fd
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/251161
Reviewed-by: Suzy Mueller <suzmue@golang.org>
  • Loading branch information
hyangah committed Sep 2, 2020
1 parent 77bda0a commit 575abde
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions src/goEnvironmentStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,20 +302,26 @@ export function addGoRuntimeBaseToPATH(newGoRuntimeBase: string) {
// environmentVariableCollection.clear();
if (process.platform !== 'darwin') {
environmentVariableCollection?.prepend(pathEnvVar, newGoRuntimeBase + path.delimiter);
} else if (!terminalCreationListener) { // process.platform === 'darwin'
// We don't use EnvironmentVariableCollection on mac
// because this gets confusing for users. Instead we send the
// shell command to change the PATH env var,
// following the suggestion to workaround described in
} else {
// When '-l' or '--login' flags are set, the terminal will invoke a login
// shell again and the paths from the user's login shell will be prepended
// again in front of the path mutated by environmentVariableCollection API.
// That causes the mutated path to be ignored which we don't want.
// So, let's not use the API if those flags are set, but go with the old way
// -- i.e. send the export shell command.
// See the open issue and the discussion here:
// https://github.com/microsoft/vscode/issues/99878#issuecomment-642808852
const terminalShellArgs = <string[]>(
vscode.workspace.getConfiguration('terminal.integrated.shellArgs').get('osx') || []);
// User explicitly chose to run the login shell. So, don't mess with their config.
if (!terminalShellArgs.includes('-l') && !terminalShellArgs.includes('--login')) {
vscode.workspace.getConfiguration('terminal.integrated.shellArgs').get('osx') || []);
if (terminalShellArgs.includes('-l') || terminalShellArgs.includes('--login')) {
for (const term of vscode.window.terminals) {
updateIntegratedTerminal(term);
}
terminalCreationListener = vscode.window.onDidOpenTerminal(updateIntegratedTerminal);
if (!terminalCreationListener) {
terminalCreationListener = vscode.window.onDidOpenTerminal(updateIntegratedTerminal);
}
} else {
environmentVariableCollection?.prepend(pathEnvVar, newGoRuntimeBase + path.delimiter);
}
}

Expand Down

0 comments on commit 575abde

Please sign in to comment.