From aa046b73fb1bb269023aaabf3d2b4c93adf8b049 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Wed, 1 Sep 2021 13:34:27 +0000 Subject: [PATCH 1/2] Handle undefined rows or cols better --- src/common/services/BufferService.ts | 4 ++-- src/common/services/OptionsService.test.ts | 14 ++++++++++++-- src/common/services/OptionsService.ts | 9 +++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/common/services/BufferService.ts b/src/common/services/BufferService.ts index 99594d22..c8c5f273 100644 --- a/src/common/services/BufferService.ts +++ b/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); } diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index c289b5be..e140b5b4 100644 --- a/src/common/services/OptionsService.test.ts +++ b/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', () => { diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 5add8283..d7ac1411 100644 --- a/src/common/services/OptionsService.ts +++ b/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; From fa778257b9c7f1fb973e3bd63fa1fea01bc3fd14 Mon Sep 17 00:00:00 2001 From: Simon Lamon Date: Fri, 3 Sep 2021 04:40:29 +0000 Subject: [PATCH 2/2] Formatting --- src/common/services/OptionsService.test.ts | 4 ++-- src/common/services/OptionsService.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index e140b5b4..8675b6b9 100644 --- a/src/common/services/OptionsService.test.ts +++ b/src/common/services/OptionsService.test.ts @@ -10,7 +10,7 @@ describe('OptionsService', () => { describe('constructor', () => { const originalError = console.error; beforeEach(() => { - console.error = () => { }; + console.error = () => {}; }); afterEach(() => { console.error = originalError; @@ -26,7 +26,7 @@ describe('OptionsService', () => { 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', () => { diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index d7ac1411..e9dcaa6a 100644 --- a/src/common/services/OptionsService.ts +++ b/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) {