Skip to content

Commit a16b918

Browse files
TyriarTyriar
committed
Pass config to external terminal service
The config service on the main process isn't complete which results in the terminal config being undefined if there are no user terminals settings set. Fixes microsoft#125985
1 parent 6bca69f commit a16b918

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)