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

get shell integration to work for zsh #143305

Merged
merged 7 commits into from Feb 18, 2022
Merged

get shell integration to work for zsh #143305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fafe5db
work on #141620
meganrogge Feb 17, 2022
a0466d2
fix things
meganrogge Feb 17, 2022
fd910f5
fix tests
meganrogge Feb 17, 2022
34ce77a
fix lint issues
meganrogge Feb 17, 2022
75c5a0d
use resolver service
meganrogge Feb 17, 2022
86af1b9
add comment
meganrogge Feb 18, 2022
0e0eb57
Merge branch 'main' into merogge/fix-zsh
meganrogge Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/gulpfile.reh.js
View file
Open in desktop
Expand Up @@ -76,7 +76,7 @@ const serverResources = [

// Terminal shell integration
'out-build/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh',
'out-build/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh',
'out-build/vs/workbench/contrib/terminal/browser/media/.zshrc',
'out-build/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1',

'!**/test/**'
Expand Down
6 changes: 4 additions & 2 deletions ...nal/browser/media/shellIntegration-zsh.sh → ...nch/contrib/terminal/browser/media/.zshrc
View file
Open in desktop
@@ -1,3 +1,5 @@
autoload -Uz add-zsh-hook

IN_COMMAND_EXECUTION="1"
prompt_start() {
printf "\033]633;A\007"
Expand Down Expand Up @@ -47,8 +49,8 @@ preexec() {
IN_COMMAND_EXECUTION="1"
command_output_start
}
precmd_functions+=($precmd_functions precmd)
preexec_functions+=($preexec_functions preexec)
add-zsh-hook precmd precmd
add-zsh-hook preexec preexec

# Show the welcome message
if [ -z "${VSCODE_SHELL_HIDE_WELCOME-}" ]; then
Expand Down
11 changes: 11 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts
View file
Open in desktop
Expand Up @@ -273,6 +273,10 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
shellLaunchConfig.env = shellLaunchConfig.env || {} as IProcessEnvironment;
shellLaunchConfig.env['VSCODE_SHELL_LOGIN'] = '1';
}
if (env?.['ZDOTDIR']) {
Copy link
Member

Choose a reason for hiding this comment

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

I think ?. for here and above isn't actually needed anymore, we should also remove the TODO: fix above

Copy link
Member

Choose a reason for hiding this comment

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

We may also be able to remove these all together as env is passed into createProcess? See how VSCODE_SHELL_HIDE_WELCOME doesn't need to get set on shellLaunchConfig.env

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see this TODO - slc.env is used, not env in remote terminal backend

vscode/src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts

Line 161 in 4a130c4

env: IProcessEnvironment, // TODO: This is ignored

shellLaunchConfig.env = shellLaunchConfig.env || {} as IProcessEnvironment;
shellLaunchConfig.env['ZDOTDIR'] = env['ZDOTDIR'].replace('${execInstallFolder}', remoteEnv.appRoot.fsPath);
}

newProcess = await backend.createProcess(
shellLaunchConfig,
Expand Down Expand Up @@ -451,6 +455,13 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
const shellIntegration = terminalEnvironment.injectShellIntegrationArgs(this._logService, this._configurationService, env, this._configHelper.config.shellIntegration?.enabled || false, shellLaunchConfig, OS);
if (shellIntegration.enableShellIntegration) {
shellLaunchConfig.args = shellIntegration.args;
if (env?.['ZDOTDIR']) {
shellLaunchConfig.env = shellLaunchConfig.env || {} as IProcessEnvironment;
const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file);
const lastActiveWorkspaceRoot = activeWorkspaceRootUri ? withNullAsUndefined(this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri)) : undefined;
const resolved = await this._configurationResolverService.resolveAsync(lastActiveWorkspaceRoot, env['ZDOTDIR']);
env['ZDOTDIR'] = resolved;
}
// Always resolve the injected arguments on local processes
await this._terminalProfileResolverService.resolveShellLaunchConfig(shellLaunchConfig, {
remoteAuthority: undefined,
Expand Down
9 changes: 7 additions & 2 deletions src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts
View file
Open in desktop
Expand Up @@ -416,8 +416,8 @@ shellIntegrationArgs.set(ShellIntegrationExecutable.WindowsPwsh, ['-noexit', ' -
shellIntegrationArgs.set(ShellIntegrationExecutable.WindowsPwshLogin, ['-l', '-noexit', ' -command', '. \"${execInstallFolder}\\out\\vs\\workbench\\contrib\\terminal\\browser\\media\\shellIntegration.ps1\"{0}']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Pwsh, ['-noexit', '-command', '. "${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1"{0}']);
shellIntegrationArgs.set(ShellIntegrationExecutable.PwshLogin, ['-l', '-noexit', '-command', '. "${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1"']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Zsh, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh"; zsh -i']);
shellIntegrationArgs.set(ShellIntegrationExecutable.ZshLogin, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh"; zsh -il']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Zsh, ['-i']);
shellIntegrationArgs.set(ShellIntegrationExecutable.ZshLogin, ['-il']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Bash, ['--init-file', '${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh']);
const loginArgs = ['-login', '-l'];
const pwshImpliedArgs = ['-nol', '-nologo'];
Expand Down Expand Up @@ -491,7 +491,12 @@ export function injectShellIntegrationArgs(
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh);
} else if (areZshBashLoginArgs(originalArgs)) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin);
} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin)) {
newArgs = originalArgs;
}
// Set the ZDOTDIR to be the dir of the shell integration script so that it runs
// as a .zshrc file and the autoload hook will work and set precmd and preexec correctly
env['ZDOTDIR'] = '${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media';
const showWelcome = configurationService.getValue(TerminalSettingId.ShellIntegrationShowWelcome);
if (!showWelcome) {
env['VSCODE_SHELL_HIDE_WELCOME'] = '1';
Expand Down
5 changes: 4 additions & 1 deletion src/vs/workbench/contrib/terminal/test/browser/terminalProcessManager.test.ts
View file
Open in desktop
Expand Up @@ -90,7 +90,10 @@ suite('Workbench - TerminalProcessManager', () => {
await configurationService.setUserConfiguration('terminal', {
integrated: {
fontFamily: 'bar',
enablePersistentSessions: true
enablePersistentSessions: true,
shellIntegration: {
enabled: false
}
}
});
instantiationService.stub(IConfigurationService, configurationService);
Expand Down