Skip to content

Commit

Permalink
Merge pull request #123174 from microsoft/tyriar/r156_defaultProfile
Browse files Browse the repository at this point in the history 
Switch priority of shell/shellArgs and defaultProfile setting
  • Loading branch information
@meganrogge
meganrogge authored May 12, 2021
2 parents 26f617a + e95e22e commit 054a929
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 19 deletions.
26 changes: 16 additions & 10 deletions src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 13 additions & 9 deletions src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
};
Expand Down

0 comments on commit 054a929

Please sign in to comment.