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.

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

Fix terminals in editor area not reloading #151852

Merged
merged 11 commits into from
Jul 6, 2022
5 changes: 5 additions & 0 deletions src/vs/platform/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ export interface IRawTerminalInstanceLayoutInfo<T> {
relativeSize: number;
terminal: T;
}
export interface IRawTerminalEditorInstanceLayoutInfo<T> {
terminal: T;
}
export type ITerminalInstanceLayoutInfoById = IRawTerminalInstanceLayoutInfo<number>;
export type ITerminalInstanceLayoutInfo = IRawTerminalInstanceLayoutInfo<IPtyHostAttachTarget>;

Expand All @@ -144,9 +147,11 @@ export interface IRawTerminalTabLayoutInfo<T> {
}

export type ITerminalTabLayoutInfoById = IRawTerminalTabLayoutInfo<number>;
export type ITerminalEditorInstanceLayoutInfoById = IRawTerminalEditorInstanceLayoutInfo<number>;

export interface IRawTerminalsLayoutInfo<T> {
tabs: IRawTerminalTabLayoutInfo<T>[];
editorTerminals: IRawTerminalEditorInstanceLayoutInfo<T>[];
}

export interface IPtyHostAttachTarget {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/platform/terminal/common/terminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { UriComponents } from 'vs/base/common/uri';
import { ISerializableEnvironmentVariableCollection, ISerializableEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariable';
import { IFixedTerminalDimensions, IRawTerminalTabLayoutInfo, ITerminalEnvironment, ITerminalTabLayoutInfoById, TerminalIcon, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { IFixedTerminalDimensions, IRawTerminalTabLayoutInfo, ITerminalEditorInstanceLayoutInfoById, ITerminalEnvironment, ITerminalTabLayoutInfoById, TerminalIcon, TitleEventSource } from 'vs/platform/terminal/common/terminal';

export interface ISingleTerminalConfiguration<T> {
userValue: T | undefined;
Expand Down Expand Up @@ -41,6 +41,7 @@ export interface IWorkspaceFolderData {
export interface ISetTerminalLayoutInfoArgs {
workspaceId: string;
tabs: ITerminalTabLayoutInfoById[];
editorTerminals: ITerminalEditorInstanceLayoutInfoById[];
}

export interface IGetTerminalLayoutInfoArgs {
Expand Down
26 changes: 24 additions & 2 deletions src/vs/platform/terminal/node/ptyService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { URI } from 'vs/base/common/uri';
import { getSystemShell } from 'vs/base/node/shell';
import { ILogService } from 'vs/platform/log/common/log';
import { RequestStore } from 'vs/platform/terminal/common/requestStore';
import { IProcessDataEvent, IProcessReadyEvent, IPtyService, IRawTerminalInstanceLayoutInfo, IReconnectConstants, IRequestResolveVariablesEvent, IShellLaunchConfig, ITerminalInstanceLayoutInfoById, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalTabLayoutInfoById, TerminalIcon, IProcessProperty, TitleEventSource, ProcessPropertyType, IProcessPropertyMap, IFixedTerminalDimensions, IPersistentTerminalProcessLaunchConfig, ICrossVersionSerializedTerminalState, ISerializedTerminalState, ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
import { IProcessDataEvent, IProcessReadyEvent, IPtyService, IRawTerminalInstanceLayoutInfo, IReconnectConstants, IRequestResolveVariablesEvent, IShellLaunchConfig, ITerminalInstanceLayoutInfoById, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalTabLayoutInfoById, TerminalIcon, IProcessProperty, TitleEventSource, ProcessPropertyType, IProcessPropertyMap, IFixedTerminalDimensions, IPersistentTerminalProcessLaunchConfig, ICrossVersionSerializedTerminalState, ISerializedTerminalState, ITerminalProcessOptions, ITerminalEditorInstanceLayoutInfoById, IRawTerminalEditorInstanceLayoutInfo } from 'vs/platform/terminal/common/terminal';
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
import { escapeNonWindowsPath } from 'vs/platform/terminal/common/terminalEnvironment';
import { Terminal as XtermTerminal } from 'xterm-headless';
Expand Down Expand Up @@ -344,8 +344,12 @@ export class PtyService extends Disposable implements IPtyService {
if (layout) {
const expandedTabs = await Promise.all(layout.tabs.map(async tab => this._expandTerminalTab(tab)));
const tabs = expandedTabs.filter(t => t.terminals.length > 0);
const expandedEditors = await Promise.all(layout.editorTerminals.map(async editorTerminal => this._expandTerminalEditorInstance(editorTerminal)));
this._logService.trace('ptyService#returnLayoutInfo', tabs);
return { tabs };
return {
tabs,
editorTerminals: expandedEditors
};
}
return undefined;
}
Expand Down Expand Up @@ -380,6 +384,24 @@ export class PtyService extends Disposable implements IPtyService {
}
}

private async _expandTerminalEditorInstance(t: ITerminalEditorInstanceLayoutInfoById): Promise<IRawTerminalEditorInstanceLayoutInfo<IProcessDetails | null>> {
try {
const revivedPtyId = this._revivedPtyIdMap.get(t.terminal)?.newId;
const persistentProcessId = revivedPtyId ?? t.terminal;
const persistentProcess = this._throwIfNoPty(persistentProcessId);
const processDetails = persistentProcess && await this._buildProcessDetails(t.terminal, persistentProcess, revivedPtyId !== undefined);
return {
terminal: { ...processDetails, id: persistentProcessId } ?? null
};
} catch (e) {
this._logService.trace(`Couldn't get layout info, a terminal was probably disconnected`, e.message);
// this will be filtered out and not reconnected
return {
terminal: null
};
}
}

private async _buildProcessDetails(id: number, persistentProcess: PersistentTerminalProcess, wasRevived: boolean = false): Promise<IProcessDetails> {
// If the process was just revived, don't do the orphan check as it will
// take some time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,15 @@ export class TerminalEditorService extends Disposable implements ITerminalEditor
// Remove the terminal from the managed instances when the editor closes. This fires when
// dragging and dropping to another editor or closing the editor via cmd/ctrl+w.
this._register(this._editorService.onDidCloseEditor(e => {
console.log('closed editor');
const instance = e.editor instanceof TerminalEditorInput ? e.editor.terminalInstance : undefined;
if (instance) {
const instanceIndex = this.instances.findIndex(e => e === instance);
if (instanceIndex !== -1) {
this.instances.splice(instanceIndex, 1);
}
}
console.log(this.instances);
}));
this._register(this._editorService.onDidActiveEditorChange(() => {
const instance = this._editorService.activeEditor instanceof TerminalEditorInput ? this._editorService.activeEditor : undefined;
Expand Down
40 changes: 36 additions & 4 deletions src/vs/workbench/contrib/terminal/browser/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
import { ILogService } from 'vs/platform/log/common/log';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { ICreateContributedTerminalProfileOptions, IShellLaunchConfig, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalsLayoutInfoById, TerminalLocation, TerminalLocationString, TitleEventSource } from 'vs/platform/terminal/common/terminal';
import { formatMessageForTerminal } from 'vs/platform/terminal/common/terminalStrings';
import { iconForeground } from 'vs/platform/theme/common/colorRegistry';
import { getIconRegistry } from 'vs/platform/theme/common/iconRegistry';
import { ColorScheme } from 'vs/platform/theme/common/theme';
Expand All @@ -38,7 +39,6 @@ import { getInstanceFromResource, getTerminalUri, parseTerminalUri } from 'vs/wo
import { TerminalViewPane } from 'vs/workbench/contrib/terminal/browser/terminalView';
import { IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalBackend, ITerminalConfigHelper, ITerminalProcessExtHostProxy, ITerminalProfileService, TERMINAL_VIEW_ID } from 'vs/workbench/contrib/terminal/common/terminal';
import { TerminalContextKeys } from 'vs/workbench/contrib/terminal/common/terminalContextKey';
import { formatMessageForTerminal } from 'vs/platform/terminal/common/terminalStrings';
import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
Expand Down Expand Up @@ -403,8 +403,22 @@ export class TerminalService implements ITerminalService {
return;
}
const layoutInfo = await localBackend.getTerminalLayoutInfo();
if (layoutInfo && layoutInfo.tabs.length > 0) {
await this._recreateTerminalGroups(layoutInfo);
if (layoutInfo) {
if (layoutInfo.tabs.length > 0) {
await this._recreateTerminalGroups(layoutInfo);
}
if (this._terminalEditorService.instances.length === 0) {
// only do this for restart because editor terminals are already restored
// on reload
if (layoutInfo.editorTerminals && layoutInfo.editorTerminals.length > 0) {
for (const editorTerminal of layoutInfo.editorTerminals) {
await this.createTerminal({
config: { attachPersistentProcess: editorTerminal.terminal! },
location: TerminalLocation.Editor
});
}
}
}
}
// now that terminals have been restored,
// attach listeners to update local state when terminals are changed
Expand Down Expand Up @@ -645,7 +659,25 @@ export class TerminalService implements ITerminalService {
return;
}
const tabs = this._terminalGroupService.groups.map(g => g.getLayoutInfo(g === this._terminalGroupService.activeGroup));
const state: ITerminalsLayoutInfoById = { tabs };

// Save terminals in editors too
const seenPersistentProcessIds: number[] = [];
for (const t of tabs) {
for (const term of t.terminals) {
seenPersistentProcessIds.push(term.terminal);
}
}
const otherInstances = this.instances.filter(instance => typeof instance.persistentProcessId === 'number' && instance.shouldPersist && seenPersistentProcessIds.indexOf(instance.persistentProcessId) === -1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be simpler to get the terminals in the editor area from terminalEditorService

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried and that doesn't work, but should.

I believe it doesn't work because of this, which happens when you close the window and then the state is saved with no terminals
https://github.com/ssigwart/vscode/blob/a084f63f4d3e5477efbcd8851008e8d996a45681/src/vs/workbench/contrib/terminal/browser/terminalEditorService.ts#L92-L103

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tyriar pls checkout the commit I made to fix this ae7a4ba

const editorTerminals = otherInstances.map((instance) => {
return {
terminal: instance.persistentProcessId || 0
};
});

const state: ITerminalsLayoutInfoById = {
tabs,
editorTerminals
};
this._primaryBackend?.setTerminalLayoutInfo(state);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ export class RemoteTerminalChannelClient implements IPtyHostController {
const workspace = this._workspaceContextService.getWorkspace();
const args: ISetTerminalLayoutInfoArgs = {
workspaceId: workspace.id,
tabs: layout ? layout.tabs : []
tabs: layout ? layout.tabs : [],
editorTerminals: layout ? layout.editorTerminals : []
};
return this._channel.call<void>('$setTerminalLayoutInfo', args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
async setTerminalLayoutInfo(layoutInfo?: ITerminalsLayoutInfoById): Promise<void> {
const args: ISetTerminalLayoutInfoArgs = {
workspaceId: this._getWorkspaceId(),
tabs: layoutInfo ? layoutInfo.tabs : []
tabs: layoutInfo ? layoutInfo.tabs : [],
editorTerminals: layoutInfo ? layoutInfo.editorTerminals : []
};
await this._localPtyService.setTerminalLayoutInfo(args);
// Store in the storage service as well to be used when reviving processes as normally this
Expand Down