From d6b8a21219fe3f94818b4e072dcfb73a7a068b02 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Sun, 11 Dec 2022 07:35:11 -0800 Subject: [PATCH] Avoid passing circular callback into CoreService ctor --- src/common/CoreTerminal.ts | 3 ++- src/common/InputHandler.test.ts | 4 ++-- src/common/TestUtils.test.ts | 1 + src/common/services/CoreService.test.ts | 1 - src/common/services/CoreService.ts | 11 +++-------- src/common/services/Services.ts | 1 + 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts index c5182554..33637421 100644 --- a/src/common/CoreTerminal.ts +++ b/src/common/CoreTerminal.ts @@ -109,7 +109,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this._instantiationService.setService(IBufferService, this._bufferService); this._logService = this.register(this._instantiationService.createInstance(LogService)); this._instantiationService.setService(ILogService, this._logService); - this.coreService = this.register(this._instantiationService.createInstance(CoreService, () => this.scrollToBottom())); + this.coreService = this.register(this._instantiationService.createInstance(CoreService)); this._instantiationService.setService(ICoreService, this.coreService); this.coreMouseService = this.register(this._instantiationService.createInstance(CoreMouseService)); this._instantiationService.setService(ICoreMouseService, this.coreMouseService); @@ -129,6 +129,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal { this.register(forwardEvent(this._bufferService.onResize, this._onResize)); this.register(forwardEvent(this.coreService.onData, this._onData)); this.register(forwardEvent(this.coreService.onBinary, this._onBinary)); + this.register(this.coreService.onRequestScrollToBottom(() => this.scrollToBottom())); this.register(this.coreService.onUserInput(() => this._writeBuffer.handleUserInput())); this.register(this.optionsService.onSpecificOptionChange('windowsMode', e => this._handleWindowsModeOptionChange(e))); this.register(this._bufferService.onScroll(event => { diff --git a/src/common/InputHandler.test.ts b/src/common/InputHandler.test.ts index 110e4f51..858019ff 100644 --- a/src/common/InputHandler.test.ts +++ b/src/common/InputHandler.test.ts @@ -65,7 +65,7 @@ describe('InputHandler', () => { optionsService = new MockOptionsService(); bufferService = new BufferService(optionsService); bufferService.resize(80, 30); - coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService); + coreService = new CoreService(bufferService, new MockLogService(), optionsService); inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); }); @@ -2300,7 +2300,7 @@ describe('InputHandler - async handlers', () => { optionsService = new MockOptionsService(); bufferService = new BufferService(optionsService); bufferService.resize(80, 30); - coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService); + coreService = new CoreService(bufferService, new MockLogService(), optionsService); coreService.onData(data => { console.log(data); }); inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService()); diff --git a/src/common/TestUtils.test.ts b/src/common/TestUtils.test.ts index 3aa0f694..cae90ec8 100644 --- a/src/common/TestUtils.test.ts +++ b/src/common/TestUtils.test.ts @@ -95,6 +95,7 @@ export class MockCoreService implements ICoreService { public onData: IEvent = new EventEmitter().event; public onUserInput: IEvent = new EventEmitter().event; public onBinary: IEvent = new EventEmitter().event; + public onRequestScrollToBottom: IEvent = new EventEmitter().event; public reset(): void { } public triggerDataEvent(data: string, wasUserInput?: boolean): void { } public triggerBinaryEvent(data: string): void { } diff --git a/src/common/services/CoreService.test.ts b/src/common/services/CoreService.test.ts index 00be4953..44018494 100644 --- a/src/common/services/CoreService.test.ts +++ b/src/common/services/CoreService.test.ts @@ -13,7 +13,6 @@ describe('CoreService', () => { beforeEach(() => { coreService = new CoreService( - () => {}, new MockBufferService(80, 30), new MockLogService(), new MockOptionsService()); diff --git a/src/common/services/CoreService.ts b/src/common/services/CoreService.ts index 9282197b..2c5d5706 100644 --- a/src/common/services/CoreService.ts +++ b/src/common/services/CoreService.ts @@ -31,26 +31,21 @@ export class CoreService extends Disposable implements ICoreService { public modes: IModes; public decPrivateModes: IDecPrivateModes; - // Circular dependency, this must be unset or memory will leak after Terminal.dispose - private _scrollToBottom: (() => void) | undefined; - private readonly _onData = this.register(new EventEmitter()); public readonly onData = this._onData.event; private readonly _onUserInput = this.register(new EventEmitter()); public readonly onUserInput = this._onUserInput.event; private readonly _onBinary = this.register(new EventEmitter()); public readonly onBinary = this._onBinary.event; + private readonly _onRequestScrollToBottom = this.register(new EventEmitter()); + public readonly onRequestScrollToBottom = this._onRequestScrollToBottom.event; constructor( - // TODO: Move this into a service - scrollToBottom: () => void, @IBufferService private readonly _bufferService: IBufferService, @ILogService private readonly _logService: ILogService, @IOptionsService private readonly _optionsService: IOptionsService ) { super(); - this._scrollToBottom = scrollToBottom; - this.register({ dispose: () => this._scrollToBottom = undefined }); this.modes = clone(DEFAULT_MODES); this.decPrivateModes = clone(DEFAULT_DEC_PRIVATE_MODES); } @@ -69,7 +64,7 @@ export class CoreService extends Disposable implements ICoreService { // Input is being sent to the terminal, the terminal should focus the prompt. const buffer = this._bufferService.buffer; if (buffer.ybase !== buffer.ydisp) { - this._scrollToBottom!(); + this._onRequestScrollToBottom.fire(); } // Fire onUserInput so listeners can react as well (eg. clear selection) diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index cc388063..3648b338 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -79,6 +79,7 @@ export interface ICoreService { readonly onData: IEvent; readonly onUserInput: IEvent; readonly onBinary: IEvent; + readonly onRequestScrollToBottom: IEvent; reset(): void;