From bec4f6d5c7fa4a824cbc83630b1381494d1ffea5 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Mon, 1 Nov 2021 15:09:25 +0000 Subject: [PATCH 01/11] clear isWrapped in eraseBufferLine --- src/common/InputHandler.test.ts | 50 +++++++++++++++++++++++++++++++++ src/common/InputHandler.ts | 28 +++++++++--------- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/common/InputHandler.test.ts b/src/common/InputHandler.test.ts index e25c3df1..a0930c51 100644 --- a/src/common/InputHandler.test.ts +++ b/src/common/InputHandler.test.ts @@ -386,6 +386,56 @@ describe('InputHandler', () => { assert.equal(bufferService.buffer.lines.get(2)!.translateToString(false), Array(bufferService.cols + 1).join(' ')); }); + it('eraseInLine reflow', async () => { + const bufferService = new MockBufferService(80, 30); + const inputHandler = new TestInputHandler( + bufferService, + new MockCharsetService(), + new MockCoreService(), + new MockDirtyRowService(), + new MockLogService(), + new MockOptionsService(), + new MockCoreMouseService(), + new MockUnicodeService() + ); + + const resetToBaseState = async (): Promise => { + // reset and add a wrapped line + bufferService.buffer.y = 0; + bufferService.buffer.x = 0; + await inputHandler.parseP(Array(bufferService.cols + 1).join('a')); // line 0 + await inputHandler.parseP(Array(bufferService.cols + 10).join('a')); // line 1 and 2 + for (let i = 3; i < bufferService.rows; ++i) await inputHandler.parseP(Array(bufferService.cols + 1).join('a')); + + // confirm precondition that line 2 is wrapped + assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, true); + }; + + // params[0] - erase from the cursor through the end of the row. + await resetToBaseState(); + bufferService.buffer.y = 2; + bufferService.buffer.x = 40; + inputHandler.eraseInLine(Params.fromArray([0])); + assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, true); + bufferService.buffer.y = 2; + bufferService.buffer.x = 0; + inputHandler.eraseInLine(Params.fromArray([0])); + assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, false); + + // params[1] - erase from the beginning of the line through the cursor + await resetToBaseState(); + bufferService.buffer.y = 2; + bufferService.buffer.x = 40; + inputHandler.eraseInLine(Params.fromArray([1])); + assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, true); + + // params[2] - erase complete line + await resetToBaseState(); + bufferService.buffer.y = 2; + bufferService.buffer.x = 40; + inputHandler.eraseInDisplay(Params.fromArray([2])); + assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, false); + }); it('eraseInDisplay', async () => { const bufferService = new MockBufferService(80, 7); const inputHandler = new TestInputHandler( diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index 9371e5f5..89089947 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -25,7 +25,7 @@ import { IBuffer } from 'common/buffer/Types'; /** * Map collect to glevel. Used in `selectCharset`. */ -const GLEVEL: {[key: string]: number} = { '(': 0, ')': 1, '*': 2, '+': 3, '-': 1, '.': 2 }; +const GLEVEL: { [key: string]: number } = { '(': 0, ')': 1, '*': 2, '+': 3, '-': 1, '.': 2 }; /** * VT commands done by the parser - FIXME: move this to the parser? @@ -167,7 +167,7 @@ class DECRQSS implements IDcsHandler { break; case 'r': // DECSTBM const pt = '' + (this._bufferService.buffer.scrollTop + 1) + - ';' + (this._bufferService.buffer.scrollBottom + 1) + 'r'; + ';' + (this._bufferService.buffer.scrollBottom + 1) + 'r'; this._coreService.triggerDataEvent(`${C0.ESC}P1$r${pt}${C0.ESC}\\`); break; case 'm': // SGR @@ -175,7 +175,7 @@ class DECRQSS implements IDcsHandler { this._coreService.triggerDataEvent(`${C0.ESC}P1$r0m${C0.ESC}\\`); break; case ' q': // DECSCUSR - const STYLES: {[key: string]: number} = { 'block': 2, 'underline': 4, 'bar': 6 }; + const STYLES: { [key: string]: number } = { 'block': 2, 'underline': 4, 'bar': 6 }; let style = STYLES[this._optionsService.options.cursorStyle]; style -= this._optionsService.options.cursorBlink ? 1 : 0; this._coreService.triggerDataEvent(`${C0.ESC}P1$r${style} q${C0.ESC}\\`); @@ -855,10 +855,9 @@ export class InputHandler extends Disposable implements IInputHandler { * - any cursor movement sequence keeps working as expected */ if (this._activeBuffer.x === 0 - && this._activeBuffer.y > this._activeBuffer.scrollTop - && this._activeBuffer.y <= this._activeBuffer.scrollBottom - && this._activeBuffer.lines.get(this._activeBuffer.ybase + this._activeBuffer.y)?.isWrapped) - { + && this._activeBuffer.y > this._activeBuffer.scrollTop + && this._activeBuffer.y <= this._activeBuffer.scrollBottom + && this._activeBuffer.lines.get(this._activeBuffer.ybase + this._activeBuffer.y)?.isWrapped) { this._activeBuffer.lines.get(this._activeBuffer.ybase + this._activeBuffer.y)!.isWrapped = false; this._activeBuffer.y--; this._activeBuffer.x = this._bufferService.cols - 1; @@ -1195,6 +1194,7 @@ export class InputHandler extends Disposable implements IInputHandler { * @param y row index * @param start first cell index to be erased * @param end end - 1 is last erased cell + * @param cleanWrap clear the isWrapped flag */ private _eraseInBufferLine(y: number, start: number, end: number, clearWrap: boolean = false): void { const line = this._activeBuffer.lines.get(this._activeBuffer.ybase + y)!; @@ -1320,13 +1320,13 @@ export class InputHandler extends Disposable implements IInputHandler { this._restrictCursor(this._bufferService.cols); switch (params.params[0]) { case 0: - this._eraseInBufferLine(this._activeBuffer.y, this._activeBuffer.x, this._bufferService.cols); + this._eraseInBufferLine(this._activeBuffer.y, this._activeBuffer.x, this._bufferService.cols, this._activeBuffer.x === 0); break; case 1: - this._eraseInBufferLine(this._activeBuffer.y, 0, this._activeBuffer.x + 1); + this._eraseInBufferLine(this._activeBuffer.y, 0, this._activeBuffer.x + 1, false); break; case 2: - this._eraseInBufferLine(this._activeBuffer.y, 0, this._bufferService.cols); + this._eraseInBufferLine(this._activeBuffer.y, 0, this._bufferService.cols, true); break; } this._dirtyRowService.markDirty(this._activeBuffer.y); @@ -1977,7 +1977,7 @@ export class InputHandler extends Disposable implements IInputHandler { break; case 1049: // alt screen buffer cursor this.saveCursor(); - // FALL-THROUGH + // FALL-THROUGH case 47: // alt screen buffer case 1047: // alt screen buffer this._bufferService.buffers.activateAltBuffer(this._eraseAttrData()); @@ -2197,7 +2197,7 @@ export class InputHandler extends Disposable implements IInputHandler { this.restoreCursor(); break; case 1049: // alt screen buffer cursor - // FALL-THROUGH + // FALL-THROUGH case 47: // normal screen buffer case 1047: // normal screen buffer - clearing it first // Ensure the selection manager has the correct buffer @@ -2264,7 +2264,7 @@ export class InputHandler extends Disposable implements IInputHandler { } // exit early if can decide color mode with semicolons if ((accu[1] === 5 && advance + cSpace >= 2) - || (accu[1] === 2 && advance + cSpace >= 5)) { + || (accu[1] === 2 && advance + cSpace >= 5)) { break; } // offset colorSpace slot for semicolon mode @@ -2683,7 +2683,7 @@ export class InputHandler extends Disposable implements IInputHandler { const top = params.params[0] || 1; let bottom: number; - if (params.length < 2 || (bottom = params.params[1]) > this._bufferService.rows || bottom === 0) { + if (params.length < 2 || (bottom = params.params[1]) > this._bufferService.rows || bottom === 0) { bottom = this._bufferService.rows; } From 086ac39095aa1dc21f24fde627e7194ebd737d26 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Mon, 1 Nov 2021 15:23:10 +0000 Subject: [PATCH 02/11] Add whitespaces --- src/common/InputHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts index 89089947..870ecf01 100644 --- a/src/common/InputHandler.ts +++ b/src/common/InputHandler.ts @@ -1977,7 +1977,7 @@ export class InputHandler extends Disposable implements IInputHandler { break; case 1049: // alt screen buffer cursor this.saveCursor(); - // FALL-THROUGH + // FALL-THROUGH case 47: // alt screen buffer case 1047: // alt screen buffer this._bufferService.buffers.activateAltBuffer(this._eraseAttrData()); @@ -2197,7 +2197,7 @@ export class InputHandler extends Disposable implements IInputHandler { this.restoreCursor(); break; case 1049: // alt screen buffer cursor - // FALL-THROUGH + // FALL-THROUGH case 47: // normal screen buffer case 1047: // normal screen buffer - clearing it first // Ensure the selection manager has the correct buffer From 47e53cbdaa8da47ddd66e920c322820af52730b8 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Mon, 1 Nov 2021 15:42:01 +0000 Subject: [PATCH 03/11] Fix unit test calling eraseInDisplay instead of eraseInLine --- src/common/InputHandler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/InputHandler.test.ts b/src/common/InputHandler.test.ts index a0930c51..bff7cbe9 100644 --- a/src/common/InputHandler.test.ts +++ b/src/common/InputHandler.test.ts @@ -433,7 +433,7 @@ describe('InputHandler', () => { await resetToBaseState(); bufferService.buffer.y = 2; bufferService.buffer.x = 40; - inputHandler.eraseInDisplay(Params.fromArray([2])); + inputHandler.eraseInLine(Params.fromArray([2])); assert.equal(bufferService.buffer.lines.get(2)!.isWrapped, false); }); it('eraseInDisplay', async () => { From 3582c00a6176fe705d68265bc7ea4bc6cd6c5f31 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Sat, 6 Nov 2021 14:36:40 +0000 Subject: [PATCH 04/11] Allow setting multiple options through term.options --- src/browser/Terminal.ts | 4 ---- src/browser/Types.d.ts | 1 - src/browser/public/Terminal.ts | 3 +++ src/common/CoreTerminal.ts | 9 +++++++-- src/common/TestUtils.test.ts | 6 ++++++ src/common/services/Services.ts | 18 ++++++++++-------- typings/xterm.d.ts | 2 +- 7 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 24dc7af9..b0506850 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -52,7 +52,6 @@ import { MouseService } from 'browser/services/MouseService'; import { Linkifier2 } from 'browser/Linkifier2'; import { CoreBrowserService } from 'browser/services/CoreBrowserService'; import { CoreTerminal } from 'common/CoreTerminal'; -import { ITerminalOptions as IInitializedTerminalOptions } from 'common/services/Services'; import { rgba } from 'browser/Color'; import { CharacterJoinerService } from 'browser/services/CharacterJoinerService'; @@ -74,9 +73,6 @@ export class Terminal extends CoreTerminal implements ITerminal { public browser: IBrowser = Browser as any; - // TODO: We should remove options once components adopt optionsService - public get options(): IInitializedTerminalOptions { return this.optionsService.options; } - private _customKeyEventHandler: CustomKeyEventHandler | undefined; // browser services diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index bafeff77..a6165840 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -16,7 +16,6 @@ export interface ITerminal extends IPublicTerminal, ICoreTerminal { browser: IBrowser; buffer: IBuffer; viewport: IViewport | undefined; - // TODO: We should remove options once components adopt optionsService options: ITerminalOptions; linkifier: ILinkifier; linkifier2: ILinkifier2; diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 087fde94..a5536855 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -92,6 +92,9 @@ export class Terminal implements ITerminalApi { public get options(): ITerminalOptions { return this._core.options; } + public set options(options: ITerminalOptions) { + this._core.options = options; + } public blur(): void { this._core.blur(); } diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index a5f78f66..6ef9011a 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -22,12 +22,12 @@ */ import { Disposable } from 'common/Lifecycle'; -import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum } from 'common/services/Services'; +import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum, ITerminalOptions } from 'common/services/Services'; import { InstantiationService } from 'common/services/InstantiationService'; import { LogService } from 'common/services/LogService'; import { BufferService, MINIMUM_COLS, MINIMUM_ROWS } from 'common/services/BufferService'; import { OptionsService } from 'common/services/OptionsService'; -import { ITerminalOptions, IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource } from 'common/Types'; +import { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource } from 'common/Types'; import { CoreService } from 'common/services/CoreService'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { CoreMouseService } from 'common/services/CoreMouseService'; @@ -87,6 +87,11 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { public get rows(): number { return this._bufferService.rows; } public get buffers(): IBufferSet { return this._bufferService.buffers; } public get options(): ITerminalOptions { return this.optionsService.publicOptions; } + public set options(options: ITerminalOptions) { + for (const key in options) { + this.optionsService.publicOptions[key] = options[key]; + } + } constructor( options: Partial diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 014142ac..00aedff6 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -131,6 +131,12 @@ export class MockOptionsService implements IOptionsService { } } } + public setOptions(options: ITerminalOptions): void { + for (const key of Object.keys(options)) { + this.options[key] = options[key]; + this.publicOptions[key] = options[key]; + } + } public setOption(key: string, value: T): void { throw new Error('Method not implemented.'); } diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 56b10f73..ed909723 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -164,6 +164,14 @@ export interface IInstantiationService { createInstance any, R extends InstanceType>(t: Ctor, ...args: GetLeadingNonServiceArgs>): R; } +export enum LogLevelEnum { + DEBUG = 0, + INFO = 1, + WARN = 2, + ERROR = 3, + OFF = 4 +} + export const ILogService = createDecorator('LogService'); export interface ILogService { serviceBrand: undefined; @@ -191,13 +199,7 @@ export interface IOptionsService { export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; export type LogLevel = 'debug' | 'info' | 'warn' | 'error' | 'off'; -export enum LogLevelEnum { - DEBUG = 0, - INFO = 1, - WARN = 2, - ERROR = 3, - OFF = 4 -} + export type RendererType = 'dom' | 'canvas'; export interface ITerminalOptions { @@ -207,6 +209,7 @@ export interface ITerminalOptions { bellSound: string; bellStyle: 'none' | 'sound' /* | 'visual' | 'both' */; cols: number; + convertEol: boolean; cursorBlink: boolean; cursorStyle: 'block' | 'underline' | 'bar'; cursorWidth: number; @@ -240,7 +243,6 @@ export interface ITerminalOptions { [key: string]: any; cancelEvents: boolean; - convertEol: boolean; termName: string; } diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index f67e3a16..39f2bfb0 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -638,7 +638,7 @@ declare module 'xterm' { /** * Get the terminal options */ - readonly options: ITerminalOptions; + options: ITerminalOptions; /** * Natural language strings that can be localized. From b1c179461d612e0ef5e2c35395218ad8d28231cc Mon Sep 17 00:00:00 2001 From: silamon <32477463+silamon@users.noreply.github.com> Date: Wed, 10 Nov 2021 09:15:43 +0100 Subject: [PATCH 05/11] Update typings/xterm.d.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> --- typings/xterm.d.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 39f2bfb0..234b7ea0 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -636,7 +636,26 @@ declare module 'xterm' { readonly modes: IModes; /** - * Get the terminal options + * Gets or sets the terminal options. This supports setting multiple options. + * + * @example Get a single option + * ```typescript + * console.log(terminal.options.fontSize); + * ``` + * + * @example Set a single option + * ```typescript + * terminal.options.fontSize = 12; + * ``` + * + * @example Set multiple options + * ```typescript + * terminal.options = { + * fontSize: 12, + * fontFamily: 'Arial', + * ; + * ``` + */ */ options: ITerminalOptions; From 2c65d9686376fa9cdc901d045c3ca9daa0c6ada3 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 10 Nov 2021 08:39:28 +0000 Subject: [PATCH 06/11] Fix tests --- src/browser/Terminal.ts | 10 +++++++++- src/common/CoreTerminal.ts | 4 ++-- typings/xterm.d.ts | 3 +-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index b0506850..5336382d 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -39,7 +39,7 @@ import { MouseZoneManager } from 'browser/MouseZoneManager'; import { AccessibilityManager } from './AccessibilityManager'; import { ITheme, IMarker, IDisposable, ISelectionPosition, ILinkProvider } from 'xterm'; import { DomRenderer } from 'browser/renderer/dom/DomRenderer'; -import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource, IAnsiColorChangeEvent } from 'common/Types'; +import { IKeyboardEvent, KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ScrollSource, IAnsiColorChangeEvent } from 'common/Types'; import { evaluateKeyboardEvent } from 'common/input/Keyboard'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; @@ -54,6 +54,7 @@ import { CoreBrowserService } from 'browser/services/CoreBrowserService'; import { CoreTerminal } from 'common/CoreTerminal'; import { rgba } from 'browser/Color'; import { CharacterJoinerService } from 'browser/services/CharacterJoinerService'; +import { ITerminalOptions } from 'common/services/Services'; // Let it work inside Node.js for automated testing purposes. const document: Document = (typeof window !== 'undefined') ? window.document : null as any; @@ -73,6 +74,13 @@ export class Terminal extends CoreTerminal implements ITerminal { public browser: IBrowser = Browser as any; + public get options(): ITerminalOptions { return this.optionsService.options; } + public set options(options: ITerminalOptions) { + for (const key in options) { + this.optionsService.options[key] = options[key]; + } + } + private _customKeyEventHandler: CustomKeyEventHandler | undefined; // browser services diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 6ef9011a..ab1d6c24 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -22,12 +22,12 @@ */ import { Disposable } from 'common/Lifecycle'; -import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum, ITerminalOptions } from 'common/services/Services'; +import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum } from 'common/services/Services'; import { InstantiationService } from 'common/services/InstantiationService'; import { LogService } from 'common/services/LogService'; import { BufferService, MINIMUM_COLS, MINIMUM_ROWS } from 'common/services/BufferService'; import { OptionsService } from 'common/services/OptionsService'; -import { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource } from 'common/Types'; +import { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource, ITerminalOptions } from 'common/Types'; import { CoreService } from 'common/services/CoreService'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { CoreMouseService } from 'common/services/CoreMouseService'; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 234b7ea0..2a274a24 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -653,10 +653,9 @@ declare module 'xterm' { * terminal.options = { * fontSize: 12, * fontFamily: 'Arial', - * ; + * }; * ``` */ - */ options: ITerminalOptions; /** From 317641c3bb4bd6ebe2ff2a345920be1bdda983cc Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 10 Nov 2021 09:42:45 +0000 Subject: [PATCH 07/11] Add some tests --- src/browser/Terminal.test.ts | 18 +++++++++++- src/browser/Terminal.ts | 7 ----- src/browser/TestUtils.test.ts | 1 + src/browser/Types.d.ts | 1 + src/browser/public/Terminal.test.ts | 43 +++++++++++++++++++++++++++++ src/browser/public/Terminal.ts | 4 +-- src/common/CoreTerminal.ts | 14 +++++++--- 7 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 src/browser/public/Terminal.test.ts diff --git a/src/browser/Terminal.test.ts b/src/browser/Terminal.test.ts index a102d93f..872bfdc7 100644 --- a/src/browser/Terminal.test.ts +++ b/src/browser/Terminal.test.ts @@ -11,7 +11,7 @@ import { IBufferService, IUnicodeService } from 'common/services/Services'; import { Linkifier } from 'browser/Linkifier'; import { MockLogService, MockUnicodeService } from 'common/TestUtils.test'; import { IRegisteredLinkMatcher, IMouseZoneManager, IMouseZone } from 'browser/Types'; -import { IMarker } from 'common/Types'; +import { IMarker, ITerminalOptions } from 'common/Types'; const INIT_COLS = 80; const INIT_ROWS = 24; @@ -1501,6 +1501,22 @@ describe('Terminal', () => { assert.deepEqual(markers.map(el => el.line), [-1, -1, 0, 1, 2]); }); }); + + describe('options', () => { + beforeEach(async () => { + term = new TestTerminal({}); + }); + it('get options', () => { + assert.equal(term.options.cols, 80); + assert.equal(term.options.rows, 24); + }); + it('set options', async () => { + term.options.cols = 40; + assert.equal(term.options.cols, 40); + term.options.rows = 20; + assert.equal(term.options.rows, 20); + }); + }); }); class TestLinkifier extends Linkifier { diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 5336382d..523b50d2 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -74,13 +74,6 @@ export class Terminal extends CoreTerminal implements ITerminal { public browser: IBrowser = Browser as any; - public get options(): ITerminalOptions { return this.optionsService.options; } - public set options(options: ITerminalOptions) { - for (const key in options) { - this.optionsService.options[key] = options[key]; - } - } - private _customKeyEventHandler: CustomKeyEventHandler | undefined; // browser services diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index 8fdf458e..268630c7 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -147,6 +147,7 @@ export class MockTerminal implements ITerminal { public linkifier2!: ILinkifier2; public isFocused!: boolean; public options: ITerminalOptions = {}; + public publicOptions: ITerminalOptions = {}; public element!: HTMLElement; public screenElement!: HTMLElement; public rowContainer!: HTMLElement; diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index a6165840..5540690d 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -16,6 +16,7 @@ export interface ITerminal extends IPublicTerminal, ICoreTerminal { browser: IBrowser; buffer: IBuffer; viewport: IViewport | undefined; + publicOptions: ITerminalOptions; options: ITerminalOptions; linkifier: ILinkifier; linkifier2: ILinkifier2; diff --git a/src/browser/public/Terminal.test.ts b/src/browser/public/Terminal.test.ts new file mode 100644 index 00000000..f03945a0 --- /dev/null +++ b/src/browser/public/Terminal.test.ts @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2016 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { Terminal } from 'browser/public/Terminal'; +import { assert } from 'chai'; +import { ITerminalOptions } from 'common/Types'; + +const INIT_COLS = 80; +const INIT_ROWS = 24; + +describe('Public Terminal', () => { + let term: Terminal; + const termOptions = { + cols: INIT_COLS, + rows: INIT_ROWS + }; + + describe('options', () => { + beforeEach(async () => { + term = new Terminal(termOptions); + }); + it('get options', () => { + const options: ITerminalOptions = term.options; + assert.equal(options.cols, 80); + assert.equal(options.rows, 24); + }); + it('set options', async () => { + const options: ITerminalOptions = term.options; + assert.throws(() => options.cols = 40); + assert.throws(() => options.rows = 20); + term.options.scrollback = 1; + assert.equal(term.options.scrollback, 1); + term.options= { + fontSize: 12, + fontFamily: 'Arial' + }; + assert.equal(term.options.fontSize, 12); + assert.equal(term.options.fontFamily, 'Arial'); + }); + }); +}); diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index a5536855..2138f116 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -90,10 +90,10 @@ export class Terminal implements ITerminalApi { }; } public get options(): ITerminalOptions { - return this._core.options; + return this._core.publicOptions; } public set options(options: ITerminalOptions) { - this._core.options = options; + this._core.publicOptions = options; } public blur(): void { this._core.blur(); diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index ab1d6c24..4223e780 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -22,12 +22,12 @@ */ import { Disposable } from 'common/Lifecycle'; -import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum } from 'common/services/Services'; +import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, IDirtyRowService, LogLevelEnum, ITerminalOptions } from 'common/services/Services'; import { InstantiationService } from 'common/services/InstantiationService'; import { LogService } from 'common/services/LogService'; import { BufferService, MINIMUM_COLS, MINIMUM_ROWS } from 'common/services/BufferService'; import { OptionsService } from 'common/services/OptionsService'; -import { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource, ITerminalOptions } from 'common/Types'; +import { IDisposable, IBufferLine, IAttributeData, ICoreTerminal, IKeyboardEvent, IScrollEvent, ScrollSource, ITerminalOptions as IPublicTerminalOptions } from 'common/Types'; import { CoreService } from 'common/services/CoreService'; import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { CoreMouseService } from 'common/services/CoreMouseService'; @@ -86,12 +86,18 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { public get cols(): number { return this._bufferService.cols; } public get rows(): number { return this._bufferService.rows; } public get buffers(): IBufferSet { return this._bufferService.buffers; } - public get options(): ITerminalOptions { return this.optionsService.publicOptions; } - public set options(options: ITerminalOptions) { + public get publicOptions(): IPublicTerminalOptions { return this.optionsService.publicOptions; } + public set publicOptions(options: IPublicTerminalOptions) { for (const key in options) { this.optionsService.publicOptions[key] = options[key]; } } + public get options(): ITerminalOptions { return this.optionsService.options; } + public set options(options: ITerminalOptions) { + for (const key in options) { + this.optionsService.options[key] = options[key]; + } + } constructor( options: Partial From 4245ce33b59f876a6c7403d0ab84a76c10bc2449 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 10 Nov 2021 10:17:14 +0000 Subject: [PATCH 08/11] Remove publicOptions completely --- src/browser/TestUtils.test.ts | 1 - src/browser/Types.d.ts | 1 - src/browser/public/Terminal.ts | 22 ++++++++++++++++++++-- src/common/CoreTerminal.ts | 6 ------ src/common/TestUtils.test.ts | 3 --- src/common/services/OptionsService.ts | 22 ++++------------------ src/common/services/Services.ts | 1 - 7 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index 268630c7..8fdf458e 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -147,7 +147,6 @@ export class MockTerminal implements ITerminal { public linkifier2!: ILinkifier2; public isFocused!: boolean; public options: ITerminalOptions = {}; - public publicOptions: ITerminalOptions = {}; public element!: HTMLElement; public screenElement!: HTMLElement; public rowContainer!: HTMLElement; diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index 5540690d..a6165840 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -16,7 +16,6 @@ export interface ITerminal extends IPublicTerminal, ICoreTerminal { browser: IBrowser; buffer: IBuffer; viewport: IViewport | undefined; - publicOptions: ITerminalOptions; options: ITerminalOptions; linkifier: ILinkifier; linkifier2: ILinkifier2; diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 2138f116..059b6402 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -13,6 +13,11 @@ import { UnicodeApi } from 'common/public/UnicodeApi'; import { AddonManager } from 'common/public/AddonManager'; import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi'; +/** + * The set of options that only have an effect when set in the Terminal constructor. + */ +const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; + export class Terminal implements ITerminalApi { private _core: ITerminal; private _addonManager: AddonManager; @@ -90,10 +95,11 @@ export class Terminal implements ITerminalApi { }; } public get options(): ITerminalOptions { - return this._core.publicOptions; + return this._core.options; } public set options(options: ITerminalOptions) { - this._core.publicOptions = options; + this._checkReadonlyOptions(options); + this._core.options = options; } public blur(): void { this._core.blur(); @@ -219,6 +225,7 @@ export class Terminal implements ITerminalApi { public setOption(key: 'cols' | 'rows', value: number): void; public setOption(key: string, value: any): void; public setOption(key: any, value: any): void { + this._checkReadonlyOptions(); this._core.optionsService.setOption(key, value); } public refresh(start: number, end: number): void { @@ -245,4 +252,15 @@ export class Terminal implements ITerminalApi { } } } + + private _checkReadonlyOptions(options?: ITerminalOptions): void { + // Throw an error if any constructor only option is modified + // from terminal.options + // Modifications from anywhere else are allowed + for (const propName in options) { + if (CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) { + throw new Error(`Option "${propName}" can only be set in the constructor`); + } + } + } } diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 4223e780..d5378247 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -86,12 +86,6 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { public get cols(): number { return this._bufferService.cols; } public get rows(): number { return this._bufferService.rows; } public get buffers(): IBufferSet { return this._bufferService.buffers; } - public get publicOptions(): IPublicTerminalOptions { return this.optionsService.publicOptions; } - public set publicOptions(options: IPublicTerminalOptions) { - for (const key in options) { - this.optionsService.publicOptions[key] = options[key]; - } - } public get options(): ITerminalOptions { return this.optionsService.options; } public set options(options: ITerminalOptions) { for (const key in options) { diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 00aedff6..67488be0 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -121,20 +121,17 @@ export class MockLogService implements ILogService { export class MockOptionsService implements IOptionsService { public serviceBrand: any; public options: ITerminalOptions = clone(DEFAULT_OPTIONS); - public publicOptions: ITerminalOptions = clone(DEFAULT_OPTIONS); public onOptionChange: IEvent = new EventEmitter().event; constructor(testOptions?: Partial) { if (testOptions) { for (const key of Object.keys(testOptions)) { this.options[key] = testOptions[key]; - this.publicOptions[key] = testOptions[key]; } } } public setOptions(options: ITerminalOptions): void { for (const key of Object.keys(options)) { this.options[key] = options[key]; - this.publicOptions[key] = options[key]; } } public setOption(key: string, value: T): void { diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index a245c153..65b0703c 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -57,17 +57,11 @@ export const DEFAULT_OPTIONS: Readonly = { const FONT_WEIGHT_OPTIONS: Extract[] = ['normal', 'bold', '100', '200', '300', '400', '500', '600', '700', '800', '900']; -/** - * The set of options that only have an effect when set in the Terminal constructor. - */ -const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows']; - export class OptionsService implements IOptionsService { public serviceBrand: any; private _options: ITerminalOptions; public options: ITerminalOptions; - public publicOptions: ITerminalOptions; private _onOptionChange = new EventEmitter(); public get onOptionChange(): IEvent { return this._onOptionChange.event; } @@ -87,11 +81,10 @@ export class OptionsService implements IOptionsService { } // set up getters and setters for each option - this.options = this._setupOptions(this._options, false); - this.publicOptions = this._setupOptions(this._options, true); + this.options = this._setupOptions(this._options); } - private _setupOptions(options: ITerminalOptions, isPublic: boolean): ITerminalOptions { + private _setupOptions(options: ITerminalOptions): ITerminalOptions { const copiedOptions = { ... options }; for (const propName in copiedOptions) { Object.defineProperty(copiedOptions, propName, { @@ -106,13 +99,6 @@ export class OptionsService implements IOptionsService { throw new Error(`No option with key "${propName}"`); } - // Throw an error if any constructor only option is modified - // from terminal.options - // Modifications from anywhere else are allowed - if (isPublic && CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) { - throw new Error(`Option "${propName}" can only be set in the constructor`); - } - value = this._sanitizeAndValidateOption(propName, value); // Don't fire an option change event if they didn't change if (this._options[propName] !== value) { @@ -126,7 +112,7 @@ export class OptionsService implements IOptionsService { } public setOption(key: string, value: any): void { - this.publicOptions[key] = value; + this.options[key] = value; } private _sanitizeAndValidateOption(key: string, value: any): any { @@ -181,6 +167,6 @@ export class OptionsService implements IOptionsService { } public getOption(key: string): any { - return this.publicOptions[key]; + return this.options[key]; } } diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index ed909723..537ac6db 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -189,7 +189,6 @@ export interface IOptionsService { serviceBrand: undefined; readonly options: ITerminalOptions; - readonly publicOptions: ITerminalOptions; readonly onOptionChange: IEvent; From 71257ec00e00a57ee557eb2087ce788f11915b31 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 10 Nov 2021 10:45:42 +0000 Subject: [PATCH 09/11] Fix tests --- src/browser/public/Terminal.ts | 46 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 059b6402..f65d48c1 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as ITerminalApi, ITerminalOptions, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, FontWeight, IModes } from 'xterm'; +import { Terminal as ITerminalApi, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBufferNamespace as IBufferNamespaceApi, IParser, ILinkProvider, IUnicodeHandling, FontWeight, IModes } from 'xterm'; import { ITerminal } from 'browser/Types'; import { Terminal as TerminalCore } from 'browser/Terminal'; import * as Strings from 'browser/LocalizableStrings'; @@ -12,6 +12,7 @@ import { ParserApi } from 'common/public/ParserApi'; import { UnicodeApi } from 'common/public/UnicodeApi'; import { AddonManager } from 'common/public/AddonManager'; import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi'; +import { ITerminalOptions } from 'common/Types'; /** * The set of options that only have an effect when set in the Terminal constructor. @@ -23,10 +24,33 @@ export class Terminal implements ITerminalApi { private _addonManager: AddonManager; private _parser: IParser | undefined; private _buffer: BufferNamespaceApi | undefined; + private _publicOptions: ITerminalOptions; constructor(options?: ITerminalOptions) { this._core = new TerminalCore(options); this._addonManager = new AddonManager(); + + this._publicOptions = {}; + for (const propName in this._core.options) { + Object.defineProperty(this._publicOptions, propName, { + get: () => { + return this._core.options[propName]; + }, + set: (value: any) => { + this._checkReadonlyOptions(propName); + this._core.options[propName] = value; + } + }); + } + } + + private _checkReadonlyOptions(propName: string): void { + // Throw an error if any constructor only option is modified + // from terminal.options + // Modifications from anywhere else are allowed + if (CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) { + throw new Error(`Option "${propName}" can only be set in the constructor`); + } } private _checkProposedApi(): void { @@ -95,11 +119,12 @@ export class Terminal implements ITerminalApi { }; } public get options(): ITerminalOptions { - return this._core.options; + return this._publicOptions; } public set options(options: ITerminalOptions) { - this._checkReadonlyOptions(options); - this._core.options = options; + for (const propName in options) { + this._publicOptions[propName] = options[propName]; + } } public blur(): void { this._core.blur(); @@ -225,7 +250,7 @@ export class Terminal implements ITerminalApi { public setOption(key: 'cols' | 'rows', value: number): void; public setOption(key: string, value: any): void; public setOption(key: any, value: any): void { - this._checkReadonlyOptions(); + this._checkReadonlyOptions(key); this._core.optionsService.setOption(key, value); } public refresh(start: number, end: number): void { @@ -252,15 +277,4 @@ export class Terminal implements ITerminalApi { } } } - - private _checkReadonlyOptions(options?: ITerminalOptions): void { - // Throw an error if any constructor only option is modified - // from terminal.options - // Modifications from anywhere else are allowed - for (const propName in options) { - if (CONSTRUCTOR_ONLY_OPTIONS.includes(propName)) { - throw new Error(`Option "${propName}" can only be set in the constructor`); - } - } - } } From 65712980614eac68bed4b03040e3a13bf86ff673 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 20 Dec 2021 10:02:49 -0800 Subject: [PATCH 10/11] Mark IKeyboardEvent.keyCode as deprecated --- src/common/Types.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/Types.d.ts b/src/common/Types.d.ts index 88497e4a..3af7e2dc 100644 --- a/src/common/Types.d.ts +++ b/src/common/Types.d.ts @@ -46,6 +46,7 @@ export interface IKeyboardEvent { ctrlKey: boolean; shiftKey: boolean; metaKey: boolean; + /** @deprecated See KeyboardEvent.keyCode */ keyCode: number; key: string; type: string; From 7535d2e527697b4c223280ed6e3a2256400961d3 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 21 Dec 2021 09:13:53 -0800 Subject: [PATCH 11/11] Move options test to Terminal.api.ts --- src/browser/public/Terminal.test.ts | 43 ----------------------------- test/api/Terminal.api.ts | 31 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 43 deletions(-) delete mode 100644 src/browser/public/Terminal.test.ts diff --git a/src/browser/public/Terminal.test.ts b/src/browser/public/Terminal.test.ts deleted file mode 100644 index f03945a0..00000000 --- a/src/browser/public/Terminal.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -/** - * Copyright (c) 2016 The xterm.js authors. All rights reserved. - * @license MIT - */ - -import { Terminal } from 'browser/public/Terminal'; -import { assert } from 'chai'; -import { ITerminalOptions } from 'common/Types'; - -const INIT_COLS = 80; -const INIT_ROWS = 24; - -describe('Public Terminal', () => { - let term: Terminal; - const termOptions = { - cols: INIT_COLS, - rows: INIT_ROWS - }; - - describe('options', () => { - beforeEach(async () => { - term = new Terminal(termOptions); - }); - it('get options', () => { - const options: ITerminalOptions = term.options; - assert.equal(options.cols, 80); - assert.equal(options.rows, 24); - }); - it('set options', async () => { - const options: ITerminalOptions = term.options; - assert.throws(() => options.cols = 40); - assert.throws(() => options.rows = 20); - term.options.scrollback = 1; - assert.equal(term.options.scrollback, 1); - term.options= { - fontSize: 12, - fontFamily: 'Arial' - }; - assert.equal(term.options.fontSize, 12); - assert.equal(term.options.fontFamily, 'Arial'); - }); - }); -}); diff --git a/test/api/Terminal.api.ts b/test/api/Terminal.api.ts index 00598e47..7243e617 100644 --- a/test/api/Terminal.api.ts +++ b/test/api/Terminal.api.ts @@ -6,6 +6,7 @@ import { assert } from 'chai'; import { pollFor, timeout, writeSync, openTerminal, launchBrowser } from './TestUtils'; import { Browser, Page } from 'playwright'; +import { fail } from 'assert'; const APP = 'http://127.0.0.1:3001/test'; @@ -160,6 +161,36 @@ describe('API Integration Tests', function(): void { assert.equal(await page.evaluate(`window.term.getOption('rendererType')`), 'dom'); }); + describe('options', () => { + it('getter', async () => { + await openTerminal(page); + assert.equal(await page.evaluate(`window.term.options.rendererType`), 'canvas'); + assert.equal(await page.evaluate(`window.term.options.cols`), 80); + assert.equal(await page.evaluate(`window.term.options.rows`), 24); + }); + it('setter', async () => { + await openTerminal(page); + try { + await page.evaluate('window.term.options.cols = 40'); + fail(); + } catch {} + try { + await page.evaluate('window.term.options.rows = 20'); + fail(); + } catch {} + await page.evaluate('window.term.options.scrollback = 1'); + assert.equal(await page.evaluate(`window.term.options.scrollback`), 1); + await page.evaluate(` + window.term.options = { + fontSize: 30, + fontFamily: 'Arial' + }; + `); + assert.equal(await page.evaluate(`window.term.options.fontSize`), 30); + assert.equal(await page.evaluate(`window.term.options.fontFamily`), 'Arial'); + }); + }); + describe('renderer', () => { it('foreground', async () => { await openTerminal(page, { rendererType: 'dom' });