Skip to content
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 RGB (256/Truecolor) support #140

Merged
merged 18 commits into from Jun 20, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions .travis.yml
@@ -1,8 +1,7 @@
sudo: false
language: node_js
node_js:
- 'stable'
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed. We already define 6 and 4.

- '6'
- '4'
- '0.12'
- '0.10'
after_success: npm run coveralls
61 changes: 53 additions & 8 deletions index.js
Expand Up @@ -6,9 +6,12 @@ var supportsColor = require('supports-color');
var defineProps = Object.defineProperties;
var isSimpleWindowsTerm = process.platform === 'win32' && !/^xterm/i.test(process.env.TERM);

var levelMapping = ['ansi', 'ansi', 'ansi256', 'ansi16m'];
Copy link
Member

Choose a reason for hiding this comment

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

You have ansi twice there.

Copy link
Member Author

Choose a reason for hiding this comment

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

The double-ansi is on purpose - if level is 0 we still need to be pulling something from ansi-styles.

Copy link
Member Author

Choose a reason for hiding this comment

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

Open to suggestions as to how to better approach it. I agree it feels a little clunky. A lot of what chalk does is clunky, though.

Copy link
Member

Choose a reason for hiding this comment

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

I see. A short comment would solve it.

Copy link
Member

Choose a reason for hiding this comment

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

Nvm the Set comment. I didn't read the code properly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done :)

var skipModels = ['gray'];
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean skipModes?

Copy link
Member Author

Choose a reason for hiding this comment

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

No - "model" refers to the color models within color-convert. gray was for CSS gray() and conflicts with our color gray or grey.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a short comment explaining that. Was not clear when reading the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍


function Chalk(options) {
// detect mode if not set manually
this.enabled = !options || options.enabled === undefined ? supportsColor : options.enabled;
// detect level if not set manually
this.level = !options || options.level === undefined ? supportsColor.level : options.level;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should still support enabled. Not enough benefit to make such a big breaking change. We can support both. Most will only need to know whether it's enabled or not anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

How do we handle when someone supplies {level: 0, enabled: true} or {level: 2, enabled: false}?

Copy link
Member

Choose a reason for hiding this comment

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

How you handle mutually exclusive things in general, you throw a user-friendly error, and document it well.

Copy link
Member Author

@Qix- Qix- Jan 17, 2017

Choose a reason for hiding this comment

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

I started making this change and it just doesn't make sense. The code is super hairy - having to check to see if keys are in the configuration object and then do some arbitrary logic on the values. What does enabled: true mean now? A straight-up level of 1 or do we use supportsColor.level?

We're already bumping the major version and those advanced users using forced enable/disable outside of the functionality supplied by supports-color will be willing to adjust how they want Chalk to behave.

Choose a reason for hiding this comment

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

I'd vote on leaving enabled as well, mostly for backwards compatibility reasons.

A straight-up level of 1 or do we use supportsColor.level?

This seems like a good solution. Anything potentially bad with it?

Choose a reason for hiding this comment

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

The code you posted is fine, but I think enabled needs to be defined via Object.defineProperty and modify/read this.level under the hood to support that:

chalk.enabled = false;

Please correct me if that's wrong, I may be missing things.

Choose a reason for hiding this comment

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

I'm genuinely curious as to how many people actually use the custom constructor?

I'm guessing like you do, but I doubt many people use that. To say the truth, I discovered it just now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally think the code I posted is bloated and huge just to support a property that doesn't really make sense now that there are different levels of support instead of a boolean on/off value.

If we were to do this, then I agree; .enabled would have to reflect the .level given a getter/setter descriptor.

Choose a reason for hiding this comment

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

I'm only voting for enabled prop, because I see it somewhat often used in tests to conveniently enable/disable colors when testing output.

Let's see what other guys think.

Copy link
Member Author

Choose a reason for hiding this comment

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

The level prop will have the same functionality, though :) haha

}

