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

Switch priority of shell/shellArgs and defaultProfile setting #123174

Merged
merged 1 commit into from
May 12, 2021
Merged

Switch priority of shell/shellArgs and defaultProfile setting #123174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts
View file
Open in desktop
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
Open in desktop
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