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

Move local pty parts from ITerminalInstanceService into ILocalTerminalService #118121

Merged
merged 7 commits into from
Mar 4, 2021
Merged

Move local pty parts from ITerminalInstanceService into ILocalTerminalService #118121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3a1f4c4
Move terminal instance service methods to local terminal service
Tyriar Mar 4, 2021
306bebf
Move responsiveness check to local terminal service
Tyriar Mar 4, 2021
468d73e
Fix compile
Tyriar Mar 4, 2021
97cb1ff
Register ILocalTerminalService
Tyriar Mar 4, 2021
c9ae209
Use safe process env from electron-sandbox
Tyriar Mar 4, 2021
8fd1b97
Merge remote-tracking branch 'origin/main' into tyriar/116467
Tyriar Mar 4, 2021
8465489
Fix unit tests
Tyriar Mar 4, 2021
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
32 changes: 32 additions & 0 deletions src/vs/platform/terminal/common/terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { Event } from 'vs/base/common/event';
import { IProcessEnvironment } from 'vs/base/common/platform';
import { URI } from 'vs/base/common/uri';
Expand Down Expand Up @@ -65,6 +66,37 @@ export enum TerminalIpcChannels {
Heartbeat = 'heartbeat'
}

export interface IOffProcessTerminalService {
readonly _serviceBrand: undefined;

/** Fired when the ptyHost process goes down, losing all connections to the service's ptys. */
onPtyHostExit: Event<void>;
/**
* Fired when the ptyHost process becomes non-responsive, this should disable stdin for all
* terminals using this pty host connection and mark them as disconnected.
*/
onPtyHostUnresponsive: Event<void>;
/**
* Fired when the ptyHost process becomes responsive after being non-responsive. Allowing
* previously disconnected terminals to reconnect.
*/
onPtyHostResponsive: Event<void>;
/**
* Fired when the ptyHost has been restarted, this is used as a signal for listening terminals
* that its pty has been lost and will remain disconnected.
*/
onPtyHostRestart: Event<void>;

createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean, shouldPersist: boolean): Promise<ITerminalChildProcess>;
attachToProcess(id: number): Promise<ITerminalChildProcess | undefined>;
setTerminalLayoutInfo(args?: ISetTerminalLayoutInfoArgs): void;
setTerminalLayoutInfo(layout: ITerminalsLayoutInfoById): void;
getTerminalLayoutInfo(): Promise<ITerminalsLayoutInfo | undefined>;
}

export const ILocalTerminalService = createDecorator<ILocalTerminalService>('localTerminalService');
export interface ILocalTerminalService extends IOffProcessTerminalService { }

