Add IntRange type for ColorIndex#4618
Conversation
| function isValidColorIndex(value: number): value is ColorIndex { | ||
| return 0 <= value && value < 259; | ||
| } |
There was a problem hiding this comment.
@jerch should this be < 256 instead? If so what would be a good name for this color index sub-range? IsValidInputColorIndex or something?
There was a problem hiding this comment.
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.
src/common/Types.d.ts
Outdated
| : Enumerate<N, [...Acc, Acc['length']]>; | ||
| type IntRange<F extends number, T extends number> = Exclude<Enumerate<T>, Enumerate<F>>; | ||
|
|
||
| type ColorIndex = IntRange<0, 259>; // number from 0 to 258 |
There was a problem hiding this comment.
Would it be clearer to limit ColorIndex here to 0..255, and to merge into a super type with the fg/bg/cursor values in a second step? Also the test function above can then refer to that narrower ColorIndex type.
jerch
left a comment
jerch
left a comment
There was a problem hiding this comment.
LGTM, beside the left-over doubled change check below.
2 participants
Fixes #4614