From 4245ce33b59f876a6c7403d0ab84a76c10bc2449 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 10 Nov 2021 10:17:14 +0000 Subject: [PATCH] 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;