Handle undefined rows or cols better

Author: silamon

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.options.cols, MINIMUM_COLS);
-    this.rows = Math.max(_optionsService.options.rows, MINIMUM_ROWS);
+    this.cols = Math.max(_optionsService.options.cols || 0, MINIMUM_COLS);
+    this.rows = Math.max(_optionsService.options.rows || 0, MINIMUM_ROWS);
     this.buffers = new BufferSet(_optionsService, this);
   }
 

src/common/services/OptionsService.test.ts

@@ -10,13 +10,23 @@ describe('OptionsService', () => {
   describe('constructor', () => {
     const originalError = console.error;
     beforeEach(() => {
-      console.error = () => {};
+      console.error = () => { };
     });
     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.getOption('rows'), DEFAULT_OPTIONS.rows);
+      assert.equal(optionsService.getOption('cols'), DEFAULT_OPTIONS.cols);
+    });
+    it('uses values from constructor option values if correctly passed', () => {
+      const optionsService = new OptionsService({ cols: 80, rows: 25 });
+      assert.equal(optionsService.getOption('rows'), 25);
+      assert.equal(optionsService.getOption('cols'), 80);
+    });
     it('uses default value if invalid constructor option value passed', () => {
-      assert.equal(new OptionsService({tabStopWidth: 0}).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
+      assert.equal(new OptionsService({ tabStopWidth: 0 }).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
     });
   });
   describe('setOption', () => {

src/common/services/OptionsService.ts

@@ -22,7 +22,7 @@ export const DEFAULT_OPTIONS: ITerminalOptions = Object.freeze({
   cursorStyle: 'block',
   cursorWidth: 1,
   customGlyphs: true,
-  bellSound:  DEFAULT_BELL_SOUND,
+  bellSound: DEFAULT_BELL_SOUND,
   bellStyle: 'none',
   drawBoldTextInBrightColors: true,
   fastScrollModifier: 'alt',
@@ -128,7 +128,7 @@ export class OptionsService implements IOptionsService {
         break;
       case 'cursorWidth':
         value = Math.floor(value);
-        // Fall through for bounds check
+      // Fall through for bounds check
       case 'lineHeight':
       case 'tabStopWidth':
         if (value < 1) {
@@ -149,6 +149,11 @@ export class OptionsService implements IOptionsService {
         if (value <= 0) {
           throw new Error(`${key} cannot be less than or equal to 0, value: ${value}`);
         }
+      case 'rows':
+      case 'cols':
+        if (!value && value !== 0) {
+          throw new Error(`${key} must be numeric, value: ${value}`);
+        }
         break;
     }
     return value;