// use bright blue on Windows as the normal blue color is illegible
Expand All @@ -23,21 +26,63 @@ Object.keys(ansiStyles).forEach(function (key) {

styles[key] = {
get: function () {
return build.call(this, this._styles ? this._styles.concat(key) : [key]);
var codes = ansiStyles[key];
return build.call(this, this._styles ? this._styles.concat(codes) : [codes], key);
}
};
});

ansiStyles.color.closeRe = new RegExp(escapeStringRegexp(ansiStyles.color.close), 'g');
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Member Author

@Qix- Qix- Jan 17, 2017

Choose a reason for hiding this comment

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

Why not that? Chalk requires a closing code regex for building the style. No sense in re-calculating it for every single color-convert model in the loop right after it since it's the same for all of them.

Copy link
Member

Choose a reason for hiding this comment

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

I mean, it's a little bit dangerous to assign to ansiStyles as it's a single-ton. So if someone uses ansi-styles another place, their instance will also be monkey-patched. Might be a non-issue in practise, but wanted to bring it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Chalk did it before in the loop above. We can change that, though - I'm not opposed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, it's not applied globally. We fixed that a while back :)

Object.keys(ansiStyles.color.ansi).forEach(function (model) {
Copy link
Member

Choose a reason for hiding this comment

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

Use a for-of loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we change the original loop as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but can do unrelated changes later so not to make this PR too noisy.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should do a complete es2015-ification PR as a separate step to prevent history fragmentation, IMO. But I agree that it should happen.

if (skipModels.indexOf(model) !== -1) {

Choose a reason for hiding this comment

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

skipModels.indexOf(model) >= 0 would look nicer, imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tend to err on !== -1 since the spec specifically states

The first index of the element in the array; -1 if not found.

It's obviously bike shedding. It'd be nice if XO made a definitive decision on it.

Choose a reason for hiding this comment

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

True that.

return;
}

styles[model] = {
get: function () {
var level = this.level;
return function () {
var open = ansiStyles.color[levelMapping[level]][model].apply(null, arguments);
var codes = {open: open, close: ansiStyles.color.close, closeRe: ansiStyles.color.closeRe};
return build.call(this, this._styles ? this._styles.concat(codes) : [codes], model);
};
}
};
});

ansiStyles.bgColor.closeRe = new RegExp(escapeStringRegexp(ansiStyles.bgColor.close), 'g');
Object.keys(ansiStyles.bgColor.ansi).forEach(function (model) {
Copy link
Member

Choose a reason for hiding this comment

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

Use a for-of loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

if (skipModels.indexOf(model) !== -1) {

Choose a reason for hiding this comment

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

return;
}

var bgModel = 'bg' + model.charAt(0).toUpperCase() + model.substring(1);
styles[bgModel] = {
get: function () {
var level = this.level;
return function () {
var open = ansiStyles.bgColor[levelMapping[level]][model].apply(null, arguments);
var codes = {open: open, close: ansiStyles.bgColor.close, closeRe: ansiStyles.bgColor.closeRe};
return build.call(this, this._styles ? this._styles.concat(codes) : [codes], model);
};
}
};
});

// eslint-disable-next-line func-names
var proto = defineProps(function chalk() {}, styles);

