Allow optionally specifying shellType in linux terminal profiles, and autodetect flatpak-spawn and host-spawn#316714
Conversation
|
@microsoft-github-policy-service agree |
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @anthonykim1Matched files:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a terminal profile shellType override and updates shell integration shell detection for wrapper executables such as host-spawn and flatpak-spawn.
Changes:
- Adds
shellTypeto terminal profile configuration schemas and profile/launch config interfaces. - Copies resolved profile
shellTypeinto shell launch configs. - Updates shell integration injection to use an explicit shell type or infer it from wrapper args.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts |
Applies resolved profile shellType to launch configs. |
src/vs/platform/terminal/node/terminalEnvironment.ts |
Uses shellType/wrapper detection when choosing shell integration injection logic. |
src/vs/platform/terminal/common/terminalPlatformConfiguration.ts |
Adds shellType to terminal profile JSON schema. |
src/vs/platform/terminal/common/terminal.ts |
Adds shellType fields to terminal profile and launch config interfaces. |
Comments suppressed due to low confidence (6)
src/vs/platform/terminal/node/terminalEnvironment.ts:123
- This
else ifis indented with spaces and does not align with the matchingif. VS Code sources use tabs for indentation.
} else if (arePwshLoginArgs(originalArgs)) {
src/vs/platform/terminal/node/terminalEnvironment.ts:136
- These
else ifbranches are indented with spaces rather than tabs, so they do not align with the surrounding block.
} else if (shell === 'bash.exe') {
if (!originalArgs || originalArgs.length === 0) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
} else if (areZshBashFishLoginArgs(originalArgs)) {
src/vs/platform/terminal/node/terminalEnvironment.ts:158
- This
else ifis indented with spaces instead of tabs and should align with the precedingif.
} else if (areZshBashFishLoginArgs(originalArgs)) {
src/vs/platform/terminal/node/terminalEnvironment.ts:176
- These
else ifbranches are indented with spaces instead of tabs and should align with the precedingifstatements.
} else if (areZshBashFishLoginArgs(originalArgs)) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.FishLogin);
} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Fish) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.FishLogin)) {
src/vs/platform/terminal/node/terminalEnvironment.ts:194
- This
else ifis indented with spaces instead of tabs and should align with the precedingif.
} else if (arePwshLoginArgs(originalArgs)) {
src/vs/platform/terminal/node/terminalEnvironment.ts:211
- These
else ifbranches are indented with spaces instead of tabs and should align with the precedingifstatements.
} else if (areZshBashFishLoginArgs(originalArgs)) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin);
addEnvMixinPathPrefix(options, envMixin, shell);
} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin)) {
| const originalArgs = shellLaunchConfig.args; | ||
| let shell = process.platform === 'win32' ? path.basename(shellLaunchConfig.executable).toLowerCase() : path.basename(shellLaunchConfig.executable); | ||
| if (shellLaunchConfig.shellType) { | ||
| shell = shellLaunchConfig.shellType.toLowerCase(); | ||
| } else if (shell === "host-spawn" || shell === "flatpak-spawn") { | ||
| if (Array.isArray(originalArgs) && originalArgs.length > 0) { | ||
| shell = path.basename(originalArgs[0]).toLowerCase(); | ||
| } | ||
| } | ||
|
|
||
| const appRoot = path.dirname(FileAccess.asFileUri('').fsPath); | ||
| const type = 'injection'; | ||
| let newArgs: string[] | undefined; |
| const originalArgs = shellLaunchConfig.args; | ||
| let shell = process.platform === 'win32' ? path.basename(shellLaunchConfig.executable).toLowerCase() : path.basename(shellLaunchConfig.executable); | ||
| if (shellLaunchConfig.shellType) { | ||
| shell = shellLaunchConfig.shellType.toLowerCase(); | ||
| } else if (shell === "host-spawn" || shell === "flatpak-spawn") { | ||
| if (Array.isArray(originalArgs) && originalArgs.length > 0) { | ||
| shell = path.basename(originalArgs[0]).toLowerCase(); |
|
|
||
|
|
||
| export interface IShellLaunchConfig { | ||
| shellType?: string; |
| shellType: { | ||
| description: localize('terminalProfile.shellType', 'Override the automatically detected shell type. Useful when using wrappers like host-spawn or flatpak-spawn.'), | ||
| type: 'string', | ||
| enum: ['bash', 'sh', 'zsh', 'pwsh', 'powershell', 'cmd', 'fish'] |
There was a problem hiding this comment.
and Windows PowerShell handling currently expects powershell.exe, not powershell),
Add support for powershell on linux
| const originalArgs = shellLaunchConfig.args; | ||
| let shell = process.platform === 'win32' ? path.basename(shellLaunchConfig.executable).toLowerCase() : path.basename(shellLaunchConfig.executable); | ||
| if (shellLaunchConfig.shellType) { | ||
| shell = shellLaunchConfig.shellType.toLowerCase(); | ||
| } else if (shell === "host-spawn" || shell === "flatpak-spawn") { | ||
| if (Array.isArray(originalArgs) && originalArgs.length > 0) { | ||
| shell = path.basename(originalArgs[0]).toLowerCase(); | ||
| } |
|
|
||
| const originalArgs = shellLaunchConfig.args; | ||
| const shell = process.platform === 'win32' ? path.basename(shellLaunchConfig.executable).toLowerCase() : path.basename(shellLaunchConfig.executable); | ||
| const originalArgs = shellLaunchConfig.args; |
| let shell = process.platform === 'win32' ? path.basename(shellLaunchConfig.executable).toLowerCase() : path.basename(shellLaunchConfig.executable); | ||
| if (shellLaunchConfig.shellType) { | ||
| shell = shellLaunchConfig.shellType.toLowerCase(); | ||
| } else if (shell === "host-spawn" || shell === "flatpak-spawn") { |
| if (resolvedProfile.shellType) { | ||
| shellLaunchConfig.shellType = resolvedProfile.shellType; | ||
| } | ||
|
|
| color?: string; | ||
| env?: ITerminalEnvironment; | ||
| requiresPath?: string | ITerminalUnsafePath; | ||
| shellType?: string; |
This introduces a shellType config option and auto-detects flatpak-spawn/host-spawn in shell integration logic.
69c513d to
63f1f3b
Compare
63f1f3b to
5646ac9
Compare
| suite('shellType auto-detection and overrides', async () => { | ||
| test('should auto-detect host-spawn wrapper', async () => { | ||
| const result = await getShellIntegrationInjection({ executable: '/app/bin/host-spawn', args: ['bash'] }, enabledProcessOptions, defaultEnvironment, logService, productService, false); | ||
| strictEqual(result.type, 'injection'); | ||
| const injection = result as IShellIntegrationConfigInjection; | ||
| strictEqual(injection.newArgs?.[0], 'bash'); // The wrapperArg | ||
| }); | ||
| test('should auto-detect flatpak-spawn wrapper', async () => { | ||
| const result = await getShellIntegrationInjection({ executable: 'flatpak-spawn', args: ['--host', 'bash'] }, enabledProcessOptions, defaultEnvironment, logService, productService, false); | ||
| strictEqual(result.type, 'injection'); | ||
| const injection = result as IShellIntegrationConfigInjection; | ||
| strictEqual(injection.newArgs?.[0], '--host'); // The wrapperArg | ||
| strictEqual(injection.newArgs?.[1], 'bash'); // The wrapperArg | ||
| }); | ||
| test('should respect a shellType=bash override when executable=unknown-shell', async () => { | ||
| const result = await getShellIntegrationInjection({ executable: 'unknown-shell', shellType: 'bash', args: ['-l'] }, enabledProcessOptions, defaultEnvironment, logService, productService, false); | ||
| strictEqual(result.type, 'injection'); | ||
| const injection = result as IShellIntegrationConfigInjection; | ||
| strictEqual(injection.envMixin?.['VSCODE_SHELL_LOGIN'], '1'); | ||
| }); | ||
| }); |
| forceShellIntegration: shellLaunchConfig.forceShellIntegration, | ||
| tabActions: shellLaunchConfig.tabActions, | ||
| shellIntegrationEnvironmentReporting: shellLaunchConfig.shellIntegrationEnvironmentReporting, | ||
| shellType: shellLaunchConfig.shellType, |
| description: localize('terminalProfile.shellType', 'Override the automatically detected shell type. Useful when using wrappers like host-spawn or flatpak-spawn.'), | ||
| type: 'string', | ||
| enum: ['bash', 'zsh', 'pwsh', 'powershell', 'fish'] |
| case 'pwsh': | ||
| case 'powershell': { |
| newArgs = [...newArgs]; | ||
| newArgs[newArgs.length - 1] = format(newArgs[newArgs.length - 1], appRoot, ''); | ||
| envMixin['VSCODE_STABLE'] = productService.quality === 'stable' ? '1' : '0'; | ||
| if (wrapperArgs) { newArgs = [...wrapperArgs, ...newArgs]; } |
…orting .exe variants; improve test coverage for shellType overrides Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
src/vs/platform/terminal/node/terminalEnvironment.ts:105
- The override is only lower-cased; it is not normalized between extensionless and
.exeforms. Because the Windows branch matches only*.exenames while the POSIX branch matches extensionless names, the cross-platform values advertised by the profile schema (for exampleshellType: 'bash.exe'on Linux orshellType: 'pwsh'on Windows) still fall through toUnsupportedShell.
if (shellLaunchConfig.shellType) {
shell = shellLaunchConfig.shellType.toLowerCase();
}
src/vs/platform/terminal/node/terminalEnvironment.ts:98
- This detects the wrapped command as the first token that does not start with
-, which misidentifies option values as the shell for flatpak-spawn options that take a separate value (for example--directory DIR). In that caseDIRbecomesshell, so shell integration falls through toUnsupportedShelleven though the actual command later in the args is supported.
} else if (executableBasename === 'flatpak-spawn') {
const wrappedShellIndex = originalArgs.findIndex(arg => !arg.startsWith('-'));
if (wrappedShellIndex >= 0) {
wrapperArgs = originalArgs.slice(0, wrappedShellIndex + 1);
originalArgs = originalArgs.slice(wrappedShellIndex + 1);
shell = path.basename(wrapperArgs[wrappedShellIndex]).toLowerCase();
| description: localize('terminalProfile.shellType', 'Override the automatically detected shell executable basename. Both variants with and without the \'.exe\' extension are supported (e.g., \'bash\' and \'bash.exe\') for cross-platform profile sharing. Useful when using wrappers like host-spawn or flatpak-spawn.'), | ||
| type: 'string', | ||
| enum: ['bash', 'bash.exe', 'zsh', 'zsh.exe', 'pwsh', 'pwsh.exe', 'powershell', 'powershell.exe', 'cmd', 'cmd.exe', 'fish', 'fish.exe'] |
| strictEqual(injection.envMixin?.['VSCODE_SHELL_LOGIN'], '1'); | ||
| }); | ||
| (isWindows ? test.skip : test)('should respect a shellType=powershell override when executable=unknown-shell', async () => { | ||
| const result = await getShellIntegrationInjection({ executable: 'unknown-shell', shellType: 'powershell', args: ['-Command', 'echo'] }, enabledProcessOptions, defaultEnvironment, logService, productService, false); |
| let originalArgs = shellLaunchConfig.args; | ||
| let wrapperArgs: string[] | undefined; | ||
| const executableBasename = path.basename(shellLaunchConfig.executable).toLowerCase(); | ||
| let shell = process.platform === 'win32' ? executableBasename : executableBasename; |
…ble names and update tests for shellType overrides Co-authored-by: Copilot <copilot@github.com>
…e' to disable shell integration and update description for clarity Co-authored-by: Copilot <copilot@github.com>
| } | ||
| logService.error(`Failed to set sticky bit on ${zdotdir}: ${err}`); | ||
| return { type: 'failure', reason: ShellIntegrationInjectionFailureReason.FailedToSetStickyBit }; |
| validatedProfile.isAutoDetected = profile.isAutoDetected; | ||
| validatedProfile.icon = icon; | ||
| validatedProfile.color = profile.color; | ||
| validatedProfile.shellType = profile.shellType; |
| if (shellLaunchConfig.shellType) { | ||
| shell = shellLaunchConfig.shellType.toLowerCase(); | ||
| } | ||
|
|
||
| // When shellType is 'none', disable shell integration without logging a warning for unsupported shells. | ||
| if (shell === 'none') { | ||
| return { type: 'failure', reason: ShellIntegrationInjectionFailureReason.IgnoreShellIntegrationFlag }; |
- Add shellType comparison to profilesEqual in terminalProfileService.ts. - Add tests in terminalProfiles.test.ts to ensure shellType is preserved and validated correctly. - Add coverage for .exe normalization and the 'none' executable/shellType disable path in terminalEnvironment.test.ts.
This introduces a
shellTypeconfig option and auto-detectsflatpak-spawn/host-spawnin shell integration logic.Fixes: