-
-
Notifications
You must be signed in to change notification settings - Fork 862
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.
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
Add overline
style and remove keyword
, hsl
, hsv
, hwb
and ansi
color spaces
#433
Conversation
…nsi` style Signed-off-by: Richie Bendall <richiebendall@gmail.com>
overline
style, remove keyword
, hsl
, hsv
, hwb
and ansi
style and stop backporting ansi256
to ansi
on older terminalsoverline
style, remove keyword
, hsl
, hsv
, hwb
and ansi
style and stop downsampling to ansi
on older terminals
Because we no longer downsample |
Just curious, why aren't we downsampling anymore? That seems like a functional regression, given that there are still loads of terminals that only support the older 16 color palette. |
@sindresorhus first suggested to remove it: #300 (comment) Then you in a seperate conversation, approved removal of |
I don't see in there why we need to remove this functionality. I would say we should inline that from The concern about upscaling (#370) was valid, but downscaling is absolutely necessary to keep compatibility with people who do not have 256 support. As for the comments about "removing" that - no, those say to remove
If we remove downsampling, we're going to irreparably break a LOT of people. |
That function doesn't exist |
Right, you have to merge them - this is untested but should be close enough. Just needs a bit of polish for the Chalk conventions. function ansi256To16(args) {
if (args < 8) return 30 + args;
if (args < 16) return 90 + (args - 8);
let r, g, b;
// Handle greyscale
if (args >= 232) {
const c = (args - 232) * 10 + 8;
r = g = b = c / 255;
} else {
args -= 16;
let rem;
r = Math.floor(args / 36) / 5;
g = Math.floor((rem = args % 36) / 6) / 5;
b = (rem % 6) / 5;
}
const value = Math.max(r, g, b) * 2;
if (value === 0) {
return 30;
}
let ansi = 30
+ ((Math.round(b) << 2)
| (Math.round(g) << 1)
| Math.round(r));
if (value === 2) {
ansi += 60;
}
return ansi;
} Then just make a bunch of test cases as such: // Just as an example.
const colorConvert = require('color-convert');
const assert = require('assert').strict.ok;
const {ansi256To16} = require('..');
// Start at 16 since color-convert hasn't been updated to
// correctly return the first 16 colors of ansi-256.
for (let i = 16; i < 256; i++) {
assert(
colorConvert.ansi256.ansi16(i) === ansi256To16(i)
);
} |
Signed-off-by: Richie Bendall <richiebendall@gmail.com>
overline
style, remove keyword
, hsl
, hsv
, hwb
and ansi
style and stop downsampling to ansi
on older terminalsoverline
style, remove keyword
, hsl
, hsv
and hwb
style
Support for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anything glaringly wrong with the code; I'll let Sindre have the final look.
overline
style, remove keyword
, hsl
, hsv
and hwb
styleoverline
style and remove keyword
, hsl
, hsv
and hwb
color spaces
I'm confused. I understand preserving the downsampling, but shouldn't we still remove the explicit |
IMO, there is no impact of leaving the function there so we might as well leave it for people wanting to use the |
People wanting to use the I agree with sindre here - no need for See also: https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it |
Signed-off-by: Richie Bendall <richiebendall@gmail.com>
overline
style and remove keyword
, hsl
, hsv
and hwb
color spacesoverline
style and remove keyword
, hsl
, hsv
, hwb
and ansi
color spaces
Removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nice work, @Richienb 👍 |
Yes indeed :) Thanks @Richienb, super appreciated. |
Update
ansi-styles
packageoverline
stylekeyword
,hsl
,hsv
,hwb
andansi
color spaces#431
Fixes #360