export interface IPtyService {
readonly _serviceBrand: undefined;

Expand Down
1 change: 0 additions & 1 deletion src/vs/platform/terminal/electron-sandbox/terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'
import { IPtyService } from 'vs/platform/terminal/common/terminal';

export const ILocalPtyService = createDecorator<ILocalPtyService>('localPtyService');

export interface ILocalPtyService extends IPtyService { }
24 changes: 0 additions & 24 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { URI } from 'vs/base/common/uri';
import { FindReplaceState } from 'vs/editor/contrib/find/findState';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensions, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalsLayoutInfoById, ITerminalTabLayoutInfoById } from 'vs/platform/terminal/common/terminal';
import { ISetTerminalLayoutInfoArgs } from 'vs/platform/terminal/common/terminalProcess';
import { IAvailableShellsRequest, ICommandTracker, IDefaultShellAndArgsRequest, INavigationMode, IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalConfigHelper, ITerminalNativeWindowsDelegate, ITerminalProcessExtHostProxy, IWindowsShellHelper, LinuxDistro, TitleEventSource } from 'vs/workbench/contrib/terminal/common/terminal';
import type { Terminal as XTermTerminal } from 'xterm';
import type { SearchAddon as XTermSearchAddon } from 'xterm-addon-search';
Expand All @@ -29,24 +28,6 @@ export const IRemoteTerminalService = createDecorator<IRemoteTerminalService>('r
export interface ITerminalInstanceService {
readonly _serviceBrand: undefined;

/** Fired when the ptyHost process goes down, losing all connections to the service's ptys. */
onPtyHostExit: Event<void>;
/**
* Fired when the ptyHost process becomes non-responsive, this should disable stdin for all
* terminals using this pty host connection and mark them as disconnected.
*/
onPtyHostUnresponsive: Event<void>;
/**
* Fired when the ptyHost process becomes responsive after being non-responsive. Allowing
* previously disconnected terminals to reconnect.
*/
onPtyHostResponsive: Event<void>;
/**
* Fired when the ptyHost has been restarted, this is used as a signal for listening terminals
* that its pty has been lost and will remain disconnected.
*/
onPtyHostRestart: Event<void>;

// These events are optional as the requests they make are only needed on the browser side
onRequestDefaultShellAndArgs?: Event<IDefaultShellAndArgsRequest>;

Expand All @@ -55,13 +36,8 @@ export interface ITerminalInstanceService {
getXtermUnicode11Constructor(): Promise<typeof XTermUnicode11Addon>;
getXtermWebglConstructor(): Promise<typeof XTermWebglAddon>;
createWindowsShellHelper(shellProcessId: number, xterm: XTermTerminal): IWindowsShellHelper;
createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean, shouldPersist: boolean): Promise<ITerminalChildProcess>;
attachToProcess(id: number): Promise<ITerminalChildProcess | undefined>;
getDefaultShellAndArgs(useAutomationShell: boolean, platformOverride?: Platform): Promise<{ shell: string, args: string[] | string | undefined }>;
getMainProcessParentEnv(): Promise<IProcessEnvironment>;
setTerminalLayoutInfo(args?: ISetTerminalLayoutInfoArgs): void;
setTerminalLayoutInfo(layout: ITerminalsLayoutInfoById): void;
getTerminalLayoutInfo(): Promise<ITerminalsLayoutInfo | undefined>;
}

export interface IBrowserTerminalConfigHelper extends ITerminalConfigHelper {
Expand Down
27 changes: 19 additions & 8 deletions src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ILogService } from 'vs/platform/log/common/log';
import { Emitter, Event } from 'vs/base/common/event';
import { IHistoryService } from 'vs/workbench/services/history/common/history';
import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IInstantiationService, optional } from 'vs/platform/instantiation/common/instantiation';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
import { Schemas } from 'vs/base/common/network';
Expand All @@ -27,7 +27,7 @@ import { EnvironmentVariableInfoChangesActive, EnvironmentVariableInfoStale } fr
import { IPathService } from 'vs/workbench/services/path/common/pathService';
import { URI } from 'vs/base/common/uri';
import { IEnvironmentVariableInfo, IEnvironmentVariableService, IMergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { IProcessDataEvent, IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensionsOverride, ITerminalEnvironment, ITerminalLaunchError, FlowControlConstants } from 'vs/platform/terminal/common/terminal';
import { IProcessDataEvent, IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensionsOverride, ITerminalEnvironment, ITerminalLaunchError, FlowControlConstants, ILocalTerminalService } from 'vs/platform/terminal/common/terminal';

/** The amount of time to consider terminal errors to be related to the launch */
const LAUNCHING_DURATION = 500;
Expand Down Expand Up @@ -101,6 +101,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
return this._hasWrittenData;
}

private readonly _localTerminalService?: ILocalTerminalService;

