Skip to content

Commit 83856c3

Browse files
TyriarTyriar
authored
Merge pull request microsoft#209711 from r-sargento/defaultProfile-env
Fix microsoft#201247 (Integrated Terminal does not set the environment variables from default profile)
2 parents 910cdff + ea09e5c commit 83856c3

File tree

2 files changed

+195
-31
lines changed

2 files changed

+195
-31
lines changed

‎src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
188188
private _lineDataEventAddon: LineDataEventAddon | undefined;
189189
private readonly _scopedContextKeyService: IContextKeyService;
190190

191-
readonly capabilities = new TerminalCapabilityStoreMultiplexer();
191+
readonly capabilities = this._register(new TerminalCapabilityStoreMultiplexer());
192192
readonly statusList: ITerminalStatusList;
193193

194194
get store(): DisposableStore {
@@ -459,7 +459,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
459459
this._setTitle(this._shellLaunchConfig.name, TitleEventSource.Api);
460460
}
461461

462-
this.statusList = this._scopedInstantiationService.createInstance(TerminalStatusList);
462+
this.statusList = this._register(this._scopedInstantiationService.createInstance(TerminalStatusList));
463463
this._initDimensions();
464464
this._processManager = this._createProcessManager();
465465

@@ -479,12 +479,14 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
479479
this.shellLaunchConfig.executable = defaultProfile.path;
480480
this.shellLaunchConfig.args = defaultProfile.args;
481481
if (this.shellLaunchConfig.isExtensionOwnedTerminal) {
482-
// Only use default icon and color if they are undefined in the SLC
482+
// Only use default icon and color and env if they are undefined in the SLC
483483
this.shellLaunchConfig.icon ??= defaultProfile.icon;
484484
this.shellLaunchConfig.color ??= defaultProfile.color;
485+
this.shellLaunchConfig.env ??= defaultProfile.env;
485486
} else {
486487
this.shellLaunchConfig.icon = defaultProfile.icon;
487488
this.shellLaunchConfig.color = defaultProfile.color;
489+
this.shellLaunchConfig.env = defaultProfile.env;
488490
}
489491
}
490492

@@ -564,7 +566,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
564566
}
565567
let contribution: ITerminalContribution;
566568
try {
567-
contribution = this._scopedInstantiationService.createInstance(desc.ctor, this, this._processManager, this._widgetManager);
569+
contribution = this._register(this._scopedInstantiationService.createInstance(desc.ctor, this, this._processManager, this._widgetManager));
568570
this._contributions.set(desc.id, contribution);
569571
} catch (err) {
570572
onUnexpectedError(err);
@@ -734,20 +736,20 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
734736
);
735737
this.xterm = xterm;
736738
this.updateAccessibilitySupport();
737-
this.xterm.onDidRequestRunCommand(e => {
739+
this._register(this.xterm.onDidRequestRunCommand(e => {
738740
if (e.copyAsHtml) {
739741
this.copySelection(true, e.command);
740742
} else {
741743
this.sendText(e.command.command, e.noNewLine ? false : true);
742744
}
743-
});
744-
this.xterm.onDidRequestFocus(() => this.focus());
745-
this.xterm.onDidRequestSendText(e => this.sendText(e, false));
745+
}));
746+
this._register(this.xterm.onDidRequestFocus(() => this.focus()));
747+
this._register(this.xterm.onDidRequestSendText(e => this.sendText(e, false)));
746748
// Write initial text, deferring onLineFeed listener when applicable to avoid firing
747749
// onLineData events containing initialText
748750
const initialTextWrittenPromise = this._shellLaunchConfig.initialText ? new Promise<void>(r => this._writeInitialText(xterm, r)) : undefined;
749751
const lineDataEventAddon = this._register(new LineDataEventAddon(initialTextWrittenPromise));
750-
lineDataEventAddon.onLineData(e => this._onLineData.fire(e));
752+
this._register(lineDataEventAddon.onLineData(e => this._onLineData.fire(e)));
751753
this._lineDataEventAddon = lineDataEventAddon;
752754
// Delay the creation of the bell listener to avoid showing the bell when the terminal
753755
// starts up or reconnects
@@ -767,21 +769,21 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
767769
this._register(xterm.raw.onSelectionChange(async () => this._onSelectionChange()));
768770
this._register(xterm.raw.buffer.onBufferChange(() => this._refreshAltBufferContextKey()));
769771

