From e95e22e03aacbfdde8339de5a4982d3de4d53279 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 6 May 2021 11:00:14 -0700 Subject: [PATCH] Switch priority of shell/shellArgs and defaultProfile setting Fixes #123159 We were too aggressive introducing the new default profile system as many users (as well as dev containers, microsoft/vscode-dev-containers#838) depend on these settings for their workflow. This also includes a change in behavior where if shellArgs are specified, they are applied to the fallback shell (even when shell.platform isn't specified), which aligns with past behavior. --- .../browser/terminalProfileResolverService.ts | 26 ++++++++++++------- .../terminal/common/terminalConfiguration.ts | 22 +++++++++------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts b/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts index fcedae4ecc8b0..e38d4b2c64c62 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts @@ -109,6 +109,12 @@ export abstract class BaseTerminalProfileResolverService implements ITerminalPro } } + // If either shell or shellArgs are specified, they will take priority for now until we + // allow users to migrate, see https://github.com/microsoft/vscode/issues/123171 + if (this.getSafeConfigValue('shell', options.os) || this.getSafeConfigValue('shellArgs', options.os, false)) { + return this._getFallbackDefaultProfile(options); + } + // Return the real default profile if it exists and is valid const defaultProfile = await this._getRealDefaultProfile(false, options.os); if (defaultProfile) { @@ -137,18 +143,18 @@ export abstract class BaseTerminalProfileResolverService implements ITerminalPro private async _getFallbackDefaultProfile(options: IShellLaunchConfigResolveOptions): Promise { let executable: string; - let args: string | string[] | undefined; const shellSetting = this.getSafeConfigValue('shell', options.os); if (this._isValidShell(shellSetting)) { executable = shellSetting; - const shellArgsSetting = this.getSafeConfigValue('shellArgs', options.os); - if (this._isValidShellArgs(shellArgsSetting, options.os)) { - args = shellArgsSetting; - } } else { executable = await this._context.getDefaultSystemShell(options.remoteAuthority, options.os); } + let args: string | string[] | undefined; + const shellArgsSetting = this.getSafeConfigValue('shellArgs', options.os); + if (this._isValidShellArgs(shellArgsSetting, options.os)) { + args = shellArgsSetting; + } if (args === undefined) { if (options.os === OperatingSystem.Macintosh && args === undefined) { // macOS should launch a login shell by default @@ -276,21 +282,21 @@ export abstract class BaseTerminalProfileResolverService implements ITerminalPro } // TODO: Remove when workspace trust is enabled - getSafeConfigValue(key: string, os: OperatingSystem): unknown | undefined { - return this.getSafeConfigValueFullKey(`terminal.integrated.${key}.${this._getOsKey(os)}`); + getSafeConfigValue(key: string, os: OperatingSystem, useDefaultValue: boolean = true): unknown | undefined { + return this.getSafeConfigValueFullKey(`terminal.integrated.${key}.${this._getOsKey(os)}`, useDefaultValue); } - getSafeConfigValueFullKey(key: string): unknown | undefined { + getSafeConfigValueFullKey(key: string, useDefaultValue: boolean = true): unknown | undefined { const isWorkspaceConfigAllowed = this._configurationService.getValue('terminal.integrated.allowWorkspaceConfiguration'); if (isWorkspaceConfigAllowed) { return this._configurationService.getValue(key); } else { const config = this._configurationService.inspect(key); - const value = config.user?.value || config.default?.value; + const value = config.user?.value || (useDefaultValue ? config.default?.value : undefined); // Clone if needed to allow extensibility if (Array.isArray(value)) { return value.slice(); } - if (typeof value === 'object') { + if (value !== null && typeof value === 'object') { return { ...value }; } return value; diff --git a/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts b/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts index d7519357c2976..f3e00b01833c5 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts @@ -47,6 +47,10 @@ const terminalProfileSchema: IJSONSchema = { } }; +const shellDeprecationMessageLinux = localize('terminal.integrated.shell.linux.deprecation', "This is deprecated, the new recommended way to configure your default shell is by creating a terminal profile in {0} and setting its profile name as the default in {1}. This will currently take priority over the new profiles settings but that will change in the future.", '`#terminal.integrated.profiles.linux#`', '`#terminal.integrated.defaultProfile.linux#`'); +const shellDeprecationMessageOsx = localize('terminal.integrated.shell.osx.deprecation', "This is deprecated, the new recommended way to configure your default shell is by creating a terminal profile in {0} and setting its profile name as the default in {1}. This will currently take priority over the new profiles settings but that will change in the future.", '`#terminal.integrated.profiles.osx#`', '`#terminal.integrated.defaultProfile.osx#`'); +const shellDeprecationMessageWindows = localize('terminal.integrated.shell.windows.deprecation', "This is deprecated, the new recommended way to configure your default shell is by creating a terminal profile in {0} and setting its profile name as the default in {1}. This will currently take priority over the new profiles settings but that will change in the future.", '`#terminal.integrated.profiles.windows#`', '`#terminal.integrated.defaultProfile.windows#`'); + export const terminalConfiguration: IConfigurationNode = { id: 'terminal', order: 100, @@ -93,7 +97,7 @@ export const terminalConfiguration: IConfigurationNode = { type: 'string' }, default: [], - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.linux#` instead' + markdownDeprecationMessage: shellDeprecationMessageLinux }, 'terminal.integrated.shellArgs.osx': { restricted: true, @@ -106,7 +110,7 @@ export const terminalConfiguration: IConfigurationNode = { // is the reason terminals on macOS typically run login shells by default which set up // the environment. See http://unix.stackexchange.com/a/119675/115410 default: ['-l'], - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.osx#` instead' + markdownDeprecationMessage: shellDeprecationMessageOsx }, 'terminal.integrated.shellArgs.windows': { restricted: true, @@ -125,7 +129,7 @@ export const terminalConfiguration: IConfigurationNode = { } ], default: [], - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.windows#` instead' + markdownDeprecationMessage: shellDeprecationMessageWindows }, 'terminal.integrated.profiles.windows': { restricted: true, @@ -261,19 +265,19 @@ export const terminalConfiguration: IConfigurationNode = { } }, 'terminal.integrated.defaultProfile.linux': { - description: localize('terminal.integrated.defaultProfile.linux', 'The default profile used on Linux. When set to a valid profile name, this will override the values of `terminal.integrated.shell.osx` and `terminal.integrated.shellArgs.osx`.'), + markdownDescription: localize('terminal.integrated.defaultProfile.linux', "The default profile used on Linux. This setting will currently be ignored if either {0} or {1} are set.", '`#terminal.integrated.shell.linux#`', '`#terminal.integrated.shellArgs.linux#`'), type: ['string', 'null'], default: null, scope: ConfigurationScope.APPLICATION // Disallow setting the default in workspace settings }, 'terminal.integrated.defaultProfile.osx': { - description: localize('terminal.integrated.defaultProfile.osx', 'The default profile used on macOS. When set to a valid profile name, this will override the values of `terminal.integrated.shell.osx` and `terminal.integrated.shellArgs.osx`.'), + description: localize('terminal.integrated.defaultProfile.osx', "The default profile used on macOS. This setting will currently be ignored if either {0} or {1} are set.", '`#terminal.integrated.shell.osx#`', '`#terminal.integrated.shellArgs.osx#`'), type: ['string', 'null'], default: null, scope: ConfigurationScope.APPLICATION // Disallow setting the default in workspace settings }, 'terminal.integrated.defaultProfile.windows': { - description: localize('terminal.integrated.defaultProfile.windows', 'The default profile used on Windows. When set to a valid profile name, this will override the values of `terminal.integrated.shell.windows` and `terminal.integrated.shellArgs.windows`.'), + description: localize('terminal.integrated.defaultProfile.windows', "The default profile used on Windows. This setting will currently be ignored if either {0} or {1} are set.", '`#terminal.integrated.shell.windows#`', '`#terminal.integrated.shellArgs.windows#`'), type: ['string', 'null'], default: null, scope: ConfigurationScope.APPLICATION // Disallow setting the default in workspace settings @@ -682,21 +686,21 @@ function getTerminalShellConfigurationStub(linux: string, osx: string, windows: markdownDescription: linux, type: ['string', 'null'], default: null, - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.linux#` instead' + markdownDeprecationMessage: shellDeprecationMessageLinux }, 'terminal.integrated.shell.osx': { restricted: true, markdownDescription: osx, type: ['string', 'null'], default: null, - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.osx#` instead' + markdownDeprecationMessage: shellDeprecationMessageOsx }, 'terminal.integrated.shell.windows': { restricted: true, markdownDescription: windows, type: ['string', 'null'], default: null, - markdownDeprecationMessage: 'This is deprecated, use `#terminal.integrated.defaultProfile.windows#` instead' + markdownDeprecationMessage: shellDeprecationMessageOsx } } };