From f0e3159ecec690c5ffa4613c5e4add49914e8ec4 Mon Sep 17 00:00:00 2001 From: IllusionMH Date: Mon, 31 Aug 2020 03:35:22 +0300 Subject: [PATCH 1/4] Add number type to fontWeight & fontWeightBold options --- src/browser/public/Terminal.ts | 5 +++-- src/common/services/OptionsService.ts | 14 +++++++++++--- src/common/services/Services.ts | 2 +- typings/xterm.d.ts | 13 +++++++++---- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index bd95599f96..4b6307a561 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -169,15 +169,16 @@ export class Terminal implements ITerminalApi { public paste(data: string): void { this._core.paste(data); } - public getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'fontWeight' | 'fontWeightBold' | 'logLevel' | 'rendererType' | 'termName' | 'wordSeparator'): string; + public getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'logLevel' | 'rendererType' | 'termName' | 'wordSeparator'): string; public getOption(key: 'allowTransparency' | 'cancelEvents' | 'convertEol' | 'cursorBlink' | 'disableStdin' | 'macOptionIsMeta' | 'rightClickSelectsWord' | 'popOnBell' | 'visualBell'): boolean; public getOption(key: 'cols' | 'fontSize' | 'letterSpacing' | 'lineHeight' | 'rows' | 'tabStopWidth' | 'scrollback'): number; + public getOption(key: 'fontWeight' | 'fontWeightBold'): string | number; public getOption(key: string): any; public getOption(key: any): any { return this._core.optionsService.getOption(key); } public setOption(key: 'bellSound' | 'fontFamily' | 'termName' | 'wordSeparator', value: string): void; - public setOption(key: 'fontWeight' | 'fontWeightBold', value: 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'): void; + public setOption(key: 'fontWeight' | 'fontWeightBold', value: 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number): void; public setOption(key: 'logLevel', value: 'debug' | 'info' | 'warn' | 'error' | 'off'): void; public setOption(key: 'bellStyle', value: 'none' | 'visual' | 'sound' | 'both'): void; public setOption(key: 'cursorStyle', value: 'block' | 'underline' | 'bar'): void; diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 2f01092382..e5f6eaa8dc 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { IOptionsService, ITerminalOptions, IPartialTerminalOptions } from 'common/services/Services'; +import { IOptionsService, ITerminalOptions, IPartialTerminalOptions, FontWeight } from 'common/services/Services'; import { EventEmitter, IEvent } from 'common/EventEmitter'; import { isMac } from 'common/Platform'; import { clone } from 'common/Clone'; @@ -56,6 +56,8 @@ export const DEFAULT_OPTIONS: ITerminalOptions = Object.freeze({ cancelEvents: false }); +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. */ @@ -109,14 +111,20 @@ export class OptionsService implements IOptionsService { switch (key) { case 'bellStyle': case 'cursorStyle': - case 'fontWeight': - case 'fontWeightBold': case 'rendererType': case 'wordSeparator': if (!value) { value = DEFAULT_OPTIONS[key]; } break; + case 'fontWeight': + case 'fontWeightBold': + if (typeof value === 'number' && 1 <= value && value <= 1000) { + // already valid numeric value + break; + } + value = FONT_WEIGHT_OPTIONS.indexOf(value) !== -1 ? value : DEFAULT_OPTIONS[key]; + break; case 'cursorWidth': value = Math.floor(value); // Fall through for bounds check diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 19a34d19f5..0ea7a30804 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -179,7 +179,7 @@ export interface IOptionsService { getOption(key: string): T | undefined; } -export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'; +export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; export type LogLevel = 'debug' | 'info' | 'warn' | 'error' | 'off'; export type RendererType = 'dom' | 'canvas'; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index dd934e358f..4c026b3e8e 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -11,9 +11,9 @@ declare module 'xterm' { /** - * A string representing text font weight. + * A string or number representing text font weight. */ - export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'; + export type FontWeight = 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number; /** * A string representing log level. @@ -933,7 +933,7 @@ declare module 'xterm' { * Retrieves an option's value from the terminal. * @param key The option key. */ - getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'fontWeight' | 'fontWeightBold' | 'logLevel' | 'rendererType' | 'termName' | 'wordSeparator'): string; + getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'logLevel' | 'rendererType' | 'termName' | 'wordSeparator'): string; /** * Retrieves an option's value from the terminal. * @param key The option key. @@ -944,6 +944,11 @@ declare module 'xterm' { * @param key The option key. */ getOption(key: 'cols' | 'fontSize' | 'letterSpacing' | 'lineHeight' | 'rows' | 'tabStopWidth' | 'scrollback'): number; + /** + * Retrieves an option's value from the terminal. + * @param key The option key. + */ + getOption(key: 'fontWeight' | 'fontWeightBold'): string | number; /** * Retrieves an option's value from the terminal. * @param key The option key. @@ -961,7 +966,7 @@ declare module 'xterm' { * @param key The option key. * @param value The option value. */ - setOption(key: 'fontWeight' | 'fontWeightBold', value: null | 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'): void; + setOption(key: 'fontWeight' | 'fontWeightBold', value: null | 'normal' | 'bold' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900' | number): void; /** * Sets an option on the terminal. * @param key The option key. From b918f0d34acfc214c7a7fdced7afdc6302bc1281 Mon Sep 17 00:00:00 2001 From: IllusionMH Date: Fri, 4 Sep 2020 23:35:38 +0300 Subject: [PATCH 2/4] Use FontWeight as return type --- src/browser/public/Terminal.ts | 4 ++-- typings/xterm.d.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 4b6307a561..33aa90246e 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal as ITerminalApi, ITerminalOptions, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBuffer as IBufferApi, IBufferNamespace as IBufferNamespaceApi, IBufferLine as IBufferLineApi, IBufferCell as IBufferCellApi, IParser, IFunctionIdentifier, ILinkProvider, IUnicodeHandling, IUnicodeVersionProvider } from 'xterm'; +import { Terminal as ITerminalApi, ITerminalOptions, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings, ITerminalAddon, ISelectionPosition, IBuffer as IBufferApi, IBufferNamespace as IBufferNamespaceApi, IBufferLine as IBufferLineApi, IBufferCell as IBufferCellApi, IParser, IFunctionIdentifier, ILinkProvider, IUnicodeHandling, IUnicodeVersionProvider, FontWeight } from 'xterm'; import { ITerminal } from 'browser/Types'; import { IBufferLine, ICellData } from 'common/Types'; import { IBuffer, IBufferSet } from 'common/buffer/Types'; @@ -172,7 +172,7 @@ export class Terminal implements ITerminalApi { public getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'logLevel' | 'rendererType' | 'termName' | 'wordSeparator'): string; public getOption(key: 'allowTransparency' | 'cancelEvents' | 'convertEol' | 'cursorBlink' | 'disableStdin' | 'macOptionIsMeta' | 'rightClickSelectsWord' | 'popOnBell' | 'visualBell'): boolean; public getOption(key: 'cols' | 'fontSize' | 'letterSpacing' | 'lineHeight' | 'rows' | 'tabStopWidth' | 'scrollback'): number; - public getOption(key: 'fontWeight' | 'fontWeightBold'): string | number; + public getOption(key: 'fontWeight' | 'fontWeightBold'): FontWeight; public getOption(key: string): any; public getOption(key: any): any { return this._core.optionsService.getOption(key); diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 4c026b3e8e..d6ed15482e 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -948,7 +948,7 @@ declare module 'xterm' { * Retrieves an option's value from the terminal. * @param key The option key. */ - getOption(key: 'fontWeight' | 'fontWeightBold'): string | number; + getOption(key: 'fontWeight' | 'fontWeightBold'): FontWeight; /** * Retrieves an option's value from the terminal. * @param key The option key. From 3e0f0331b3cdb0ed10d5a02ee341ed11314e2b02 Mon Sep 17 00:00:00 2001 From: IllusionMH Date: Sat, 5 Sep 2020 00:48:12 +0300 Subject: [PATCH 3/4] Add tests for fontWeight normalization --- src/common/services/OptionsService.test.ts | 40 +++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index 59c17faf9e..a42220fec6 100644 --- a/src/common/services/OptionsService.test.ts +++ b/src/common/services/OptionsService.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert } from 'chai'; +import { assert, expect } from 'chai'; import { OptionsService, DEFAULT_OPTIONS } from 'common/services/OptionsService'; describe('OptionsService', () => { @@ -19,4 +19,42 @@ describe('OptionsService', () => { assert.equal(new OptionsService({tabStopWidth: 0}).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth); }); }); + describe('setOption', () => { + let service: OptionsService; + beforeEach(() => { + service = new OptionsService({}); + }); + it('applies valid fontWeight option values', () => { + service.setOption('fontWeight', 'bold'); + assert.equal(service.getOption('fontWeight'), 'bold', '"bold" keyword value should be applied'); + + service.setOption('fontWeight', 'normal'); + assert.equal(service.getOption('fontWeight'), 'normal', '"normal" keyword value should be applied'); + + service.setOption('fontWeight', '600'); + assert.equal(service.getOption('fontWeight'), '600', 'String numeric values should be applied'); + + service.setOption('fontWeight', 350); + assert.equal(service.getOption('fontWeight'), 350, 'Values between 1 and 1000 should be applied as is'); + + service.setOption('fontWeight', 1); + assert.equal(service.getOption('fontWeight'), 1, 'Range should include minimum value: 1'); + + service.setOption('fontWeight', 1000); + assert.equal(service.getOption('fontWeight'), 1000, 'Range should include maximum value: 1000'); + }); + it('normalizes invalid fontWeight option values', () => { + service.setOption('fontWeight', 350); + expect(() => service.setOption('fontWeight', 10000)).to.not.throw('', 'fontWeight should be normalized instead of throwing'); + assert.equal(service.getOption('fontWeight'), DEFAULT_OPTIONS.fontWeight, 'Values greater than 1000 should be reset to default'); + + service.setOption('fontWeight', 350); + service.setOption('fontWeight', -10); + assert.equal(service.getOption('fontWeight'), DEFAULT_OPTIONS.fontWeight, 'Values less than 1 should be reset to default'); + + service.setOption('fontWeight', 350); + service.setOption('fontWeight', 'bold700'); + assert.equal(service.getOption('fontWeight'), DEFAULT_OPTIONS.fontWeight, 'Wrong string literals should be reset to default'); + }); + }); }); From 4975f389ffb2b5543c6f8ea2f4a0abebba8dd6b6 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 8 Sep 2020 06:18:37 -0700 Subject: [PATCH 4/4] Use assert over expect --- src/common/services/OptionsService.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/services/OptionsService.test.ts b/src/common/services/OptionsService.test.ts index a42220fec6..c289b5be1d 100644 --- a/src/common/services/OptionsService.test.ts +++ b/src/common/services/OptionsService.test.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { assert, expect } from 'chai'; +import { assert } from 'chai'; import { OptionsService, DEFAULT_OPTIONS } from 'common/services/OptionsService'; describe('OptionsService', () => { @@ -45,7 +45,7 @@ describe('OptionsService', () => { }); it('normalizes invalid fontWeight option values', () => { service.setOption('fontWeight', 350); - expect(() => service.setOption('fontWeight', 10000)).to.not.throw('', 'fontWeight should be normalized instead of throwing'); + assert.doesNotThrow(() => service.setOption('fontWeight', 10000), 'fontWeight should be normalized instead of throwing'); assert.equal(service.getOption('fontWeight'), DEFAULT_OPTIONS.fontWeight, 'Values greater than 1000 should be reset to default'); service.setOption('fontWeight', 350);