constructor(
private readonly _instanceId: number,
private readonly _configHelper: ITerminalConfigHelper,
Expand All @@ -117,8 +119,11 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
@IPathService private readonly _pathService: IPathService,
@IEnvironmentVariableService private readonly _environmentVariableService: IEnvironmentVariableService,
@IRemoteTerminalService private readonly _remoteTerminalService: IRemoteTerminalService,
@optional(ILocalTerminalService) localTerminalService: ILocalTerminalService,
) {
super();
this._localTerminalService = localTerminalService;

this.ptyProcessReady = new Promise<void>(c => {
this.onProcessReady(() => {
this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`);
Expand Down Expand Up @@ -190,18 +195,22 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
const shouldPersist = !shellLaunchConfig.isFeatureTerminal && this._configHelper.config.enablePersistentSessions;
this._process = await this._remoteTerminalService.createRemoteTerminalProcess(this._instanceId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, shouldPersist, this._configHelper);
} else {
if (!this._localTerminalService) {
this._logService.trace(`Tried to launch a local terminal which is not supported in this window`);
return undefined;
}
// Flow control is not needed for ptys hosted in the same process (ie. the electron
// renderer).
if (shellLaunchConfig.attachPersistentProcess) {
const result = await this._terminalInstanceService.attachToProcess(shellLaunchConfig.attachPersistentProcess.id);
const result = await this._localTerminalService.attachToProcess(shellLaunchConfig.attachPersistentProcess.id);
if (result) {
this._process = result;
} else {
this._logService.trace(`Attach to process failed for terminal ${shellLaunchConfig.attachPersistentProcess}`);
return undefined;
}
} else {
this._process = await this._launchLocalProcess(shellLaunchConfig, cols, rows, this.userHome, isScreenReaderModeEnabled);
this._process = await this._launchLocalProcess(this._localTerminalService, shellLaunchConfig, cols, rows, this.userHome, isScreenReaderModeEnabled);
}
}
}
Expand Down Expand Up @@ -284,6 +293,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}

private async _launchLocalProcess(
localTerminalService: ILocalTerminalService,
shellLaunchConfig: IShellLaunchConfig,
cols: number,
rows: number,
Expand Down Expand Up @@ -325,21 +335,22 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
const useConpty = this._configHelper.config.windowsEnableConpty && !isScreenReaderModeEnabled;
const shouldPersist = this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.isFeatureTerminal;

this._register(this._terminalInstanceService.onPtyHostUnresponsive(() => {
this._register(localTerminalService.onPtyHostUnresponsive(() => {
this.isDisconnected = true;
this._onPtyDisconnect.fire();
}));
this._ptyResponsiveListener = this._terminalInstanceService.onPtyHostResponsive(() => {
this._ptyResponsiveListener = localTerminalService.onPtyHostResponsive(() => {
this.isDisconnected = false;
this._onPtyReconnect.fire();
});
this._register(toDisposable(() => this._ptyResponsiveListener?.dispose()));
this._register(this._terminalInstanceService.onPtyHostRestart(() => {
this._register(localTerminalService.onPtyHostRestart(() => {
// When the pty host restarts, reconnect is no longer possible
this._ptyResponsiveListener?.dispose();
this._ptyResponsiveListener = undefined;
}));
return await this._terminalInstanceService.createTerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env, useConpty, shouldPersist);

return await localTerminalService.createTerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env, useConpty, shouldPersist);
}

public setDimensions(cols: number, rows: number): void {
Expand Down
21 changes: 14 additions & 7 deletions src/vs/workbench/contrib/terminal/browser/terminalService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import * as nls from 'vs/nls';
import { ConfigurationTarget, IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IInstantiationService, optional } from 'vs/platform/instantiation/common/instantiation';
import { IPickOptions, IQuickInputService, IQuickPickItem } from 'vs/platform/quickinput/common/quickInput';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IShellLaunchConfig, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalsLayoutInfoById } from 'vs/platform/terminal/common/terminal';
import { ILocalTerminalService, IShellLaunchConfig, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalsLayoutInfoById } from 'vs/platform/terminal/common/terminal';
import { IViewDescriptorService, IViewsService, ViewContainerLocation } from 'vs/workbench/common/views';
import { IRemoteTerminalService, ITerminalExternalLinkProvider, ITerminalInstance, ITerminalInstanceService, ITerminalService, ITerminalTab, TerminalConnectionState, TerminalShellType, WindowsShellType } from 'vs/workbench/contrib/terminal/browser/terminal';
import { IRemoteTerminalService, ITerminalExternalLinkProvider, ITerminalInstance, ITerminalService, ITerminalTab, TerminalConnectionState, TerminalShellType, WindowsShellType } from 'vs/workbench/contrib/terminal/browser/terminal';
import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper';
import { TerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminalInstance';
import { TerminalTab } from 'vs/workbench/contrib/terminal/browser/terminalTab';
Expand Down Expand Up @@ -102,6 +102,8 @@ export class TerminalService implements ITerminalService {
public get onDidChangeConnectionState(): Event<void> { return this._onDidChangeConnectionState.event; }
public get connectionState(): TerminalConnectionState { return this._connectionState; }

private readonly _localTerminalService?: ILocalTerminalService;

constructor(
@IContextKeyService private _contextKeyService: IContextKeyService,
@IWorkbenchLayoutService private _layoutService: IWorkbenchLayoutService,
Expand All @@ -116,8 +118,10 @@ export class TerminalService implements ITerminalService {
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService,
@IRemoteTerminalService private readonly _remoteTerminalService: IRemoteTerminalService,
@ITelemetryService private readonly _telemetryService: ITelemetryService,
@ITerminalInstanceService private readonly _terminalInstanceService: ITerminalInstanceService
@optional(ILocalTerminalService) localTerminalService: ILocalTerminalService
) {
this._localTerminalService = localTerminalService;

this._activeTabIndex = 0;
this._isShuttingDown = false;
this._findState = new FindReplaceState();
Expand Down Expand Up @@ -172,8 +176,11 @@ export class TerminalService implements ITerminalService {
}

private async _reconnectToLocalTerminals(): Promise<void> {
if (!this._localTerminalService) {
return;
}
// Reattach to all local terminals
const layoutInfo = await this._terminalInstanceService.getTerminalLayoutInfo();
const layoutInfo = await this._localTerminalService.getTerminalLayoutInfo();
if (layoutInfo && layoutInfo.tabs.length > 0) {
this._recreateTerminalTabs(layoutInfo);
// now that terminals have been restored,
Expand Down Expand Up @@ -317,7 +324,7 @@ export class TerminalService implements ITerminalService {
// windows
this.terminalInstances.forEach(instance => instance.dispose(!isWindows));

this._terminalInstanceService.setTerminalLayoutInfo(undefined);
this._localTerminalService!.setTerminalLayoutInfo(undefined);
}

public getTabLabels(): string[] {
Expand Down Expand Up @@ -345,7 +352,7 @@ export class TerminalService implements ITerminalService {
const state: ITerminalsLayoutInfoById = {
tabs: this.terminalTabs.map(t => t.getLayoutInfo(t === this.getActiveTab()))
};
this._terminalInstanceService.setTerminalLayoutInfo(state);
this._localTerminalService!.setTerminalLayoutInfo(state);
}

private _removeTab(tab: ITerminalTab): void {
Expand Down
Loading