Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid passing circular callback into CoreService ctor #4301

Merged
merged 2 commits into from
Dec 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/common/CoreTerminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 => {
Expand Down
4 changes: 2 additions & 2 deletions src/common/InputHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
Expand Down Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions src/common/TestUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export class MockCoreService implements ICoreService {
public onData: IEvent<string> = new EventEmitter<string>().event;
public onUserInput: IEvent<void> = new EventEmitter<void>().event;
public onBinary: IEvent<string> = new EventEmitter<string>().event;
public onRequestScrollToBottom: IEvent<void> = new EventEmitter<void>().event;
public reset(): void { }
public triggerDataEvent(data: string, wasUserInput?: boolean): void { }
public triggerBinaryEvent(data: string): void { }
Expand Down
1 change: 0 additions & 1 deletion src/common/services/CoreService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ describe('CoreService', () => {

beforeEach(() => {
coreService = new CoreService(
() => {},
new MockBufferService(80, 30),
new MockLogService(),
new MockOptionsService());
Expand Down
11 changes: 3 additions & 8 deletions src/common/services/CoreService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>());
public readonly onData = this._onData.event;
private readonly _onUserInput = this.register(new EventEmitter<void>());
public readonly onUserInput = this._onUserInput.event;
private readonly _onBinary = this.register(new EventEmitter<string>());
public readonly onBinary = this._onBinary.event;
private readonly _onRequestScrollToBottom = this.register(new EventEmitter<void>());
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);
}
Expand All @@ -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 (wasUserInput && this._optionsService.rawOptions.scrollOnUserInput && buffer.ybase !== buffer.ydisp) {
this._scrollToBottom!();
this._onRequestScrollToBottom.fire();
}

// Fire onUserInput so listeners can react as well (eg. clear selection)
Expand Down
1 change: 1 addition & 0 deletions src/common/services/Services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export interface ICoreService {
readonly onData: IEvent<string>;
readonly onUserInput: IEvent<void>;
readonly onBinary: IEvent<string>;
readonly onRequestScrollToBottom: IEvent<void>;

reset(): void;

Expand Down