770-
this._processManager.onProcessData(e => this._onProcessData(e));
772+
this._register(this._processManager.onProcessData(e => this._onProcessData(e)));
771773
this._register(xterm.raw.onData(async data => {
772774
await this._processManager.write(data);
773775
this._onDidInputData.fire(this);
774776
}));
775777
this._register(xterm.raw.onBinary(data => this._processManager.processBinary(data)));
776778
// Init winpty compat and link handler after process creation as they rely on the
777779
// underlying process OS
778-
this._processManager.onProcessReady(async (processTraits) => {
780+
this._register(this._processManager.onProcessReady(async (processTraits) => {
779781
if (this._processManager.os) {
780782
lineDataEventAddon.setOperatingSystem(this._processManager.os);
781783
}
782784
xterm.raw.options.windowsPty = processTraits.windowsPty;
783-
});
784-
this._processManager.onRestoreCommands(e => this.xterm?.shellIntegration.deserialize(e));
785+
}));
786+
this._register(this._processManager.onRestoreCommands(e => this.xterm?.shellIntegration.deserialize(e)));
785787

786788
this._register(this._viewDescriptorService.onDidChangeLocation(({ views }) => {
787789
if (views.some(v => v.id === TERMINAL_VIEW_ID)) {
@@ -1328,7 +1330,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
13281330
this.shellLaunchConfig.attachPersistentProcess?.shellIntegrationNonce
13291331
);
13301332
this.capabilities.add(processManager.capabilities);
1331-
processManager.onProcessReady(async (e) => {
1333+
this._register(processManager.onProcessReady(async (e) => {
13321334
this._onProcessIdReady.fire(this);
13331335
this._initialCwd = await this.getInitialCwd();
13341336
// Set the initial name based on the _resolved_ shell launch config, this will also
@@ -1356,9 +1358,9 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
13561358
});
13571359
this._setTitle(this._shellLaunchConfig.executable, TitleEventSource.Process);
13581360
}
1359-
});
1360-
processManager.onProcessExit(exitCode => this._onProcessExit(exitCode));
1361-
processManager.onDidChangeProperty(({ type, value }) => {
1361+
}));
1362+
this._register(processManager.onProcessExit(exitCode => this._onProcessExit(exitCode)));
1363+
this._register(processManager.onDidChangeProperty(({ type, value }) => {
13621364
switch (type) {
13631365
case ProcessPropertyType.Cwd:
13641366
this._cwd = value;
@@ -1390,15 +1392,15 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
13901392
this._usedShellIntegrationInjection = true;
13911393
break;
13921394
}
1393-
});
1395+
}));
13941396

1395-
processManager.onProcessData(ev => {
1397+
this._register(processManager.onProcessData(ev => {
13961398
this._initialDataEvents?.push(ev.data);
13971399
this._onData.fire(ev.data);
1398-
});
1399-
processManager.onProcessReplayComplete(() => this._onProcessReplayComplete.fire());
1400-
processManager.onEnvironmentVariableInfoChanged(e => this._onEnvironmentVariableInfoChanged(e));
1401-
processManager.onPtyDisconnect(() => {
1400+
}));
1401+
this._register(processManager.onProcessReplayComplete(() => this._onProcessReplayComplete.fire()));
1402+
this._register(processManager.onEnvironmentVariableInfoChanged(e => this._onEnvironmentVariableInfoChanged(e)));
1403+
this._register(processManager.onPtyDisconnect(() => {
14021404
if (this.xterm) {
14031405
this.xterm.raw.options.disableStdin = true;
14041406
}
@@ -1408,13 +1410,13 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
14081410
icon: Codicon.debugDisconnect,
14091411
tooltip: nls.localize('disconnectStatus', "Lost connection to process")
14101412
});
1411-
});
1412-
processManager.onPtyReconnect(() => {
1413+
}));
1414+
this._register(processManager.onPtyReconnect(() => {
14131415
if (this.xterm) {
14141416
this.xterm.raw.options.disableStdin = false;
14151417
}
14161418
this.statusList.remove(TerminalStatus.Disconnected);
1417-
});
1419+
}));
14181420

14191421
return processManager;
14201422
}

‎src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts

Lines changed: 167 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,184 @@ import { TestConfigurationService } from 'vs/platform/configuration/test/common/
1313
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
1414
import { TerminalCapability } from 'vs/platform/terminal/common/capabilities/capabilities';
1515
import { TerminalCapabilityStore } from 'vs/platform/terminal/common/capabilities/terminalCapabilityStore';
16-
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
17-
import { ITerminalConfigurationService, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal';
16+
import { IWorkspaceFolder, IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
17+
import { ITerminalConfigurationService, ITerminalInstance, ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal';
1818
import { TerminalConfigurationService } from 'vs/workbench/contrib/terminal/browser/terminalConfigurationService';
19-
import { TerminalLabelComputer, parseExitResult } from 'vs/workbench/contrib/terminal/browser/terminalInstance';
20-
import { ProcessState } from 'vs/workbench/contrib/terminal/common/terminal';
19+
import { TerminalLabelComputer, parseExitResult, TerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminalInstance';
20+
import { ProcessState, ITerminalProfileResolverService } from 'vs/workbench/contrib/terminal/common/terminal';
2121
import { fixPath } from 'vs/workbench/services/search/test/browser/queryBuilder.test';
22-
import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
22+
import { workbenchInstantiationService, TestEnvironmentService, TestLayoutService, TestLifecycleService, TestPathService, TestTerminalProfileResolverService } from 'vs/workbench/test/browser/workbenchTestServices';
23+
import { TestContextService, TestExtensionService, TestHistoryService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';
24+
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
25+
import { TestAccessibilityService } from 'vs/platform/accessibility/test/common/testAccessibilityService';
26+
import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService';
27+
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
28+
import { ContextMenuService } from 'vs/platform/contextview/browser/contextMenuService';
29+
import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
30+
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
31+
import { MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
32+
import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
33+
import { NullLogService } from 'vs/platform/log/common/log';
34+
import { IProductService } from 'vs/platform/product/common/productService';
35+
import { IStorageService } from 'vs/platform/storage/common/storage';
36+
import { ITerminalChildProcess, ITerminalLogService, ITerminalProfile } from 'vs/platform/terminal/common/terminal';
37+
import { IThemeService } from 'vs/platform/theme/common/themeService';
38+
import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService';
39+
import { IViewDescriptorService } from 'vs/workbench/common/views';
40+
import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
41+
import { EnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariableService';
42+
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
43+
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
44+
import { IHistoryService } from 'vs/workbench/services/history/common/history';
45+
import { ILifecycleService } from 'vs/workbench/services/lifecycle/common/lifecycle';
46+
import { IPathService } from 'vs/workbench/services/path/common/pathService';
47+
import { Event } from 'vs/base/common/event';
48+
import { Disposable } from 'vs/base/common/lifecycle';
49+
import { TestViewDescriptorService } from 'vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test';
2350

2451
const root1 = '/foo/root1';
2552
const ROOT_1 = fixPath(root1);
2653
const root2 = '/foo/root2';
2754
const ROOT_2 = fixPath(root2);
2855

56+
class MockTerminalProfileResolverService extends TestTerminalProfileResolverService {
57+
override async getDefaultProfile(): Promise<ITerminalProfile> {
58+
return {
59+
profileName: "my-sh",
60+
path: "/usr/bin/zsh",
61+
env: {
62+
TEST: "TEST",
63+
},
64+
isDefault: true,
65+
isUnsafePath: false,
66+
isFromPath: true,
67+
icon: {
68+
id: "terminal-linux",
69+
},
70+
color: "terminal.ansiYellow",
71+
};
72+
}
73+
}
74+
75+
const terminalShellTypeContextKey = {
76+
set: () => { },
77+
reset: () => { },
78+
get: () => undefined
79+
};
80+
81+
const terminalInRunCommandPicker = {
82+
set: () => { },
83+
reset: () => { },
84+
get: () => undefined
85+
};
86+
87+
class TestTerminalChildProcess extends Disposable implements ITerminalChildProcess {
88+
id: number = 0;
89+
get capabilities() { return []; }
90+
constructor(
91+
readonly shouldPersist: boolean
92+
) {
93+
super();
94+
}
95+
updateProperty(property: any, value: any): Promise<void> {
96+
throw new Error('Method not implemented.');
97+
}
98+
99+
onProcessOverrideDimensions?: Event<any> | undefined;
100+
onProcessResolvedShellLaunchConfig?: Event<any> | undefined;
101+
onDidChangeHasChildProcesses?: Event<any> | undefined;
102+
103+
onDidChangeProperty = Event.None;
104+
onProcessData = Event.None;
105+
onProcessExit = Event.None;
106+
onProcessReady = Event.None;
107+
onProcessTitleChanged = Event.None;
108+
onProcessShellTypeChanged = Event.None;
109+
async start(): Promise<undefined> { return undefined; }
110+
shutdown(immediate: boolean): void { }
111+
input(data: string): void { }
112+
resize(cols: number, rows: number): void { }
113+
clearBuffer(): void { }
114+
acknowledgeDataEvent(charCount: number): void { }
115+
async setUnicodeVersion(version: '6' | '11'): Promise<void> { }
116+
async getInitialCwd(): Promise<string> { return ''; }
117+
async getCwd(): Promise<string> { return ''; }
118+
async processBinary(data: string): Promise<void> { }
119+
refreshProperty(property: any): Promise<any> { return Promise.resolve(''); }
120+
}
121+
122+
class TestTerminalInstanceService extends Disposable implements Partial<ITerminalInstanceService> {
123+
getBackend() {
124+
return {
125+
onPtyHostExit: Event.None,
126+
onPtyHostUnresponsive: Event.None,
127+
onPtyHostResponsive: Event.None,
128+
onPtyHostRestart: Event.None,
129+
onDidMoveWindowInstance: Event.None,
130+
onDidRequestDetach: Event.None,
131+
createProcess: (
132+
shellLaunchConfig: any,
133+
cwd: string,
134+
cols: number,
135+
rows: number,
136+
unicodeVersion: '6' | '11',
137+
env: any,
138+
windowsEnableConpty: boolean,
139+
shouldPersist: boolean
140+
) => this._register(new TestTerminalChildProcess(shouldPersist)),
141+
getLatency: () => Promise.resolve([])
142+
} as any;
143+
}
144+
}
145+
29146
suite('Workbench - TerminalInstance', () => {
30147
const store = ensureNoDisposablesAreLeakedInTestSuite();
31148

149+
suite('TerminalInstance', () => {
150+
let terminalInstance: ITerminalInstance;
151+
test('should create an instance of TerminalInstance with env from default profile', async () => {
152+
const instantiationService = workbenchInstantiationService(undefined, store);
153+
instantiationService.stub(IConfigurationService, new TestConfigurationService({
154+
terminal: {
155+
integrated: {
156+
fontFamily: 'monospace',
157+
scrollback: 1000,
158+
fastScrollSensitivity: 2,
159+
mouseWheelScrollSensitivity: 1,
160+
unicodeVersion: '6',
161+
shellIntegration: {
162+
enabled: true
163+
},
164+
}
165+
},
166+
}));
167+
instantiationService.stub(IContextKeyService, store.add(instantiationService.createInstance(ContextKeyService)));
168+
instantiationService.stub(IWorkbenchEnvironmentService, TestEnvironmentService);
169+
const mockTerminalProfileResolverService = new MockTerminalProfileResolverService();
170+
instantiationService.set(ITerminalProfileResolverService, mockTerminalProfileResolverService);
171+
instantiationService.stub(IWorkspaceContextService, new TestContextService());
172+
instantiationService.stub(IHistoryService, new TestHistoryService());
173+
instantiationService.stub(ITerminalLogService, store.add(new NullLogService()));
174+
instantiationService.stub(IKeybindingService, new MockKeybindingService());
175+
instantiationService.stub(ITerminalConfigurationService, store.add(instantiationService.createInstance(TerminalConfigurationService)));
176+
instantiationService.stub(ILayoutService, new TestLayoutService());
177+
instantiationService.stub(IThemeService, new TestThemeService());
178+
instantiationService.stub(IViewDescriptorService, new TestViewDescriptorService());
179+
instantiationService.stub(ILifecycleService, store.add(new TestLifecycleService()));
180+
instantiationService.stub(IContextMenuService, store.add(instantiationService.createInstance(ContextMenuService)));
181+
instantiationService.stub(IAccessibilityService, new TestAccessibilityService());
182+
instantiationService.stub(IPathService, new TestPathService());
183+
instantiationService.stub(IProductService, {});
184+
instantiationService.stub(IExtensionService, new TestExtensionService());
185+
instantiationService.stub(IStorageService, store.add(new TestStorageService()));
186+
instantiationService.stub(IEnvironmentVariableService, store.add(instantiationService.createInstance(EnvironmentVariableService)));
187+
instantiationService.stub(ITerminalInstanceService, store.add(new TestTerminalInstanceService()));
188+
terminalInstance = store.add(instantiationService.createInstance(TerminalInstance, terminalShellTypeContextKey, terminalInRunCommandPicker, {}));
189+
// //Wait for the teminalInstance._xtermReadyPromise to resolve
190+
await new Promise(resolve => setTimeout(resolve, 100));
191+
deepStrictEqual(terminalInstance.shellLaunchConfig.env, { TEST: 'TEST' });
192+
});
193+
});
32194
suite('parseExitResult', () => {
33195
test('should return no message for exit code = undefined', () => {
34196
deepStrictEqual(

0 commit comments

Comments
 (0)