From 68952151c17f70ddac7da6af73655d341410950c Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 13 May 2022 17:52:15 -0700 Subject: [PATCH 1/7] if cached line is empty, translate buffer line to string --- addons/xterm-addon-search/src/SearchAddon.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 1eb126e5c4..b870981698 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -498,7 +498,10 @@ export class SearchAddon implements ITerminalAddon { return this._findInLine(term, searchPosition, searchOptions); } let cache = this._linesCache?.[row]; - if (!cache) { + if (!cache || cache[0] === '') { + // sometimes the search refresh happens before the + // buffer has content for a given line, so if the cached + // line is empty, check it again to avoid issues like #3794 cache = this._translateBufferLineToStringWithWrap(row, true); if (this._linesCache) { this._linesCache[row] = cache; From 820dad31102492e093d10adc07add4dab87ea1f2 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 16 May 2022 11:24:51 -0700 Subject: [PATCH 2/7] onRenderedBufferChange => onRenderedViewportChange --- src/browser/Linkifier2.ts | 2 +- src/browser/Terminal.ts | 2 +- src/browser/TestUtils.test.ts | 2 +- src/browser/decorations/BufferDecorationRenderer.ts | 2 +- src/browser/decorations/OverviewRulerRenderer.ts | 2 +- src/browser/services/RenderService.ts | 6 +++--- src/browser/services/Services.ts | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier2.ts index 8954293660..acbf1c931c 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier2.ts @@ -303,7 +303,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { // Add listener for rerendering if (this._renderService) { - this._linkCacheDisposables.push(this._renderService.onRenderedBufferChange(e => { + this._linkCacheDisposables.push(this._renderService.onRenderedViewportChange(e => { // When start is 0 a scroll most likely occurred, make sure links above the fold also get // cleared. const start = e.start === 0 ? 0 : e.start + 1 + this._bufferService.buffer.ydisp; diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index de3fff9058..9ab3087c5f 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -526,7 +526,7 @@ export class Terminal extends CoreTerminal implements ITerminal { const renderer = this._createRenderer(); this._renderService = this.register(this._instantiationService.createInstance(RenderService, renderer, this.rows, this.screenElement)); this._instantiationService.setService(IRenderService, this._renderService); - this.register(this._renderService.onRenderedBufferChange(e => this._onRender.fire(e))); + this.register(this._renderService.onRenderedViewportChange(e => this._onRender.fire(e))); this.onResize(e => this._renderService!.resize(e.cols, e.rows)); this._compositionView = document.createElement('div'); diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index 85e2bb55b6..60091f8689 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -370,7 +370,7 @@ export class MockMouseService implements IMouseService { export class MockRenderService implements IRenderService { public serviceBrand: undefined; public onDimensionsChange: IEvent = new EventEmitter().event; - public onRenderedBufferChange: IEvent<{ start: number, end: number }, void> = new EventEmitter<{ start: number, end: number }>().event; + public onRenderedViewportChange: IEvent<{ start: number, end: number }, void> = new EventEmitter<{ start: number, end: number }>().event; public onRender: IEvent<{ start: number, end: number }, void> = new EventEmitter<{ start: number, end: number }>().event; public onRefreshRequest: IEvent<{ start: number, end: number}, void> = new EventEmitter<{ start: number, end: number }>().event; public dimensions: IRenderDimensions = { diff --git a/src/browser/decorations/BufferDecorationRenderer.ts b/src/browser/decorations/BufferDecorationRenderer.ts index ac3457f336..65947cdc48 100644 --- a/src/browser/decorations/BufferDecorationRenderer.ts +++ b/src/browser/decorations/BufferDecorationRenderer.ts @@ -27,7 +27,7 @@ export class BufferDecorationRenderer extends Disposable { this._container.classList.add('xterm-decoration-container'); this._screenElement.appendChild(this._container); - this.register(this._renderService.onRenderedBufferChange(() => this._queueRefresh())); + this.register(this._renderService.onRenderedViewportChange(() => this._queueRefresh())); this.register(this._renderService.onDimensionsChange(() => this._queueRefresh())); this.register(addDisposableDomListener(window, 'resize', () => this._queueRefresh())); this.register(this._bufferService.buffers.onBufferActivate(() => { diff --git a/src/browser/decorations/OverviewRulerRenderer.ts b/src/browser/decorations/OverviewRulerRenderer.ts index 39480ca2a4..f31409cab3 100644 --- a/src/browser/decorations/OverviewRulerRenderer.ts +++ b/src/browser/decorations/OverviewRulerRenderer.ts @@ -82,7 +82,7 @@ export class OverviewRulerRenderer extends Disposable { * and hide the canvas if the alt buffer is active */ private _registerBufferChangeListeners(): void { - this.register(this._renderService.onRenderedBufferChange(() => this._queueRefresh())); + this.register(this._renderService.onRenderedViewportChange(() => this._queueRefresh())); this.register(this._bufferService.buffers.onBufferActivate(() => { this._canvas!.style.display = this._bufferService.buffer === this._bufferService.buffers.alt ? 'none' : 'block'; })); diff --git a/src/browser/services/RenderService.ts b/src/browser/services/RenderService.ts index b2e619fed6..fba3195450 100644 --- a/src/browser/services/RenderService.ts +++ b/src/browser/services/RenderService.ts @@ -39,8 +39,8 @@ export class RenderService extends Disposable implements IRenderService { private _onDimensionsChange = new EventEmitter(); public get onDimensionsChange(): IEvent { return this._onDimensionsChange.event; } - private _onRenderedBufferChange = new EventEmitter<{ start: number, end: number }>(); - public get onRenderedBufferChange(): IEvent<{ start: number, end: number }> { return this._onRenderedBufferChange.event; } + private _onRenderedViewportChange = new EventEmitter<{ start: number, end: number }>(); + public get onRenderedViewportChange(): IEvent<{ start: number, end: number }> { return this._onRenderedViewportChange.event; } private _onRender = new EventEmitter<{ start: number, end: number }>(); public get onRender(): IEvent<{ start: number, end: number }> { return this._onRender.event; } private _onRefreshRequest = new EventEmitter<{ start: number, end: number }>(); @@ -131,7 +131,7 @@ export class RenderService extends Disposable implements IRenderService { // Fire render event only if it was not a redraw if (!this._isNextRenderRedrawOnly) { - this._onRenderedBufferChange.fire({ start, end }); + this._onRenderedViewportChange.fire({ start, end }); } this._onRender.fire({ start, end }); this._isNextRenderRedrawOnly = true; diff --git a/src/browser/services/Services.ts b/src/browser/services/Services.ts index 7191d0edd3..3e1e080411 100644 --- a/src/browser/services/Services.ts +++ b/src/browser/services/Services.ts @@ -49,7 +49,7 @@ export interface IRenderService extends IDisposable { * Fires when buffer changes are rendered. This does not fire when only cursor * or selections are rendered. */ - onRenderedBufferChange: IEvent<{ start: number, end: number }>; + onRenderedViewportChange: IEvent<{ start: number, end: number }>; /** * Fires on render */ From 1db5323844986f9ca8e7f0b48f515b15b423dbe2 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 16 May 2022 11:41:21 -0700 Subject: [PATCH 3/7] add onBufferContentsChange --- addons/xterm-addon-search/src/SearchAddon.ts | 2 +- src/browser/TestUtils.test.ts | 1 + src/browser/Types.d.ts | 1 + src/browser/public/Terminal.ts | 1 + src/common/CoreTerminal.ts | 3 +++ src/common/input/WriteBuffer.ts | 5 +++++ typings/xterm.d.ts | 6 ++++++ 7 files changed, 18 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index b870981698..9c45998d3e 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -77,7 +77,7 @@ export class SearchAddon implements ITerminalAddon { public activate(terminal: Terminal): void { this._terminal = terminal; - this._onDataDisposable = this._terminal.onData(() => this._updateMatches()); + this._onDataDisposable = this._terminal.onBufferContentsChange(() => this._updateMatches()); this._onResizeDisposable = this._terminal.onResize(() => this._updateMatches()); } diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index 60091f8689..1a7fa88bcf 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -30,6 +30,7 @@ export class MockTerminal implements ITerminal { public onBlur!: IEvent; public onFocus!: IEvent; public onA11yChar!: IEvent; + public onBufferContentsChange!: IEvent; public onA11yTab!: IEvent; public onCursorMove!: IEvent; public onLineFeed!: IEvent; diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index 0e83c213c2..7044458360 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -44,6 +44,7 @@ export interface IPublicTerminal extends IDisposable { onSelectionChange: IEvent; onRender: IEvent<{ start: number, end: number }>; onResize: IEvent<{ cols: number, rows: number }>; + onBufferContentsChange: IEvent; onTitleChange: IEvent; onBell: IEvent; blur(): void; diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 1acde934d8..942e7335e9 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -74,6 +74,7 @@ export class Terminal implements ITerminalApi { public get onScroll(): IEvent { return this._core.onScroll; } public get onSelectionChange(): IEvent { return this._core.onSelectionChange; } public get onTitleChange(): IEvent { return this._core.onTitleChange; } + public get onBufferContentsChange(): IEvent { return this._core.onBufferContentsChange; } public get element(): HTMLElement | undefined { return this._core.element; } public get parser(): IParser { diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 12b374c848..6be887fba5 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -68,6 +68,8 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { private _onResize = new EventEmitter<{ cols: number, rows: number }>(); public get onResize(): IEvent<{ cols: number, rows: number }> { return this._onResize.event; } protected _onScroll = new EventEmitter(); + public get onBufferContentsChange(): IEvent { return this._onBufferContentsChange.event; } + protected _onBufferContentsChange = new EventEmitter(); /** * Internally we track the source of the scroll but this is meaningless outside the library so * it's filtered out. @@ -138,6 +140,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { // Setup WriteBuffer this._writeBuffer = new WriteBuffer((data, promiseResult) => this._inputHandler.parse(data, promiseResult)); + this.register(forwardEvent( this._writeBuffer.onBufferContentsChange, this._onBufferContentsChange)); } public dispose(): void { diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index cc84c9abef..e269ccd935 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -4,6 +4,8 @@ * @license MIT */ +import { EventEmitter, IEvent } from 'common/EventEmitter'; + declare const setTimeout: (handler: () => void, timeout?: number) => void; /** @@ -44,6 +46,8 @@ export class WriteBuffer { private _bufferOffset = 0; private _isSyncWriting = false; private _syncCalls = 0; + public get onBufferContentsChange(): IEvent { return this._onBufferContentsChange.event; } + private _onBufferContentsChange = new EventEmitter(); constructor(private _action: (data: string | Uint8Array, promiseResult?: boolean) => void | Promise) { } @@ -220,5 +224,6 @@ export class WriteBuffer { this._pendingData = 0; this._bufferOffset = 0; } + this._onBufferContentsChange.fire(); } } diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 15ee4650ba..acaf2b0854 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -839,6 +839,12 @@ declare module 'xterm' { */ onRender: IEvent<{ start: number, end: number }>; + /** + * Adds an event listener for when a chunk of data has been + * processed in the write buffer. + */ + onBufferContentsChange: IEvent; + /** * Adds an event listener for when the terminal is resized. The event value * contains the new size. From 0b0d0d860a9a56ea44e10e63280e872e1c4f74c7 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 16 May 2022 12:16:20 -0700 Subject: [PATCH 4/7] remove check for empty string --- addons/xterm-addon-search/src/SearchAddon.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 9c45998d3e..53525056a7 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -498,10 +498,7 @@ export class SearchAddon implements ITerminalAddon { return this._findInLine(term, searchPosition, searchOptions); } let cache = this._linesCache?.[row]; - if (!cache || cache[0] === '') { - // sometimes the search refresh happens before the - // buffer has content for a given line, so if the cached - // line is empty, check it again to avoid issues like #3794 + if (!cache) { cache = this._translateBufferLineToStringWithWrap(row, true); if (this._linesCache) { this._linesCache[row] = cache; From 6b573b99d002dfd8522c0843131eb4e55e2af208 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Mon, 16 May 2022 16:08:02 -0700 Subject: [PATCH 5/7] Update src/common/CoreTerminal.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> --- src/common/CoreTerminal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 6be887fba5..4f866a9ff2 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -140,7 +140,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { // Setup WriteBuffer this._writeBuffer = new WriteBuffer((data, promiseResult) => this._inputHandler.parse(data, promiseResult)); - this.register(forwardEvent( this._writeBuffer.onBufferContentsChange, this._onBufferContentsChange)); + this.register(forwardEvent(this._writeBuffer.onBufferContentsChange, this._onBufferContentsChange)); } public dispose(): void { From 564ea1a08e008cdb440ca93187a3aae43e7c040e Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Mon, 16 May 2022 16:08:07 -0700 Subject: [PATCH 6/7] Update typings/xterm.d.ts Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> --- typings/xterm.d.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index acaf2b0854..4ebd0254bb 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -840,10 +840,15 @@ declare module 'xterm' { onRender: IEvent<{ start: number, end: number }>; /** - * Adds an event listener for when a chunk of data has been - * processed in the write buffer. - */ - onBufferContentsChange: IEvent; + * Adds an event listener for when data has been parsed by the terminal, + * after {@link write} is called. This event is useful to listen for any + * changes in the buffer. + * + * This fires at most once per frame, after data parsing completes. Note + * that this can fire when there are still writes pending if there is a lot + * of data. + */ + onWriteParsed: IEvent; /** * Adds an event listener for when the terminal is resized. The event value From 25e1d591b2b1d16638b24b37c1fe41a9e7dedacf Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 17 May 2022 07:08:16 -0700 Subject: [PATCH 7/7] Fix compile --- addons/xterm-addon-search/src/SearchAddon.ts | 2 +- addons/xterm-addon-web-links/test/tsconfig.json | 3 +-- src/browser/TestUtils.test.ts | 2 +- src/browser/Types.d.ts | 2 +- src/browser/public/Terminal.ts | 2 +- src/common/CoreTerminal.ts | 6 +++--- src/common/input/WriteBuffer.ts | 6 +++--- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 53525056a7..0f356e1446 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -77,7 +77,7 @@ export class SearchAddon implements ITerminalAddon { public activate(terminal: Terminal): void { this._terminal = terminal; - this._onDataDisposable = this._terminal.onBufferContentsChange(() => this._updateMatches()); + this._onDataDisposable = this._terminal.onWriteParsed(() => this._updateMatches()); this._onResizeDisposable = this._terminal.onResize(() => this._updateMatches()); } diff --git a/addons/xterm-addon-web-links/test/tsconfig.json b/addons/xterm-addon-web-links/test/tsconfig.json index 1c772984ff..9f4d23dfff 100644 --- a/addons/xterm-addon-web-links/test/tsconfig.json +++ b/addons/xterm-addon-web-links/test/tsconfig.json @@ -12,8 +12,7 @@ "strict": true, "types": [ "../../../node_modules/@types/mocha", - "../../../node_modules/@types/node", - "../../../out-test/api/TestUtils" + "../../../node_modules/@types/node" ] }, "include": [ diff --git a/src/browser/TestUtils.test.ts b/src/browser/TestUtils.test.ts index d4e24b5288..76407dcc72 100644 --- a/src/browser/TestUtils.test.ts +++ b/src/browser/TestUtils.test.ts @@ -30,7 +30,7 @@ export class MockTerminal implements ITerminal { public onBlur!: IEvent; public onFocus!: IEvent; public onA11yChar!: IEvent; - public onBufferContentsChange!: IEvent; + public onWriteParsed!: IEvent; public onA11yTab!: IEvent; public onCursorMove!: IEvent; public onLineFeed!: IEvent; diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index 7044458360..a294ca78f4 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -44,7 +44,7 @@ export interface IPublicTerminal extends IDisposable { onSelectionChange: IEvent; onRender: IEvent<{ start: number, end: number }>; onResize: IEvent<{ cols: number, rows: number }>; - onBufferContentsChange: IEvent; + onWriteParsed: IEvent; onTitleChange: IEvent; onBell: IEvent; blur(): void; diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 942e7335e9..187bd3b51b 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -74,7 +74,7 @@ export class Terminal implements ITerminalApi { public get onScroll(): IEvent { return this._core.onScroll; } public get onSelectionChange(): IEvent { return this._core.onSelectionChange; } public get onTitleChange(): IEvent { return this._core.onTitleChange; } - public get onBufferContentsChange(): IEvent { return this._core.onBufferContentsChange; } + public get onWriteParsed(): IEvent { return this._core.onWriteParsed; } public get element(): HTMLElement | undefined { return this._core.element; } public get parser(): IParser { diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index 4f866a9ff2..af9ec3f9ef 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -68,8 +68,8 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { private _onResize = new EventEmitter<{ cols: number, rows: number }>(); public get onResize(): IEvent<{ cols: number, rows: number }> { return this._onResize.event; } protected _onScroll = new EventEmitter(); - public get onBufferContentsChange(): IEvent { return this._onBufferContentsChange.event; } - protected _onBufferContentsChange = new EventEmitter(); + public get onWriteParsed(): IEvent { return this._onWriteParsed.event; } + protected _onWriteParsed = new EventEmitter(); /** * Internally we track the source of the scroll but this is meaningless outside the library so * it's filtered out. @@ -140,7 +140,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { // Setup WriteBuffer this._writeBuffer = new WriteBuffer((data, promiseResult) => this._inputHandler.parse(data, promiseResult)); - this.register(forwardEvent(this._writeBuffer.onBufferContentsChange, this._onBufferContentsChange)); + this.register(forwardEvent(this._writeBuffer.onWriteParsed, this._onWriteParsed)); } public dispose(): void { diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index e269ccd935..67fd751e99 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -46,8 +46,8 @@ export class WriteBuffer { private _bufferOffset = 0; private _isSyncWriting = false; private _syncCalls = 0; - public get onBufferContentsChange(): IEvent { return this._onBufferContentsChange.event; } - private _onBufferContentsChange = new EventEmitter(); + public get onWriteParsed(): IEvent { return this._onWriteParsed.event; } + private _onWriteParsed = new EventEmitter(); constructor(private _action: (data: string | Uint8Array, promiseResult?: boolean) => void | Promise) { } @@ -224,6 +224,6 @@ export class WriteBuffer { this._pendingData = 0; this._bufferOffset = 0; } - this._onBufferContentsChange.fire(); + this._onWriteParsed.fire(); } }