Merge pull request #123174 from microsoft/tyriar/r156_defaultProfile

Switch priority of shell/shellArgs and defaultProfile setting

Author: meganrogge

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<ITerminalProfile> {
 		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;

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
 			}
 		}
 	};