Skip to content

Add IntRange type for ColorIndex #4618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 30, 2023
Merged

Add IntRange type for ColorIndex #4618

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79c1fff
Add IntRange type for ColorIndex
Tyriar Jul 28, 2023
5660c40
Differentiate special color indexes and others
Tyriar Jul 28, 2023
d22e3e0
Fix compile
Tyriar Jul 29, 2023
2e63859
Simplify condition
Tyriar Jul 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/browser/Terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import * as Strings from 'browser/LocalizableStrings';
import { AccessibilityManager } from './AccessibilityManager';
import { ITheme, IMarker, IDisposable, ILinkProvider, IDecorationOptions, IDecoration } from 'xterm';
import { DomRenderer } from 'browser/renderer/dom/DomRenderer';
import { KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource, IColorEvent, ColorIndex, ColorRequestType } from 'common/Types';
import { KeyboardResultType, CoreMouseEventType, CoreMouseButton, CoreMouseAction, ITerminalOptions, ScrollSource, IColorEvent, ColorIndex, ColorRequestType, SpecialColorIndex } from 'common/Types';
import { evaluateKeyboardEvent } from 'common/input/Keyboard';
import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter';
import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine';
Expand Down Expand Up @@ -203,15 +203,15 @@ export class Terminal extends CoreTerminal implements ITerminal {
let acc: 'foreground' | 'background' | 'cursor' | 'ansi';
let ident = '';
switch (req.index) {
case ColorIndex.FOREGROUND: // OSC 10 | 110
case SpecialColorIndex.FOREGROUND: // OSC 10 | 110
acc = 'foreground';
ident = '10';
break;
case ColorIndex.BACKGROUND: // OSC 11 | 111
case SpecialColorIndex.BACKGROUND: // OSC 11 | 111
acc = 'background';
ident = '11';
break;
case ColorIndex.CURSOR: // OSC 12 | 112
case SpecialColorIndex.CURSOR: // OSC 12 | 112
acc = 'cursor';
ident = '12';
break;
Expand Down
5 changes: 2 additions & 3 deletions src/browser/services/Services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import { IRenderDimensions, IRenderer } from 'browser/renderer/shared/Types';
import { IColorSet, ReadonlyColorSet } from 'browser/Types';
import { ISelectionRedrawRequestEvent as ISelectionRequestRedrawEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types';
import { createDecorator } from 'common/services/ServiceRegistry';
import { ColorIndex, IDisposable } from 'common/Types';
import { ITheme } from 'common/services/Services';
import { AllColorIndex, IDisposable } from 'common/Types';

export const ICharSizeService = createDecorator<ICharSizeService>('CharSizeService');
export interface ICharSizeService {
Expand Down Expand Up @@ -130,7 +129,7 @@ export interface IThemeService {

readonly onChangeColors: IEvent<ReadonlyColorSet>;

restoreColor(slot?: ColorIndex): void;
restoreColor(slot?: AllColorIndex): void;
/**
* Allows external modifying of colors in the theme, this is used instead of {@link colors} to
* prevent accidental writes.
Expand Down
12 changes: 6 additions & 6 deletions src/browser/services/ThemeService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { channels, color, css, NULL_COLOR } from 'common/Color';
import { EventEmitter } from 'common/EventEmitter';
import { Disposable } from 'common/Lifecycle';
import { IOptionsService, ITheme } from 'common/services/Services';
import { ColorIndex, IColor } from 'common/Types';
import { ColorIndex, SpecialColorIndex, IColor, AllColorIndex } from 'common/Types';

interface IRestoreColorSet {
foreground: IColor;
Expand Down Expand Up @@ -176,12 +176,12 @@ export class ThemeService extends Disposable implements IThemeService {
this._onChangeColors.fire(this.colors);
}

public restoreColor(slot?: ColorIndex): void {
public restoreColor(slot?: AllColorIndex): void {
this._restoreColor(slot);
this._onChangeColors.fire(this.colors);
}

private _restoreColor(slot: ColorIndex | undefined): void {
private _restoreColor(slot: AllColorIndex | undefined): void {
// unset slot restores all ansi colors
if (slot === undefined) {
for (let i = 0; i < this._restoreColors.ansi.length; ++i) {
Expand All @@ -190,13 +190,13 @@ export class ThemeService extends Disposable implements IThemeService {
return;
}
switch (slot) {
case ColorIndex.FOREGROUND:
case SpecialColorIndex.FOREGROUND:
this._colors.foreground = this._restoreColors.foreground;
break;
case ColorIndex.BACKGROUND:
case SpecialColorIndex.BACKGROUND:
this._colors.background = this._restoreColors.background;
break;
case ColorIndex.CURSOR:
case SpecialColorIndex.CURSOR:
this._colors.cursor = this._restoreColors.cursor;
break;
default:
Expand Down
60 changes: 30 additions & 30 deletions src/common/InputHandler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { assert } from 'chai';
import { InputHandler } from 'common/InputHandler';
import { IBufferLine, IAttributeData, IColorEvent, ColorIndex, ColorRequestType } from 'common/Types';
import { IBufferLine, IAttributeData, IColorEvent, ColorIndex, ColorRequestType, SpecialColorIndex } from 'common/Types';
import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine';
import { CellData } from 'common/buffer/CellData';
import { Attributes, BgFlags, UnderlineStyle } from 'common/buffer/Constants';
Expand Down Expand Up @@ -1950,47 +1950,47 @@ describe('InputHandler', () => {
inputHandler.onColor(ev => stack.push(ev));
// single color query
await inputHandler.parseP('\x1b]4;0;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 0 as ColorIndex }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 0 }]]);
stack.length = 0;
await inputHandler.parseP('\x1b]4;123;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 123 as ColorIndex }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 123 }]]);
stack.length = 0;
// multiple queries
await inputHandler.parseP('\x1b]4;0;?;123;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 0 as ColorIndex }, { type: ColorRequestType.REPORT, index: 123 as ColorIndex }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: 0 }, { type: ColorRequestType.REPORT, index: 123 }]]);
stack.length = 0;
});
it('4: set color events', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
// single color query
await inputHandler.parseP('\x1b]4;0;rgb:01/02/03\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0 as ColorIndex, color: [1, 2, 3] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0, color: [1, 2, 3] }]]);
stack.length = 0;
await inputHandler.parseP('\x1b]4;123;#aabbcc\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 123 as ColorIndex, color: [170, 187, 204] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 123, color: [170, 187, 204] }]]);
stack.length = 0;
// multiple queries
await inputHandler.parseP('\x1b]4;0;rgb:aa/bb/cc;123;#001122\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0 as ColorIndex, color: [170, 187, 204] }, { type: ColorRequestType.SET, index: 123 as ColorIndex, color: [0, 17, 34] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0, color: [170, 187, 204] }, { type: ColorRequestType.SET, index: 123, color: [0, 17, 34] }]]);
stack.length = 0;
});
it('4: should ignore invalid values', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
await inputHandler.parseP('\x1b]4;0;rgb:aa/bb/cc;45;rgb:1/22/333;123;#001122\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0 as ColorIndex, color: [170, 187, 204] }, { type: ColorRequestType.SET, index: 123 as ColorIndex, color: [0, 17, 34] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: 0, color: [170, 187, 204] }, { type: ColorRequestType.SET, index: 123, color: [0, 17, 34] }]]);
stack.length = 0;
});
it('104: restore events', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
await inputHandler.parseP('\x1b]104;0\x07\x1b]104;43\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: 0 as ColorIndex }], [{ type: ColorRequestType.RESTORE, index: 43 as ColorIndex }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: 0 }], [{ type: ColorRequestType.RESTORE, index: 43 }]]);
stack.length = 0;
// multiple in one command
await inputHandler.parseP('\x1b]104;0;43\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: 0 as ColorIndex }, { type: ColorRequestType.RESTORE, index: 43 as ColorIndex }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: 0 }, { type: ColorRequestType.RESTORE, index: 43 }]]);
stack.length = 0;
// full ANSI table restore
await inputHandler.parseP('\x1b]104\x07');
Expand All @@ -2002,87 +2002,87 @@ describe('InputHandler', () => {
inputHandler.onColor(ev => stack.push(ev));
// single foreground query --> color undefined
await inputHandler.parseP('\x1b]10;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.FOREGROUND }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.FOREGROUND }]]);
stack.length = 0;
// OSC with multiple values maps to OSC 10 & OSC 11 & OSC 12
await inputHandler.parseP('\x1b]10;?;?;?;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.FOREGROUND }], [{ type: ColorRequestType.REPORT, index: ColorIndex.BACKGROUND }], [{ type: ColorRequestType.REPORT, index: ColorIndex.CURSOR }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.FOREGROUND }], [{ type: ColorRequestType.REPORT, index: SpecialColorIndex.BACKGROUND }], [{ type: ColorRequestType.REPORT, index: SpecialColorIndex.CURSOR }]]);
stack.length = 0;
// set foreground color events
await inputHandler.parseP('\x1b]10;rgb:01/02/03\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.FOREGROUND, color: [1, 2, 3] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.FOREGROUND, color: [1, 2, 3] }]]);
stack.length = 0;
await inputHandler.parseP('\x1b]10;#aabbcc\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.FOREGROUND, color: [170, 187, 204] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.FOREGROUND, color: [170, 187, 204] }]]);
stack.length = 0;
// set FG, BG and cursor color at once
await inputHandler.parseP('\x1b]10;rgb:aa/bb/cc;#001122;rgb:12/34/56\x07');
assert.deepEqual(stack, [
[{ type: ColorRequestType.SET, index: ColorIndex.FOREGROUND, color: [170, 187, 204] }],
[{ type: ColorRequestType.SET, index: ColorIndex.BACKGROUND, color: [0, 17, 34] }],
[{ type: ColorRequestType.SET, index: ColorIndex.CURSOR, color: [18, 52, 86] }]
[{ type: ColorRequestType.SET, index: SpecialColorIndex.FOREGROUND, color: [170, 187, 204] }],
[{ type: ColorRequestType.SET, index: SpecialColorIndex.BACKGROUND, color: [0, 17, 34] }],
[{ type: ColorRequestType.SET, index: SpecialColorIndex.CURSOR, color: [18, 52, 86] }]
]);
});
it('110: restore FG color', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
await inputHandler.parseP('\x1b]110\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: ColorIndex.FOREGROUND }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.FOREGROUND }]]);
});
it('11: BG set & query events', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
// single background query --> color undefined
await inputHandler.parseP('\x1b]11;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.BACKGROUND }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.BACKGROUND }]]);
stack.length = 0;
// OSC 11 with multiple values creates only BG and cursor event
await inputHandler.parseP('\x1b]11;?;?;?;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.BACKGROUND }], [{ type: ColorRequestType.REPORT, index: ColorIndex.CURSOR }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.BACKGROUND }], [{ type: ColorRequestType.REPORT, index: SpecialColorIndex.CURSOR }]]);
stack.length = 0;
// set background color events
await inputHandler.parseP('\x1b]11;rgb:01/02/03\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.BACKGROUND, color: [1, 2, 3] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.BACKGROUND, color: [1, 2, 3] }]]);
stack.length = 0;
await inputHandler.parseP('\x1b]11;#aabbcc\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.BACKGROUND, color: [170, 187, 204] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.BACKGROUND, color: [170, 187, 204] }]]);
stack.length = 0;
// set BG and cursor color at once
await inputHandler.parseP('\x1b]11;#001122;rgb:12/34/56\x07');
assert.deepEqual(stack, [
[{ type: ColorRequestType.SET, index: ColorIndex.BACKGROUND, color: [0, 17, 34] }],
[{ type: ColorRequestType.SET, index: ColorIndex.CURSOR, color: [18, 52, 86] }]
[{ type: ColorRequestType.SET, index: SpecialColorIndex.BACKGROUND, color: [0, 17, 34] }],
[{ type: ColorRequestType.SET, index: SpecialColorIndex.CURSOR, color: [18, 52, 86] }]
]);
});
it('111: restore BG color', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
await inputHandler.parseP('\x1b]111\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: ColorIndex.BACKGROUND }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.BACKGROUND }]]);
});
it('12: cursor color set & query events', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
// single cursor query --> color undefined
await inputHandler.parseP('\x1b]12;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.CURSOR }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.CURSOR }]]);
stack.length = 0;
// OSC 12 with multiple values creates only cursor event
await inputHandler.parseP('\x1b]12;?;?;?;?\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: ColorIndex.CURSOR }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.REPORT, index: SpecialColorIndex.CURSOR }]]);
stack.length = 0;
// set cursor color events
await inputHandler.parseP('\x1b]12;rgb:01/02/03\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.CURSOR, color: [1, 2, 3] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.CURSOR, color: [1, 2, 3] }]]);
stack.length = 0;
await inputHandler.parseP('\x1b]12;#aabbcc\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: ColorIndex.CURSOR, color: [170, 187, 204] }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.SET, index: SpecialColorIndex.CURSOR, color: [170, 187, 204] }]]);
});
it('112: restore cursor color', async () => {
const stack: IColorEvent[] = [];
inputHandler.onColor(ev => stack.push(ev));
await inputHandler.parseP('\x1b]112\x07');
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: ColorIndex.CURSOR }]]);
assert.deepEqual(stack, [[{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.CURSOR }]]);
});
});

Expand Down
18 changes: 11 additions & 7 deletions src/common/InputHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* @license MIT
*/

import { IInputHandler, IAttributeData, IDisposable, IWindowOptions, IColorEvent, IParseStack, ColorIndex, ColorRequestType } from 'common/Types';
import { IInputHandler, IAttributeData, IDisposable, IWindowOptions, IColorEvent, IParseStack, ColorIndex, ColorRequestType, SpecialColorIndex } from 'common/Types';
import { C0, C1 } from 'common/data/EscapeSequences';
import { CHARSETS, DEFAULT_CHARSET } from 'common/data/Charsets';
import { EscapeSequenceParser } from 'common/parser/EscapeSequenceParser';
Expand Down Expand Up @@ -2915,7 +2915,7 @@ export class InputHandler extends Disposable implements IInputHandler {
const spec = slots.shift() as string;
if (/^\d+$/.exec(idx)) {
const index = parseInt(idx);
if (0 <= index && index < 256) {
if (isValidColorIndex(index) && index < 256) {
if (spec === '?') {
event.push({ type: ColorRequestType.REPORT, index });
} else {
Expand Down Expand Up @@ -2988,7 +2988,7 @@ export class InputHandler extends Disposable implements IInputHandler {
}

// special colors - OSC 10 | 11 | 12
private _specialColors = [ColorIndex.FOREGROUND, ColorIndex.BACKGROUND, ColorIndex.CURSOR];
private _specialColors = [SpecialColorIndex.FOREGROUND, SpecialColorIndex.BACKGROUND, SpecialColorIndex.CURSOR];

/**
* Apply colors requests for special colors in OSC 10 | 11 | 12.
Expand Down Expand Up @@ -3073,7 +3073,7 @@ export class InputHandler extends Disposable implements IInputHandler {
for (let i = 0; i < slots.length; ++i) {
if (/^\d+$/.exec(slots[i])) {
const index = parseInt(slots[i]);
if (0 <= index && index < 256) {
if (isValidColorIndex(index) && index < 256) {
event.push({ type: ColorRequestType.RESTORE, index });
}
}
Expand All @@ -3090,7 +3090,7 @@ export class InputHandler extends Disposable implements IInputHandler {
* @vt: #Y OSC 110 "Restore default foreground color" "OSC 110 BEL" "Restore default foreground to themed color."
*/
public restoreFgColor(data: string): boolean {
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: ColorIndex.FOREGROUND }]);
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.FOREGROUND }]);
return true;
}

Expand All @@ -3100,7 +3100,7 @@ export class InputHandler extends Disposable implements IInputHandler {
* @vt: #Y OSC 111 "Restore default background color" "OSC 111 BEL" "Restore default background to themed color."
*/
public restoreBgColor(data: string): boolean {
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: ColorIndex.BACKGROUND }]);
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.BACKGROUND }]);
return true;
}

Expand All @@ -3110,7 +3110,7 @@ export class InputHandler extends Disposable implements IInputHandler {
* @vt: #Y OSC 112 "Restore default cursor color" "OSC 112 BEL" "Restore default cursor to themed color."
*/
public restoreCursorColor(data: string): boolean {
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: ColorIndex.CURSOR }]);
this._onColor.fire([{ type: ColorRequestType.RESTORE, index: SpecialColorIndex.CURSOR }]);
return true;
}

Expand Down Expand Up @@ -3429,3 +3429,7 @@ class DirtyRowTracker implements IDirtyRowTracker {
this.markRangeDirty(0, this._bufferService.rows - 1);
}
}

function isValidColorIndex(value: number): value is ColorIndex {
return 0 <= value && value < 259;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerch should this be < 256 instead? If so what would be a good name for this color index sub-range? IsValidInputColorIndex or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for colors the index is only 0..255, so its better to test for that range in during input. The fg/bg/cursor entries on top are an amalgamation, so the events and methods dont need re-declaration and/or branching.

Loading