Skip to content

Commit

Permalink
Merge pull request #230046 from microsoft/tyriar/addon_loader_class
Browse files Browse the repository at this point in the history
Encapsulate xterm addon importing into own class
  • Loading branch information
Tyriar authored Sep 29, 2024
2 parents 2b0935a + bf1663a commit 6ffa99f
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
Terminal,
this._cols,
this._rows,
undefined,
this._scopedInstantiationService.createInstance(TerminalInstanceColorProvider, this._targetRef),
this.capabilities,
this._processManager.shellIntegrationNonce,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,7 @@ export class TerminalService extends Disposable implements ITerminalService {
ctor,
options.cols,
options.rows,
undefined,
options.colorProvider,
options.capabilities || new TerminalCapabilityStore(),
'',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import type { ClipboardAddon as ClipboardAddonType } from '@xterm/addon-clipboard';
import type { ImageAddon as ImageAddonType } from '@xterm/addon-image';
import type { SearchAddon as SearchAddonType } from '@xterm/addon-search';
import type { SerializeAddon as SerializeAddonType } from '@xterm/addon-serialize';
import type { Unicode11Addon as Unicode11AddonType } from '@xterm/addon-unicode11';
import type { WebglAddon as WebglAddonType } from '@xterm/addon-webgl';
import { importAMDNodeModule } from '../../../../../amdX.js';

export interface IXtermAddonNameToCtor {
clipboard: typeof ClipboardAddonType;
image: typeof ImageAddonType;
search: typeof SearchAddonType;
serialize: typeof SerializeAddonType;
unicode11: typeof Unicode11AddonType;
webgl: typeof WebglAddonType;
}

// This interface lets a maps key and value be linked with generics
interface IImportedXtermAddonMap extends Map<keyof IXtermAddonNameToCtor, IXtermAddonNameToCtor[keyof IXtermAddonNameToCtor]> {
get<K extends keyof IXtermAddonNameToCtor>(name: K): IXtermAddonNameToCtor[K] | undefined;
set<K extends keyof IXtermAddonNameToCtor>(name: K, value: IXtermAddonNameToCtor[K]): this;
}

const importedAddons: IImportedXtermAddonMap = new Map();

/**
* Exposes a simple interface to consumers, encapsulating the messy import xterm
* addon import and caching logic.
*/
export class XtermAddonImporter {
async importAddon<T extends keyof IXtermAddonNameToCtor>(name: T): Promise<IXtermAddonNameToCtor[T]> {
let addon = importedAddons.get(name);
if (!addon) {
switch (name) {
case 'clipboard': addon = (await importAMDNodeModule<typeof import('@xterm/addon-clipboard')>('@xterm/addon-clipboard', 'lib/addon-clipboard.js')).ClipboardAddon as IXtermAddonNameToCtor[T]; break;
case 'image': addon = (await importAMDNodeModule<typeof import('@xterm/addon-image')>('@xterm/addon-image', 'lib/addon-image.js')).ImageAddon as IXtermAddonNameToCtor[T]; break;
case 'search': addon = (await importAMDNodeModule<typeof import('@xterm/addon-search')>('@xterm/addon-search', 'lib/addon-search.js')).SearchAddon as IXtermAddonNameToCtor[T]; break;
case 'serialize': addon = (await importAMDNodeModule<typeof import('@xterm/addon-serialize')>('@xterm/addon-serialize', 'lib/addon-serialize.js')).SerializeAddon as IXtermAddonNameToCtor[T]; break;
case 'unicode11': addon = (await importAMDNodeModule<typeof import('@xterm/addon-unicode11')>('@xterm/addon-unicode11', 'lib/addon-unicode11.js')).Unicode11Addon as IXtermAddonNameToCtor[T]; break;
case 'webgl': addon = (await importAMDNodeModule<typeof import('@xterm/addon-webgl')>('@xterm/addon-webgl', 'lib/addon-webgl.js')).WebglAddon as IXtermAddonNameToCtor[T]; break;
}
if (!addon) {
throw new Error(`Could not load addon ${name}`);
}
importedAddons.set(name, addon);
}
return addon as IXtermAddonNameToCtor[T];
}
}
65 changes: 9 additions & 56 deletions src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { DecorationAddon } from './decorationAddon.js';
import { ITerminalCapabilityStore, ITerminalCommand, TerminalCapability } from '../../../../../platform/terminal/common/capabilities/capabilities.js';
import { Emitter } from '../../../../../base/common/event.js';
import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js';
import { importAMDNodeModule } from '../../../../../amdX.js';
import { IContextKey, IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
import { TerminalContextKeys } from '../../common/terminalContextKey.js';
import { IClipboardService } from '../../../../../platform/clipboard/common/clipboardService.js';
Expand All @@ -41,17 +40,12 @@ import { IMouseWheelEvent, StandardWheelEvent } from '../../../../../base/browse
import { ILayoutService } from '../../../../../platform/layout/browser/layoutService.js';
import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js';
import { scrollbarSliderActiveBackground, scrollbarSliderBackground, scrollbarSliderHoverBackground } from '../../../../../platform/theme/common/colorRegistry.js';
import { XtermAddonImporter } from './xtermAddonImporter.js';

const enum RenderConstants {
SmoothScrollDuration = 125
}

let ClipboardAddon: typeof ClipboardAddonType;
let ImageAddon: typeof ImageAddonType;
let SearchAddon: typeof SearchAddonType;
let SerializeAddon: typeof SerializeAddonType;
let Unicode11Addon: typeof Unicode11AddonType;
let WebglAddon: typeof WebglAddonType;

function getFullBufferLineAsString(lineIndex: number, buffer: IBuffer): { lineData: string | undefined; lineIndex: number } {
let line = buffer.getLine(lineIndex);
Expand Down Expand Up @@ -180,6 +174,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
xtermCtor: typeof RawXtermTerminal,
cols: number,
rows: number,
private readonly _xtermAddonLoader: XtermAddonImporter = new XtermAddonImporter(),
private readonly _xtermColorProvider: IXtermColorProvider,
private readonly _capabilities: ITerminalCapabilityStore,
shellIntegrationNonce: string,
Expand Down Expand Up @@ -281,7 +276,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
this.raw.loadAddon(this._decorationAddon);
this._shellIntegrationAddon = new ShellIntegrationAddon(shellIntegrationNonce, disableShellIntegrationReporting, this._telemetryService, this._logService);
this.raw.loadAddon(this._shellIntegrationAddon);
this._getClipboardAddonConstructor().then(ClipboardAddon => {
this._xtermAddonLoader.importAddon('clipboard').then(ClipboardAddon => {
this._clipboardAddon = this._instantiationService.createInstance(ClipboardAddon, undefined, {
async readText(type: ClipboardSelectionType): Promise<string> {
return _clipboardService.readText(type === 'p' ? 'selection' : 'clipboard');
Expand Down Expand Up @@ -309,7 +304,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach

async getContentsAsHtml(): Promise<string> {
if (!this._serializeAddon) {
const Addon = await this._getSerializeAddonConstructor();
const Addon = await this._xtermAddonLoader.importAddon('serialize');
this._serializeAddon = new Addon();
this.raw.loadAddon(this._serializeAddon);
}
Expand All @@ -319,7 +314,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach

async getSelectionAsHtml(command?: ITerminalCommand): Promise<string> {
if (!this._serializeAddon) {
const Addon = await this._getSerializeAddonConstructor();
const Addon = await this._xtermAddonLoader.importAddon('serialize');
this._serializeAddon = new Addon();
this.raw.loadAddon(this._serializeAddon);
}
Expand Down Expand Up @@ -485,7 +480,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
private _searchAddonPromise: Promise<SearchAddonType> | undefined;
private _getSearchAddon(): Promise<SearchAddonType> {
if (!this._searchAddonPromise) {
this._searchAddonPromise = this._getSearchAddonConstructor().then((AddonCtor) => {
this._searchAddonPromise = this._xtermAddonLoader.importAddon('search').then((AddonCtor) => {
this._searchAddon = new AddonCtor({ highlightLimit: XtermTerminalConstants.SearchHighlightLimit });
this.raw.loadAddon(this._searchAddon);
this._searchAddon.onDidChangeResults((results: { resultIndex: number; resultCount: number }) => {
Expand Down Expand Up @@ -687,7 +682,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
}
}

const Addon = await this._getWebglAddonConstructor();
const Addon = await this._xtermAddonLoader.importAddon('webgl');
this._webglAddon = new Addon();
try {
this.raw.loadAddon(this._webglAddon);
Expand Down Expand Up @@ -722,7 +717,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
// Only allow the image addon when webgl is being used to avoid possible GPU issues
if (this._terminalConfigurationService.config.enableImages && this._webglAddon) {
if (!this._imageAddon) {
const AddonCtor = await this._getImageAddonConstructor();
const AddonCtor = await this._xtermAddonLoader.importAddon('image');
this._imageAddon = new AddonCtor();
this.raw.loadAddon(this._imageAddon);
}
Expand All @@ -736,48 +731,6 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach
}
}

protected async _getClipboardAddonConstructor(): Promise<typeof ClipboardAddonType> {
if (!ClipboardAddon) {
ClipboardAddon = (await importAMDNodeModule<typeof import('@xterm/addon-clipboard')>('@xterm/addon-clipboard', 'lib/addon-clipboard.js')).ClipboardAddon;
}
return ClipboardAddon;
}

protected async _getImageAddonConstructor(): Promise<typeof ImageAddonType> {
if (!ImageAddon) {
ImageAddon = (await importAMDNodeModule<typeof import('@xterm/addon-image')>('@xterm/addon-image', 'lib/addon-image.js')).ImageAddon;
}
return ImageAddon;
}

protected async _getSearchAddonConstructor(): Promise<typeof SearchAddonType> {
if (!SearchAddon) {
SearchAddon = (await importAMDNodeModule<typeof import('@xterm/addon-search')>('@xterm/addon-search', 'lib/addon-search.js')).SearchAddon;
}
return SearchAddon;
}

protected async _getUnicode11Constructor(): Promise<typeof Unicode11AddonType> {
if (!Unicode11Addon) {
Unicode11Addon = (await importAMDNodeModule<typeof import('@xterm/addon-unicode11')>('@xterm/addon-unicode11', 'lib/addon-unicode11.js')).Unicode11Addon;
}
return Unicode11Addon;
}

protected async _getWebglAddonConstructor(): Promise<typeof WebglAddonType> {
if (!WebglAddon) {
WebglAddon = (await importAMDNodeModule<typeof import('@xterm/addon-webgl')>('@xterm/addon-webgl', 'lib/addon-webgl.js')).WebglAddon;
}
return WebglAddon;
}

protected async _getSerializeAddonConstructor(): Promise<typeof SerializeAddonType> {
if (!SerializeAddon) {
SerializeAddon = (await importAMDNodeModule<typeof import('@xterm/addon-serialize')>('@xterm/addon-serialize', 'lib/addon-serialize.js')).SerializeAddon;
}
return SerializeAddon;
}

private _disposeOfWebglRenderer(): void {
try {
this._webglAddon?.dispose();
Expand Down Expand Up @@ -849,7 +802,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach

private async _updateUnicodeVersion(): Promise<void> {
if (!this._unicode11Addon && this._terminalConfigurationService.config.unicodeVersion === '11') {
const Addon = await this._getUnicode11Constructor();
const Addon = await this._xtermAddonLoader.importAddon('unicode11');
this._unicode11Addon = new Addon();
this.raw.loadAddon(this._unicode11Addon);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { WebglAddon } from '@xterm/addon-webgl';
import type { IEvent, Terminal } from '@xterm/xterm';
import { deepStrictEqual, strictEqual } from 'assert';
import { importAMDNodeModule } from '../../../../../../amdX.js';
import { isSafari } from '../../../../../../base/browser/browser.js';
import { Color, RGBA } from '../../../../../../base/common/color.js';
import { Emitter } from '../../../../../../base/common/event.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
Expand All @@ -23,6 +22,7 @@ import { XtermTerminal } from '../../../browser/xterm/xtermTerminal.js';
import { ITerminalConfiguration, TERMINAL_VIEW_ID } from '../../../common/terminal.js';
import { registerColors, TERMINAL_BACKGROUND_COLOR, TERMINAL_CURSOR_BACKGROUND_COLOR, TERMINAL_CURSOR_FOREGROUND_COLOR, TERMINAL_FOREGROUND_COLOR, TERMINAL_INACTIVE_SELECTION_BACKGROUND_COLOR, TERMINAL_SELECTION_BACKGROUND_COLOR, TERMINAL_SELECTION_FOREGROUND_COLOR } from '../../../common/terminalColorRegistry.js';
import { workbenchInstantiationService } from '../../../../../test/browser/workbenchTestServices.js';
import { IXtermAddonNameToCtor, XtermAddonImporter } from '../../../browser/xterm/xtermAddonImporter.js';

registerColors();

Expand All @@ -45,11 +45,12 @@ class TestWebglAddon implements WebglAddon {
clearTextureAtlas() { }
}

class TestXtermTerminal extends XtermTerminal {
webglAddonPromise: Promise<typeof WebglAddon> = Promise.resolve(TestWebglAddon);
// Force synchronous to avoid async when activating the addon
protected override _getWebglAddonConstructor() {
return this.webglAddonPromise;
class TestXtermAddonImporter extends XtermAddonImporter {
override async importAddon<T extends keyof IXtermAddonNameToCtor>(name: T): Promise<IXtermAddonNameToCtor[T]> {
if (name === 'webgl') {
return Promise.resolve(TestWebglAddon) as any;
}
return super.importAddon(name);
}
}

Expand Down Expand Up @@ -90,7 +91,7 @@ suite('XtermTerminal', () => {
let instantiationService: TestInstantiationService;
let configurationService: TestConfigurationService;
let themeService: TestThemeService;
let xterm: TestXtermTerminal;
let xterm: XtermTerminal;
let XTermBaseCtor: typeof Terminal;

setup(async () => {
Expand All @@ -113,7 +114,7 @@ suite('XtermTerminal', () => {
XTermBaseCtor = (await importAMDNodeModule<typeof import('@xterm/xterm')>('@xterm/xterm', 'lib/xterm.js')).Terminal;

const capabilityStore = store.add(new TerminalCapabilityStore());
xterm = store.add(instantiationService.createInstance(TestXtermTerminal, XTermBaseCtor, 80, 30, { getBackgroundColor: () => undefined }, capabilityStore, '', true));
xterm = store.add(instantiationService.createInstance(XtermTerminal, XTermBaseCtor, 80, 30, new TestXtermAddonImporter(), { getBackgroundColor: () => undefined }, capabilityStore, '', true));

TestWebglAddon.shouldThrow = false;
TestWebglAddon.isEnabled = false;
Expand All @@ -130,7 +131,7 @@ suite('XtermTerminal', () => {
[PANEL_BACKGROUND]: '#ff0000',
[SIDE_BAR_BACKGROUND]: '#00ff00'
}));
xterm = store.add(instantiationService.createInstance(XtermTerminal, XTermBaseCtor, 80, 30, { getBackgroundColor: () => new Color(new RGBA(255, 0, 0)) }, store.add(new TerminalCapabilityStore()), '', true));
xterm = store.add(instantiationService.createInstance(XtermTerminal, XTermBaseCtor, 80, 30, new TestXtermAddonImporter(), { getBackgroundColor: () => new Color(new RGBA(255, 0, 0)) }, store.add(new TerminalCapabilityStore()), '', true));
strictEqual(xterm.raw.options.theme?.background, '#ff0000');
});
test('should react to and apply theme changes', () => {
Expand Down Expand Up @@ -159,7 +160,7 @@ suite('XtermTerminal', () => {
'terminal.ansiBrightCyan': '#150000',
'terminal.ansiBrightWhite': '#160000',
}));
xterm = store.add(instantiationService.createInstance(XtermTerminal, XTermBaseCtor, 80, 30, { getBackgroundColor: () => undefined }, store.add(new TerminalCapabilityStore()), '', true));
xterm = store.add(instantiationService.createInstance(XtermTerminal, XTermBaseCtor, 80, 30, new TestXtermAddonImporter(), { getBackgroundColor: () => undefined }, store.add(new TerminalCapabilityStore()), '', true));
deepStrictEqual(xterm.raw.options.theme, {
background: undefined,
foreground: '#000200',
Expand Down Expand Up @@ -245,40 +246,4 @@ suite('XtermTerminal', () => {
});
});
});

suite('renderers', () => {
// This is skipped until the webgl renderer bug is fixed in Chromium
// https://bugs.chromium.org/p/chromium/issues/detail?id=1476475
test.skip('should re-evaluate gpu acceleration auto when the setting is changed', async () => {
// Check initial state
strictEqual(TestWebglAddon.isEnabled, false);

// Open xterm as otherwise the webgl addon won't activate
const container = document.createElement('div');
xterm.attachToElement(container);

// Auto should activate the webgl addon
await configurationService.setUserConfiguration('terminal', { integrated: { ...defaultTerminalConfig, gpuAcceleration: 'auto' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await xterm.webglAddonPromise; // await addon activate
if (isSafari) {
strictEqual(TestWebglAddon.isEnabled, false, 'The webgl renderer is always disabled on Safari');
} else {
strictEqual(TestWebglAddon.isEnabled, true);
}

// Turn off to reset state
await configurationService.setUserConfiguration('terminal', { integrated: { ...defaultTerminalConfig, gpuAcceleration: 'off' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await xterm.webglAddonPromise; // await addon activate
strictEqual(TestWebglAddon.isEnabled, false);

// Set to auto again but throw when activating the webgl addon
TestWebglAddon.shouldThrow = true;
await configurationService.setUserConfiguration('terminal', { integrated: { ...defaultTerminalConfig, gpuAcceleration: 'auto' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await xterm.webglAddonPromise; // await addon activate
strictEqual(TestWebglAddon.isEnabled, false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ suite('Buffer Content Tracker', () => {
capabilities.add(TerminalCapability.NaiveCwdDetection, null!);
}
const TerminalCtor = (await importAMDNodeModule<typeof import('@xterm/xterm')>('@xterm/xterm', 'lib/xterm.js')).Terminal;
xterm = store.add(instantiationService.createInstance(XtermTerminal, TerminalCtor, 80, 30, { getBackgroundColor: () => undefined }, capabilities, '', true));
xterm = store.add(instantiationService.createInstance(XtermTerminal, TerminalCtor, 80, 30, undefined, { getBackgroundColor: () => undefined }, capabilities, '', true));
const container = document.createElement('div');
xterm.raw.open(container);
configurationService = new TestConfigurationService({ terminal: { integrated: { tabs: { separator: ' - ', title: '${cwd}', description: '${cwd}' } } } });
Expand Down

0 comments on commit 6ffa99f

Please sign in to comment.