Skip to content

Commit d9b1d6b

Browse files
TyriarTyriar
authored
Merge pull request microsoft#126234 from microsoft/tyriar/157_125985
Fix external terminal when the user has no terminal user settings
2 parents 6bca69f + a16b918 commit d9b1d6b

File tree

6 files changed

+33
-56
lines changed

6 files changed

+33
-56
lines changed

‎src/vs/platform/externalTerminal/common/externalTerminal.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface ITerminalForPlatform {
2222

2323
export interface IExternalTerminalService {
2424
readonly _serviceBrand: undefined;
25-
openTerminal(path: string): Promise<void>;
25+
openTerminal(configuration: IExternalTerminalSettings, path: string): Promise<void>;
2626
runInTerminal(title: string, cwd: string, args: string[], env: ITerminalEnvironment, settings: IExternalTerminalSettings): Promise<number | undefined>;
2727
getDefaultTerminalForPlatforms(): Promise<ITerminalForPlatform>;
2828
}

‎src/vs/platform/externalTerminal/electron-main/externalTerminalService.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ suite('ExternalTerminalService', () => {
4242
};
4343
}
4444
};
45-
let testService = new WindowsExternalTerminalService(mockConfig);
45+
let testService = new WindowsExternalTerminalService();
4646
(<any>testService).spawnTerminal(
4747
mockSpawner,
4848
mockConfig,
@@ -67,7 +67,7 @@ suite('ExternalTerminalService', () => {
6767
}
6868
};
6969
mockConfig.terminal.external.windowsExec = undefined;
70-
let testService = new WindowsExternalTerminalService(mockConfig);
70+
let testService = new WindowsExternalTerminalService();
7171
(<any>testService).spawnTerminal(
7272
mockSpawner,
7373
mockConfig,
@@ -91,7 +91,7 @@ suite('ExternalTerminalService', () => {
9191
};
9292
}
9393
};
94-
let testService = new WindowsExternalTerminalService(mockConfig);
94+
let testService = new WindowsExternalTerminalService();
9595
(<any>testService).spawnTerminal(
9696
mockSpawner,
9797
mockConfig,
@@ -115,7 +115,7 @@ suite('ExternalTerminalService', () => {
115115
return { on: (evt: any) => evt };
116116
}
117117
};
118-
let testService = new WindowsExternalTerminalService(mockConfig);
118+
let testService = new WindowsExternalTerminalService();
119119
(<any>testService).spawnTerminal(
120120
mockSpawner,
121121
mockConfig,
@@ -137,7 +137,7 @@ suite('ExternalTerminalService', () => {
137137
return { on: (evt: any) => evt };
138138
}
139139
};
140-
let testService = new WindowsExternalTerminalService(mockConfig);
140+
let testService = new WindowsExternalTerminalService();
141141
(<any>testService).spawnTerminal(
142142
mockSpawner,
143143
mockConfig,
@@ -160,7 +160,7 @@ suite('ExternalTerminalService', () => {
160160
};
161161
}
162162
};
163-
let testService = new MacExternalTerminalService(mockConfig);
163+
let testService = new MacExternalTerminalService();
164164
(<any>testService).spawnTerminal(
165165
mockSpawner,
166166
mockConfig,
@@ -183,7 +183,7 @@ suite('ExternalTerminalService', () => {
183183
}
184184
};
185185
mockConfig.terminal.external.osxExec = undefined;
186-
let testService = new MacExternalTerminalService(mockConfig);
186+
let testService = new MacExternalTerminalService();
187187
(<any>testService).spawnTerminal(
188188
mockSpawner,
189189
mockConfig,
@@ -206,7 +206,7 @@ suite('ExternalTerminalService', () => {
206206
};
207207
}
208208
};
209-
let testService = new LinuxExternalTerminalService(mockConfig);
209+
let testService = new LinuxExternalTerminalService();
210210
(<any>testService).spawnTerminal(
211211
mockSpawner,
212212
mockConfig,
@@ -230,7 +230,7 @@ suite('ExternalTerminalService', () => {
230230
}
231231
};
232232
mockConfig.terminal.external.linuxExec = undefined;
233-
let testService = new LinuxExternalTerminalService(mockConfig);
233+
let testService = new LinuxExternalTerminalService();
234234
(<any>testService).spawnTerminal(
235235
mockSpawner,
236236
mockConfig,

‎src/vs/platform/externalTerminal/node/externalTerminalService.ts

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ import * as processes from 'vs/base/node/processes';
99
import * as nls from 'vs/nls';
1010
import * as pfs from 'vs/base/node/pfs';
1111
import * as env from 'vs/base/common/platform';
12-
import { IExternalTerminalConfiguration, IExternalTerminalSettings, DEFAULT_TERMINAL_OSX, ITerminalForPlatform, IExternalTerminalMainService } from 'vs/platform/externalTerminal/common/externalTerminal';
13-
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
14-
import { optional } from 'vs/platform/instantiation/common/instantiation';
12+
import { IExternalTerminalSettings, DEFAULT_TERMINAL_OSX, ITerminalForPlatform, IExternalTerminalMainService } from 'vs/platform/externalTerminal/common/externalTerminal';
1513
import { FileAccess } from 'vs/base/common/network';
1614
import { ITerminalEnvironment } from 'vs/platform/terminal/common/terminal';
1715
import { sanitizeProcessEnvironment } from 'vs/base/common/processes';
@@ -31,20 +29,12 @@ export class WindowsExternalTerminalService extends ExternalTerminalService impl
3129
private static readonly CMD = 'cmd.exe';
3230
private static _DEFAULT_TERMINAL_WINDOWS: string;
3331

34-
constructor(
35-
@optional(IConfigurationService) private readonly _configurationService: IConfigurationService
36-
) {
37-
super();
38-
}
39-
40-
public openTerminal(cwd?: string): Promise<void> {
41-
const configuration = this._configurationService.getValue<IExternalTerminalConfiguration>();
32+
public openTerminal(configuration: IExternalTerminalSettings, cwd?: string): Promise<void> {
4233
return this.spawnTerminal(cp, configuration, processes.getWindowsShell(), cwd);
4334
}
4435

45-
public spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalConfiguration, command: string, cwd?: string): Promise<void> {
46-
const terminalConfig = configuration.terminal.external;
47-
const exec = terminalConfig?.windowsExec || WindowsExternalTerminalService.getDefaultTerminalWindows();
36+
public spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalSettings, command: string, cwd?: string): Promise<void> {
37+
const exec = configuration.windowsExec || WindowsExternalTerminalService.getDefaultTerminalWindows();
4838

4939
// Make the drive letter uppercase on Windows (see #9448)
5040
if (cwd && cwd[1] === ':') {
@@ -124,14 +114,7 @@ export class WindowsExternalTerminalService extends ExternalTerminalService impl
124114
export class MacExternalTerminalService extends ExternalTerminalService implements IExternalTerminalMainService {
125115
private static readonly OSASCRIPT = '/usr/bin/osascript'; // osascript is the AppleScript interpreter on OS X
126116

127-
constructor(
128-
@optional(IConfigurationService) private readonly _configurationService: IConfigurationService
129-
) {
130-
super();
131-
}
132-
133-
public openTerminal(cwd?: string): Promise<void> {
134-
const configuration = this._configurationService.getValue<IExternalTerminalConfiguration>();
117+
public openTerminal(configuration: IExternalTerminalSettings, cwd?: string): Promise<void> {
135118
return this.spawnTerminal(cp, configuration, cwd);
136119
}
137120

@@ -199,9 +182,8 @@ export class MacExternalTerminalService extends ExternalTerminalService implemen
199182
});
200183
}
201184

202-
spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalConfiguration, cwd?: string): Promise<void> {
203-
const terminalConfig = configuration.terminal.external;
204-
const terminalApp = terminalConfig?.osxExec || DEFAULT_TERMINAL_OSX;
185+
spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalSettings, cwd?: string): Promise<void> {
186+
const terminalApp = configuration.osxExec || DEFAULT_TERMINAL_OSX;
205187

206188
return new Promise<void>((c, e) => {
207189
const args = ['-a', terminalApp];
@@ -219,14 +201,7 @@ export class LinuxExternalTerminalService extends ExternalTerminalService implem
219201

220202
private static readonly WAIT_MESSAGE = nls.localize('press.any.key', "Press any key to continue...");
221203

222-
constructor(
223-
@optional(IConfigurationService) private readonly _configurationService: IConfigurationService
224-
) {
225-
super();
226-
}
227-
228-
public openTerminal(cwd?: string): Promise<void> {
229-
const configuration = this._configurationService.getValue<IExternalTerminalConfiguration>();
204+
public openTerminal(configuration: IExternalTerminalSettings, cwd?: string): Promise<void> {
230205
return this.spawnTerminal(cp, configuration, cwd);
231206
}
232207

@@ -314,9 +289,8 @@ export class LinuxExternalTerminalService extends ExternalTerminalService implem
314289
return LinuxExternalTerminalService._DEFAULT_TERMINAL_LINUX_READY;
315290
}
316291

317-
spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalConfiguration, cwd?: string): Promise<void> {
318-
const terminalConfig = configuration.terminal.external;
319-
const execPromise = terminalConfig?.linuxExec ? Promise.resolve(terminalConfig.linuxExec) : LinuxExternalTerminalService.getDefaultTerminalLinuxReady();
292+
spawnTerminal(spawner: typeof cp, configuration: IExternalTerminalSettings, cwd?: string): Promise<void> {
293+
const execPromise = configuration.linuxExec ? Promise.resolve(configuration.linuxExec) : LinuxExternalTerminalService.getDefaultTerminalLinuxReady();
320294

321295
return new Promise<void>((c, e) => {
322296
execPromise.then(exec => {

‎src/vs/workbench/contrib/debug/node/terminals.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import * as platform from 'vs/base/common/platform';
88
import { getDriveLetter } from 'vs/base/common/extpath';
99
import { LinuxExternalTerminalService, MacExternalTerminalService, WindowsExternalTerminalService } from 'vs/platform/externalTerminal/node/externalTerminalService';
1010
import { IExternalTerminalService } from 'vs/platform/externalTerminal/common/externalTerminal';
11-
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1211
import { ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration';
1312

1413

@@ -36,11 +35,11 @@ let externalTerminalService: IExternalTerminalService | undefined = undefined;
3635
export function runInExternalTerminal(args: DebugProtocol.RunInTerminalRequestArguments, configProvider: ExtHostConfigProvider): Promise<number | undefined> {
3736
if (!externalTerminalService) {
3837
if (platform.isWindows) {
39-
externalTerminalService = new WindowsExternalTerminalService(<IConfigurationService><unknown>undefined);
38+
externalTerminalService = new WindowsExternalTerminalService();
4039
} else if (platform.isMacintosh) {
41-
externalTerminalService = new MacExternalTerminalService(<IConfigurationService><unknown>undefined);
40+
externalTerminalService = new MacExternalTerminalService();
4241
} else if (platform.isLinux) {
43-
externalTerminalService = new LinuxExternalTerminalService(<IConfigurationService><unknown>undefined);
42+
externalTerminalService = new LinuxExternalTerminalService();
4443
} else {
4544
throw new Error('external terminals not supported on this platform');
4645
}

‎src/vs/workbench/contrib/externalTerminal/browser/externalTerminal.contribution.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ CommandsRegistry.registerCommand({
4242
return fileService.resolveAll(resources.map(r => ({ resource: r }))).then(async stats => {
4343
const targets = distinct(stats.filter(data => data.success));
4444
// Always use integrated terminal when using a remote
45-
const useIntegratedTerminal = remoteAgentService.getConnection() || configurationService.getValue<IExternalTerminalConfiguration>().terminal.explorerKind === 'integrated';
45+
const config = configurationService.getValue<IExternalTerminalConfiguration>();
46+
const useIntegratedTerminal = remoteAgentService.getConnection() || config.terminal.explorerKind === 'integrated';
4647
if (useIntegratedTerminal) {
4748
// TODO: Use uri for cwd in createterminal
4849
const opened: { [path: string]: boolean } = {};
@@ -71,7 +72,7 @@ CommandsRegistry.registerCommand({
7172
});
7273
} else {
7374
distinct(targets.map(({ stat }) => stat!.isDirectory ? stat!.resource.fsPath : dirname(stat!.resource.fsPath))).forEach(cwd => {
74-
terminalService!.openTerminal(cwd);
75+
terminalService!.openTerminal(config.terminal.external, cwd);
7576
});
7677
}
7778
});

‎src/vs/workbench/contrib/externalTerminal/electron-sandbox/externalTerminal.contribution.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as nls from 'vs/nls';
77
import * as paths from 'vs/base/common/path';
8-
import { DEFAULT_TERMINAL_OSX, IExternalTerminalService } from 'vs/platform/externalTerminal/common/externalTerminal';
8+
import { DEFAULT_TERMINAL_OSX, IExternalTerminalService, IExternalTerminalSettings } from 'vs/platform/externalTerminal/common/externalTerminal';
99
import { MenuId, MenuRegistry } from 'vs/platform/actions/common/actions';
1010
import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
1111
import { KEYBINDING_CONTEXT_TERMINAL_NOT_FOCUSED } from 'vs/workbench/contrib/terminal/common/terminal';
@@ -17,6 +17,7 @@ import { IConfigurationRegistry, Extensions, ConfigurationScope } from 'vs/platf
1717
import { Registry } from 'vs/platform/registry/common/platform';
1818
import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
1919
import { IExternalTerminalMainService } from 'vs/platform/externalTerminal/electron-sandbox/externalTerminalMainService';
20+
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
2021

2122
const OPEN_NATIVE_CONSOLE_COMMAND_ID = 'workbench.action.terminal.openNativeConsole';
2223
KeybindingsRegistry.registerCommandAndKeybindingRule({
@@ -28,18 +29,20 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
2829
const historyService = accessor.get(IHistoryService);
2930
// Open external terminal in local workspaces
3031
const terminalService = accessor.get(IExternalTerminalService);
32+
const configurationService = accessor.get(IConfigurationService);
3133
const root = historyService.getLastActiveWorkspaceRoot(Schemas.file);
34+
const config = configurationService.getValue<IExternalTerminalSettings>('terminal.external');
3235
if (root) {
33-
terminalService.openTerminal(root.fsPath);
36+
terminalService.openTerminal(config, root.fsPath);
3437
} else {
3538
// Opens current file's folder, if no folder is open in editor
3639
const activeFile = historyService.getLastActiveFile(Schemas.file);
3740
if (activeFile) {
38-
terminalService.openTerminal(paths.dirname(activeFile.fsPath));
41+
terminalService.openTerminal(config, paths.dirname(activeFile.fsPath));
3942
} else {
4043
const pathService = accessor.get(IPathService);
4144
const userHome = await pathService.userHome();
42-
terminalService.openTerminal(userHome.fsPath);
45+
terminalService.openTerminal(config, userHome.fsPath);
4346
}
4447
}
4548
}

0 commit comments

Comments
 (0)