Remove cols/rows from internal term.options

demo/client.ts

@@ -256,7 +256,6 @@ function createTerminal(): void {
   // fit is called within a setTimeout, cols and rows need this.
   setTimeout(() => {
     initOptions(term);
-    // TODO: Clean this up, opt-cols/rows doesn't exist anymore
     (<HTMLInputElement>document.getElementById(`opt-cols`)).value = term.cols;
     (<HTMLInputElement>document.getElementById(`opt-rows`)).value = term.rows;
     paddingElement.value = '0';
@@ -342,6 +341,8 @@ function initOptions(term: TerminalType): void {
   const options = Object.getOwnPropertyNames(term.options);
   const booleanOptions = [];
   const numberOptions = [
+    'cols',
+    'rows',
     'overviewRulerWidth'
   ];
   options.filter(o => blacklistedOptions.indexOf(o) === -1).forEach(o => {

src/browser/Terminal.test.ts

@@ -1399,20 +1399,4 @@ 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);
-    });
-  });
 });

src/browser/Terminal.ts

@@ -33,7 +33,7 @@ import * as Browser from 'common/Platform';
 import { addDisposableDomListener } from 'browser/Lifecycle';
 import * as Strings from 'browser/LocalizableStrings';
 import { AccessibilityManager } from './AccessibilityManager';
-import { ITheme, IMarker, IDisposable, ILinkProvider, IDecorationOptions, IDecoration } from 'xterm';
+import { ITheme, IMarker, IDisposable, ILinkProvider, IDecorationOptions, IDecoration, ITerminalInitOnlyOptions } from 'xterm';
 import { DomRenderer } from 'browser/renderer/dom/DomRenderer';
 import { KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource, IColorEvent, ColorIndex, ColorRequestType } from 'common/Types';
 import { evaluateKeyboardEvent } from 'common/input/Keyboard';
@@ -156,7 +156,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
    * @alias module:xterm/src/xterm
    */
   constructor(
-    options: Partial<ITerminalOptions> = {}
+    options: Partial<ITerminalOptions> & Partial<ITerminalInitOnlyOptions> = {}
   ) {
     super(options);
 
@@ -1289,8 +1289,6 @@ export class Terminal extends CoreTerminal implements ITerminal {
      * Since _setup handles a full terminal creation, we have to carry forward
      * a few things that should not reset.
      */
-    this.options.rows = this.rows;
-    this.options.cols = this.cols;
     const customKeyEventHandler = this._customKeyEventHandler;
 
     this._setup();

src/browser/public/Terminal.ts

@@ -14,47 +14,15 @@ 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.
- */
-const CONSTRUCTOR_ONLY_OPTIONS = ['cols', 'rows'];
-
 export class Terminal implements ITerminalApi {
   private _core: ITerminal;
   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 = { ... this._core.options };
-    const getter = (propName: string): any => {
-      return this._core.options[propName];
-    };
-    const setter = (propName: string, value: any): void => {
-      this._checkReadonlyOptions(propName);
-      this._core.options[propName] = value;
-    };
-
-    for (const propName in this._core.options) {
-      const desc = {
-        get: getter.bind(this, propName),
-        set: setter.bind(this, propName)
-      };
-      Object.defineProperty(this._publicOptions, propName, desc);
-    }
-  }
-
-  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 {
@@ -124,11 +92,11 @@ export class Terminal implements ITerminalApi {
     };
   }
   public get options(): ITerminalOptions {
-    return this._publicOptions;
+    return this._core.options;
   }
   public set options(options: ITerminalOptions) {
     for (const propName in options) {
-      this._publicOptions[propName] = options[propName];
+      this._core.options[propName] = options[propName];
     }
   }
   public blur(): void {

src/common/CoreTerminal.ts

@@ -27,7 +27,7 @@ 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 as IPublicTerminalOptions } 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';

src/common/services/BufferService.ts

@@ -36,8 +36,8 @@ export class BufferService extends Disposable implements IBufferService {
     @IOptionsService private _optionsService: IOptionsService
   ) {
     super();
-    this.cols = Math.max(_optionsService.rawOptions.cols || 0, MINIMUM_COLS);
-    this.rows = Math.max(_optionsService.rawOptions.rows || 0, MINIMUM_ROWS);
+    this.cols = Math.max(80 || 0, MINIMUM_COLS);
+    this.rows = Math.max(24 || 0, MINIMUM_ROWS);
     this.buffers = new BufferSet(_optionsService, this);
   }
 

src/common/services/OptionsService.test.ts

@@ -15,21 +15,21 @@ describe('OptionsService', () => {
     afterEach(() => {
       console.error = originalError;
     });
-    it('uses default value if invalid constructor option values passed for cols/rows', () => {
-      const optionsService = new OptionsService({ cols: undefined, rows: undefined });
-      assert.equal(optionsService.options.rows, DEFAULT_OPTIONS.rows);
-      assert.equal(optionsService.options.cols, DEFAULT_OPTIONS.cols);
+    it('uses default value if invalid constructor option values passed for lineHeight/cursorWidth', () => {
+      const optionsService = new OptionsService({ lineHeight: undefined, cursorWidth: undefined });
+      assert.equal(optionsService.options.lineHeight, DEFAULT_OPTIONS.lineHeight);
+      assert.equal(optionsService.options.cursorWidth, DEFAULT_OPTIONS.cursorWidth);
     });
     it('uses values from constructor option values if correctly passed', () => {
-      const optionsService = new OptionsService({ cols: 80, rows: 25 });
-      assert.equal(optionsService.options.rows, 25);
-      assert.equal(optionsService.options.cols, 80);
+      const optionsService = new OptionsService({ lineHeight: 2, cursorWidth: 2 });
+      assert.equal(optionsService.options.lineHeight, 2);
+      assert.equal(optionsService.options.cursorWidth, 2);
     });
     it('uses default value if invalid constructor option value passed', () => {
       assert.equal(new OptionsService({ tabStopWidth: 0 }).options.tabStopWidth, DEFAULT_OPTIONS.tabStopWidth);
     });
     it('object.keys return the correct number of options', () => {
-      const optionsService = new OptionsService({ cols: 80, rows: 25 });
+      const optionsService = new OptionsService({});
       assert.notEqual(Object.keys(optionsService.options).length, 0);
     });
   });

src/common/services/OptionsService.ts

@@ -9,8 +9,6 @@ import { isMac } from 'common/Platform';
 import { CursorStyle } from 'common/Types';
 
 export const DEFAULT_OPTIONS: Readonly<ITerminalOptions> = {
-  cols: 80,
-  rows: 24,
   cursorBlink: false,
   cursorStyle: 'block',
   cursorWidth: 1,

src/common/services/Services.ts

@@ -207,7 +207,6 @@ export interface ITerminalOptions {
   allowProposedApi: boolean;
   allowTransparency: boolean;
   altClickMovesCursor: boolean;
-  cols: number;
   convertEol: boolean;
   cursorBlink: boolean;
   cursorStyle: CursorStyle;
@@ -228,7 +227,6 @@ export interface ITerminalOptions {
   macOptionClickForcesSelection: boolean;
   minimumContrastRatio: number;
   rightClickSelectsWord: boolean;
-  rows: number;
   screenReaderMode: boolean;
   scrollback: number;
   scrollSensitivity: number;

src/headless/Terminal.ts

@@ -157,13 +157,6 @@ export class Terminal extends CoreTerminal {
    * using DECSTR (soft reset, CSI ! p) or RIS instead (hard reset, ESC c).
    */
   public reset(): void {
-    /**
-     * Since _setup handles a full terminal creation, we have to carry forward
-     * a few things that should not reset.
-     */
-    this.options.rows = this.rows;
-    this.options.cols = this.cols;
-
     this._setup();
     super.reset();
   }

src/headless/public/Terminal.test.ts

@@ -124,14 +124,10 @@ describe('Headless API Tests', function (): void {
       term = new Terminal(termOptions);
     });
     it('get options', () => {
-      const options: ITerminalOptions = term.options;
-      strictEqual(options.cols, 80);
-      strictEqual(options.rows, 24);
+      strictEqual(term.options.lineHeight, 1);
+      strictEqual(term.options.cursorWidth, 1);
     });
     it('set options', async () => {
-      const options: ITerminalOptions = term.options;
-      throws(() => options.cols = 40);
-      throws(() => options.rows = 20);
       term.options.scrollback = 1;
       strictEqual(term.options.scrollback, 1);
       term.options= {

src/headless/public/Terminal.ts

@@ -7,7 +7,7 @@ import { IEvent } from 'common/EventEmitter';
 import { BufferNamespaceApi } from 'common/public/BufferNamespaceApi';
 import { ParserApi } from 'common/public/ParserApi';
 import { UnicodeApi } from 'common/public/UnicodeApi';
-import { IBufferNamespace as IBufferNamespaceApi, IMarker, IModes, IParser, ITerminalAddon, IUnicodeHandling, Terminal as ITerminalApi } from 'xterm-headless';
+import { IBufferNamespace as IBufferNamespaceApi, IMarker, IModes, IParser, ITerminalAddon, ITerminalInitOnlyOptions, IUnicodeHandling, Terminal as ITerminalApi } from 'xterm-headless';
 import { Terminal as TerminalCore } from 'headless/Terminal';
 import { AddonManager } from 'common/public/AddonManager';
 import { ITerminalOptions } from 'common/Types';
@@ -24,7 +24,7 @@ export class Terminal implements ITerminalApi {
   private _buffer: BufferNamespaceApi | undefined;
   private _publicOptions: ITerminalOptions;
 
-  constructor(options?: ITerminalOptions) {
+  constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions) {
     this._core = new TerminalCore(options);
     this._addonManager = new AddonManager();
 

test/api/Terminal.api.ts

@@ -157,19 +157,11 @@ describe('API Integration Tests', function(): void {
   describe('options', () => {
     it('getter', async () => {
       await openTerminal(page);
-      assert.equal(await page.evaluate(`window.term.options.cols`), 80);
-      assert.equal(await page.evaluate(`window.term.options.rows`), 24);
+      assert.equal(await page.evaluate(`window.term.lineHeight`), 1);
+      assert.equal(await page.evaluate(`window.term.cursorWidth`), 1);
     });
     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(`

typings/xterm-headless.d.ts

@@ -48,11 +48,6 @@ declare module 'xterm-headless' {
      */
     convertEol?: boolean;
 
-    /**
-     * The number of columns in the terminal.
-     */
-    cols?: number;
-
     /**
      * Whether the cursor blinks.
      */
@@ -145,11 +140,6 @@ declare module 'xterm-headless' {
      */
     rightClickSelectsWord?: boolean;
 
-    /**
-     * The number of rows in the terminal.
-     */
-    rows?: number;
-
     /**
      * Whether screen reader support is enabled. When on this will expose
      * supporting elements in the DOM to support NVDA on Windows and VoiceOver
@@ -210,6 +200,22 @@ declare module 'xterm-headless' {
     windowOptions?: IWindowOptions;
   }
 
+  /**
+   * An object containing additional options for the terminal that can only be
+   * set on start up.
+   */
+  export interface ITerminalInitOnlyOptions {
+    /**
+     * The number of columns in the terminal.
+     */
+    cols?: number;
+
+    /**
+     * The number of rows in the terminal.
+     */
+    rows?: number;
+  }
+
   /**
    * Contains colors to theme the terminal with.
    */
@@ -558,7 +564,7 @@ declare module 'xterm-headless' {
      *
      * @param options An object containing a set of options.
      */
-    constructor(options?: ITerminalOptions);
+    constructor(options?: ITerminalOptions & ITerminalInitOnlyOptions);
 
     /**
      * Adds an event listener for when the bell is triggered.