function build(_styles) {
function build(_styles, key) {
var builder = function () {
return applyStyle.apply(builder, arguments);
};

builder._styles = _styles;
builder.enabled = this.enabled;
builder.level = this.level;

// see below for fix regarding invisible grey/dim combination on windows.
builder.hasGrey = this.hasGrey || key === 'gray' || key === 'grey';

// __proto__ is used because we must return a function, but there is
// no way to create a function with a different prototype.
/* eslint-disable no-proto */
Expand All @@ -59,7 +104,7 @@ function applyStyle() {
}
}

if (!this.enabled || !str) {
if (!this.level || !str) {
return str;
}

Expand All @@ -70,12 +115,12 @@ function applyStyle() {
// see https://github.com/chalk/chalk/issues/58
// If we're on Windows and we're dealing with a gray color, temporarily make 'dim' a noop.
var originalDim = ansiStyles.dim.open;
if (isSimpleWindowsTerm && (nestedStyles.indexOf('gray') !== -1 || nestedStyles.indexOf('grey') !== -1)) {
if (isSimpleWindowsTerm && this.hasGrey) {
ansiStyles.dim.open = '';
}

while (i--) {
var code = ansiStyles[nestedStyles[i]];
var code = nestedStyles[i];

// Replace any instances already present with a re-opening code
// otherwise only the part of the string until said closing code
Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -44,9 +44,9 @@
"text"
],
"dependencies": {
"ansi-styles": "^2.1.0",
"ansi-styles": "^3.0.0",
"escape-string-regexp": "^1.0.2",
"supports-color": "^3.1.2"
"supports-color": "^3.2.3"
},
"devDependencies": {
"coveralls": "^2.11.2",
Expand Down
47 changes: 42 additions & 5 deletions readme.md
Expand Up @@ -76,14 +76,23 @@ CPU: ${chalk.red('90%')}
RAM: ${chalk.green('40%')}
DISK: ${chalk.yellow('70%')}
`);

// Use RGB colors in terminal emulators that support it.
log(chalk.keyword('orange')('Yay for orange colored text!'));
log(chalk.rgb(123, 45, 67).underline('Underlined reddish color'));
log(chalk.hex('#DEADED').bold('Bold gray!'));
```

Easily define your own themes.

```js
const chalk = require('chalk');

const error = chalk.bold.red;
const warning = chalk.keyword('orange');

console.log(error('Error!'));
console.log(warning('Warning!'));
```

Take advantage of console.log [string substitution](http://nodejs.org/docs/latest/api/console.html#console_console_log_data).
Expand All @@ -105,16 +114,23 @@ Chain [styles](#styles) and call the last one as a method with a string argument

Multiple arguments will be separated by space.

### chalk.enabled
### chalk.level

Color support is automatically detected, but you can override it by setting the `enabled` property. You should however only do this in your own code as it applies globally to all chalk consumers.
Color support is automatically detected, but you can override it by setting the `level` property. You should however only do this in your own code as it applies globally to all chalk consumers.

If you need to change this in a reusable module create a new instance:

```js
const ctx = new chalk.constructor({enabled: false});
const ctx = new chalk.constructor({level: 0});
```

Levels are as follows:

0. All color disabled
Copy link
Member

Choose a reason for hiding this comment

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

colors

1. Basic color support (16 colors)
2. 256 color support
3. RGB/Truecolor support (16 million colors)

### chalk.supportsColor

Detect whether the terminal [supports color](https://github.com/chalk/supports-color). Used internally and handled for you, but exposed for convenience.
Expand Down Expand Up @@ -174,10 +190,30 @@ console.log(chalk.styles.red.open + 'Hello' + chalk.styles.red.close);
- `bgWhite`


## 256-colors
## 256/16 million (Truecolor) color support

Chalk supports 256 colors and, when manually specified, [Truecolor (16 million colors)](https://gist.github.com/XVilka/8346728) on all supported terminal emulators.
Copy link
Member

Choose a reason for hiding this comment

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

No need for comma after and

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not? It's a qualifier for the subsequent clause. Grammatically the lack of a comma wouldn't make sense here.


For the methods that support it (listed below), the color will be 'fit' to the color level supported (i.e. RGB colors will be downsampled to 16 colors if only basic support is enabled).
Copy link
Contributor

Choose a reason for hiding this comment

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

"For supported methods"

Copy link
Member Author

Choose a reason for hiding this comment

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

👍


For a complete list of color models, see [`color-convert`'s list of conversions](https://github.com/Qix-/color-convert/blob/master/conversions.js). Background versions of these models are prefixed with `bg` and the first level of the module capitalized (e.g. `keyword` for foreground colors and `bgKeyword` for background colors).
Copy link
Member

Choose a reason for hiding this comment

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

Add some examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this paragraph below the examples.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Does that look better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's fine.


Chalk does not support anything other than the base eight colors, which guarantees it will work on all terminals and systems. Some terminals, specifically `xterm` compliant ones, will support the full range of 8-bit colors. For this the lower level [ansi-256-colors](https://github.com/jbnicolai/ansi-256-colors) package can be used.
As of this writing, these are the supported color models that are exposed in Chalk:

- `rgb`
- `hsl`
- `hsv`
- `hwb`
- `cmyk`
- `xyz`
- `lab`
- `lch`
- `hex`
- `keyword`
- `ansi16`
- `ansi256`
- `hcg`
- `apple`
Copy link
Member

Choose a reason for hiding this comment

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

These needs to be explained, and put the most common ones at the top, like ansi* and hex.

Copy link
Member Author

Choose a reason for hiding this comment

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

ansi* aren't very common since Chalk handles them already. I'll add hex to the top.


## Windows

Expand All @@ -194,6 +230,7 @@ If you're on Windows, do yourself a favor and use [`cmder`](http://cmder.net/) i
- [ansi-regex](https://github.com/chalk/ansi-regex) - Regular expression for matching ANSI escape codes
- [wrap-ansi](https://github.com/chalk/wrap-ansi) - Wordwrap a string with ANSI escape codes
- [slice-ansi](https://github.com/chalk/slice-ansi) - Slice a string with ANSI escape codes
- [color-convert](https://github.com/qix-/color-convert) - Converts colors between different models


## License
Expand Down
29 changes: 25 additions & 4 deletions test.js
Expand Up @@ -72,6 +72,26 @@ describe('chalk', function () {
it('line breaks should open and close colors', function () {
assert.equal(chalk.grey('hello\nworld'), '\u001b[90mhello\u001b[39m\n\u001b[90mworld\u001b[39m');
});

it('should properly convert RGB to 16 colors on basic color terminals', function () {
assert.equal(new chalk.constructor({level: 1}).hex('#FF0000')('hello'), '\u001b[91mhello\u001b[39m');
assert.equal(new chalk.constructor({level: 1}).bgHex('#FF0000')('hello'), '\u001b[101mhello\u001b[49m');
});

it('should properly convert RGB to 256 colors on basic color terminals', function () {
assert.equal(new chalk.constructor({level: 2}).hex('#FF0000')('hello'), '\u001b[38;5;196mhello\u001b[39m');
assert.equal(new chalk.constructor({level: 2}).bgHex('#FF0000')('hello'), '\u001b[48;5;196mhello\u001b[49m');
});

it('should properly convert RGB to 256 colors on basic color terminals', function () {
assert.equal(new chalk.constructor({level: 3}).hex('#FF0000')('hello'), '\u001b[38;2;255;0;0mhello\u001b[39m');
assert.equal(new chalk.constructor({level: 3}).bgHex('#FF0000')('hello'), '\u001b[48;2;255;0;0mhello\u001b[49m');
});

it('should not emit RGB codes if level is 0', function () {
assert.equal(new chalk.constructor({level: 0}).hex('#FF0000')('hello'), 'hello');
assert.equal(new chalk.constructor({level: 0}).bgHex('#FF0000')('hello'), 'hello');
});
});

describe('chalk on windows', function () {
Expand Down Expand Up @@ -132,17 +152,18 @@ describe('chalk on windows', function () {
});
});

describe('chalk.enabled', function () {
describe('chalk.level', function () {
it('should not output colors when manually disabled', function () {
chalk.enabled = false;
var oldLevel = chalk.level;
chalk.level = 0;
assert.equal(chalk.red('foo'), 'foo');
chalk.enabled = true;
chalk.level = oldLevel;
});
});

describe('chalk.constructor', function () {
it('should create a isolated context where colors can be disabled', function () {
var ctx = new chalk.constructor({enabled: false});
var ctx = new chalk.constructor({level: 0});
assert.equal(ctx.red('foo'), 'foo');
assert.equal(chalk.red('foo'), '\u001b[31mfoo\u001b[39m');
});
Expand Down