Skip to content

Commit bf38fbe

Browse files
TyriarTyriar
committed
Remove sync fs call in terminal.ts start up
Fixes microsoft#16378
1 parent 0c89d39 commit bf38fbe

4 files changed

Lines changed: 114 additions & 97 deletions

File tree

‎src/vs/workbench/parts/execution/electron-browser/terminal.contribution.ts‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,33 @@ import { asFileEditorInput } from 'vs/workbench/common/editor';
2323
import { KeyMod, KeyCode } from 'vs/base/common/keyCodes';
2424
import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry';
2525
import { KEYBINDING_CONTEXT_TERMINAL_NOT_FOCUSED } from 'vs/workbench/parts/terminal/common/terminal';
26-
import { DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
27-
28-
let configurationRegistry = <IConfigurationRegistry>Registry.as(Extensions.Configuration);
29-
configurationRegistry.registerConfiguration({
30-
'id': 'externalTerminal',
31-
'order': 100,
32-
'title': nls.localize('terminalConfigurationTitle', "External Terminal"),
33-
'type': 'object',
34-
'properties': {
35-
'terminal.external.windowsExec': {
36-
'type': 'string',
37-
'description': nls.localize('terminal.external.windowsExec', "Customizes which terminal to run on Windows."),
38-
'default': DEFAULT_TERMINAL_WINDOWS
39-
},
40-
'terminal.external.osxExec': {
41-
'type': 'string',
42-
'description': nls.localize('terminal.external.osxExec', "Customizes which terminal application to run on OS X."),
43-
'default': DEFAULT_TERMINAL_OSX
44-
},
45-
'terminal.external.linuxExec': {
46-
'type': 'string',
47-
'description': nls.localize('terminal.external.linuxExec', "Customizes which terminal to run on Linux."),
48-
'default': DEFAULT_TERMINAL_LINUX
26+
import { DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX_READY, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
27+
28+
DEFAULT_TERMINAL_LINUX_READY.then(defaultTerminalLinux => {
29+
let configurationRegistry = <IConfigurationRegistry>Registry.as(Extensions.Configuration);
30+
configurationRegistry.registerConfiguration({
31+
'id': 'externalTerminal',
32+
'order': 100,
33+
'title': nls.localize('terminalConfigurationTitle', "External Terminal"),
34+
'type': 'object',
35+
'properties': {
36+
'terminal.external.windowsExec': {
37+
'type': 'string',
38+
'description': nls.localize('terminal.external.windowsExec', "Customizes which terminal to run on Windows."),
39+
'default': DEFAULT_TERMINAL_WINDOWS
40+
},
41+
'terminal.external.osxExec': {
42+
'type': 'string',
43+
'description': nls.localize('terminal.external.osxExec', "Customizes which terminal application to run on OS X."),
44+
'default': DEFAULT_TERMINAL_OSX
45+
},
46+
'terminal.external.linuxExec': {
47+
'type': 'string',
48+
'description': nls.localize('terminal.external.linuxExec', "Customizes which terminal to run on Linux."),
49+
'default': defaultTerminalLinux
50+
}
4951
}
50-
}
52+
});
5153
});
5254

5355
export class OpenConsoleAction extends Action {

‎src/vs/workbench/parts/execution/electron-browser/terminal.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,35 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
5-
import fs = require('fs');
5+
'use strict';
6+
67
import env = require('vs/base/common/platform');
8+
import * as pfs from 'vs/base/node/pfs';
9+
import { TPromise } from 'vs/base/common/winjs.base';
710

8-
let defaultTerminalLinux = 'xterm';
9-
if (env.isLinux) {
10-
if (fs.existsSync('/etc/debian_version')) {
11-
defaultTerminalLinux = 'x-terminal-emulator';
12-
} else if (process.env.DESKTOP_SESSION === 'gnome' || process.env.DESKTOP_SESSION === 'gnome-classic') {
13-
defaultTerminalLinux = 'gnome-terminal';
14-
} else if (process.env.DESKTOP_SESSION === 'kde-plasma') {
15-
defaultTerminalLinux = 'konsole';
16-
} else if (process.env.COLORTERM) {
17-
defaultTerminalLinux = process.env.COLORTERM;
18-
} else if (process.env.TERM) {
19-
defaultTerminalLinux = process.env.TERM;
11+
export const DEFAULT_TERMINAL_LINUX_READY = new TPromise<string>(c => {
12+
if (env.isLinux) {
13+
pfs.exists('/etc/debian_version').then(isDebian => {
14+
console.log('isDebian' + isDebian);
15+
if (isDebian) {
16+
c('x-terminal-emulator');
17+
} else if (process.env.DESKTOP_SESSION === 'gnome' || process.env.DESKTOP_SESSION === 'gnome-classic') {
18+
c('gnome-terminal');
19+
} else if (process.env.DESKTOP_SESSION === 'kde-plasma') {
20+
c('konsole');
21+
} else if (process.env.COLORTERM) {
22+
c(process.env.COLORTERM);
23+
} else if (process.env.TERM) {
24+
c(process.env.TERM);
25+
} else {
26+
c('xterm');
27+
}
28+
});
29+
return;
2030
}
21-
}
2231

23-
export const DEFAULT_TERMINAL_LINUX = defaultTerminalLinux;
32+
c('xterm');
33+
});
2434

2535
export const DEFAULT_TERMINAL_OSX = 'Terminal.app';
2636

‎src/vs/workbench/parts/execution/electron-browser/terminalService.ts‎

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import errors = require('vs/base/common/errors');
1313
import { TPromise } from 'vs/base/common/winjs.base';
1414
import { ITerminalService } from 'vs/workbench/parts/execution/common/execution';
1515
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
16-
import { ITerminalConfiguration, DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
16+
import { ITerminalConfiguration, DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX_READY, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
1717
import uri from 'vs/base/common/uri';
1818
import { IProcessEnvironment } from 'vs/base/common/platform';
1919

@@ -216,62 +216,66 @@ export class LinuxTerminalService implements ITerminalService {
216216

217217
const configuration = this._configurationService.getConfiguration<ITerminalConfiguration>();
218218
const terminalConfig = configuration.terminal.external;
219-
const exec = terminalConfig.linuxExec || DEFAULT_TERMINAL_LINUX;
219+
const execPromise = terminalConfig.linuxExec ? TPromise.as(terminalConfig.linuxExec) : DEFAULT_TERMINAL_LINUX_READY;
220220

221221
return new TPromise<void>((c, e) => {
222222

223223
let termArgs: string[] = [];
224224
//termArgs.push('--title');
225225
//termArgs.push(`"${TERMINAL_TITLE}"`);
226-
if (exec.indexOf('gnome-terminal') >= 0) {
227-
termArgs.push('-x');
228-
} else {
229-
termArgs.push('-e');
230-
}
231-
termArgs.push('bash');
232-
termArgs.push('-c');
226+
execPromise.then(exec => {
227+
if (exec.indexOf('gnome-terminal') >= 0) {
228+
termArgs.push('-x');
229+
} else {
230+
termArgs.push('-e');
231+
}
232+
termArgs.push('bash');
233+
termArgs.push('-c');
233234

234-
const bashCommand = `${quote(args)}; echo; read -p "${LinuxTerminalService.WAIT_MESSAGE}" -n1;`;
235-
termArgs.push(`''${bashCommand}''`); // wrapping argument in two sets of ' because node is so "friendly" that it removes one set...
235+
const bashCommand = `${quote(args)}; echo; read -p "${LinuxTerminalService.WAIT_MESSAGE}" -n1;`;
236+
termArgs.push(`''${bashCommand}''`); // wrapping argument in two sets of ' because node is so "friendly" that it removes one set...
236237

237-
// merge environment variables into a copy of the process.env
238-
const env = extendObject(extendObject({}, process.env), envVars);
238+
// merge environment variables into a copy of the process.env
239+
const env = extendObject(extendObject({}, process.env), envVars);
239240

240-
const options: any = {
241-
cwd: dir,
242-
env: env
243-
};
241+
const options: any = {
242+
cwd: dir,
243+
env: env
244+
};
244245

245-
let stderr = '';
246-
const cmd = cp.spawn(exec, termArgs, options);
247-
cmd.on('error', e);
248-
cmd.stderr.on('data', (data) => {
249-
stderr += data.toString();
250-
});
251-
cmd.on('exit', (code: number) => {
252-
if (code === 0) { // OK
253-
c(null);
254-
} else {
255-
if (stderr) {
256-
const lines = stderr.split('\n', 1);
257-
e(new Error(lines[0]));
246+
let stderr = '';
247+
const cmd = cp.spawn(exec, termArgs, options);
248+
cmd.on('error', e);
249+
cmd.stderr.on('data', (data) => {
250+
stderr += data.toString();
251+
});
252+
cmd.on('exit', (code: number) => {
253+
if (code === 0) { // OK
254+
c(null);
258255
} else {
259-
e(new Error(nls.localize('linux.term.failed', "'{0}' failed with exit code {1}", exec, code)));
256+
if (stderr) {
257+
const lines = stderr.split('\n', 1);
258+
e(new Error(lines[0]));
259+
} else {
260+
e(new Error(nls.localize('linux.term.failed', "'{0}' failed with exit code {1}", exec, code)));
261+
}
260262
}
261-
}
263+
});
262264
});
263265
});
264266
}
265267

266268
private spawnTerminal(spawner, configuration: ITerminalConfiguration, cwd?: string): TPromise<void> {
267269
const terminalConfig = configuration.terminal.external;
268-
const exec = terminalConfig.linuxExec || DEFAULT_TERMINAL_LINUX;
270+
const execPromise = terminalConfig.linuxExec ? TPromise.as(terminalConfig.linuxExec) : DEFAULT_TERMINAL_LINUX_READY;
269271
const env = cwd ? { cwd: cwd } : void 0;
270272

271273
return new TPromise<void>((c, e) => {
272-
const child = spawner.spawn(exec, [], env);
273-
child.on('error', e);
274-
child.on('exit', () => c(null));
274+
execPromise.then(exec => {
275+
const child = spawner.spawn(exec, [], env);
276+
child.on('error', e);
277+
child.on('exit', () => c(null));
278+
});
275279
});
276280
}
277281
}

‎src/vs/workbench/parts/execution/test/electron-browser/terminalService.test.ts‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { deepEqual, equal } from 'assert';
99
import { WinTerminalService, LinuxTerminalService, MacTerminalService } from 'vs/workbench/parts/execution/electron-browser/terminalService';
10-
import { DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
10+
import { DEFAULT_TERMINAL_WINDOWS, DEFAULT_TERMINAL_LINUX_READY, DEFAULT_TERMINAL_OSX } from 'vs/workbench/parts/execution/electron-browser/terminal';
1111

1212
suite('Execution - TerminalService', () => {
1313
let mockOnExit;
@@ -196,26 +196,27 @@ suite('Execution - TerminalService', () => {
196196
});
197197

198198
test(`LinuxTerminalService - uses default terminal when configuration.terminal.external.linuxExec is undefined`, done => {
199-
let testCwd = 'path/to/workspace';
200-
let mockSpawner = {
201-
spawn: (command, args, opts) => {
202-
// assert
203-
equal(command, DEFAULT_TERMINAL_LINUX, 'terminal should equal expected');
204-
done();
205-
return {
206-
on: (evt) => evt
207-
};
208-
}
209-
};
210-
mockConfig.terminal.external.linuxExec = undefined;
211-
let testService = new LinuxTerminalService(mockConfig);
212-
(<any>testService).spawnTerminal(
213-
mockSpawner,
214-
mockConfig,
215-
testCwd,
216-
mockOnExit,
217-
mockOnError
218-
);
199+
DEFAULT_TERMINAL_LINUX_READY.then(defaultTerminalLinux => {
200+
let testCwd = 'path/to/workspace';
201+
let mockSpawner = {
202+
spawn: (command, args, opts) => {
203+
// assert
204+
equal(command, defaultTerminalLinux, 'terminal should equal expected');
205+
done();
206+
return {
207+
on: (evt) => evt
208+
};
209+
}
210+
};
211+
mockConfig.terminal.external.linuxExec = undefined;
212+
let testService = new LinuxTerminalService(mockConfig);
213+
(<any>testService).spawnTerminal(
214+
mockSpawner,
215+
mockConfig,
216+
testCwd,
217+
mockOnExit,
218+
mockOnError
219+
);
220+
});
219221
});
220-
221222
});

0 commit comments

Comments
 (0)