colorette icon indicating copy to clipboard operation
colorette copied to clipboard

FORCE_COLOR=0 forces color

Open chocolateboy opened this issue 4 years ago • 11 comments

Re-opening this to ensure it doesn't get overlooked. Test case copied from https://github.com/jorgebucaran/colorette/pull/41#issuecomment-629703746. Fixed in https://github.com/jorgebucaran/colorette/pull/41, but the patch was modified, which introduced the new bug.

In the current version (1.2.0) of Colorette, the following all have the same effect:

  • FORCE_COLOR=1
  • FORCE_COLOR=0
  • FORCE_COLOR=

i.e. FORCE_COLOR=<falsey/no value> incorrectly forces color which wouldn't otherwise be displayed.

test.js

const { green } = require('colorette')

console.log(green('Hello, world!'))

1) expect color

$ FORCE_COLOR=1 node ./test.js | strings

output ✔

[32mHello, world!
[39m

2) expect no color (because of the pipe)

$ node ./test.js | strings

output ✔

Hello, world!

3) expect no color (same as 2)

$ FORCE_COLOR=0 node ./test.js | strings

output :x: :bug:

[32mHello, world!
[39m

4) expect no color (same as 2)

$ FORCE_COLOR= node ./test.js | strings

output :x: :bug:

[32mHello, world!
[39m

chocolateboy avatar Jun 22 '20 10:06 chocolateboy

Thank you, @chocolateboy. I recently added tests for FORCE_COLOR and NO_COLOR.

Could you explain why FORCE_COLOR=0 node ./test.js | strings or FORCE_COLOR= node ./test.js | strings expects no color?

I thought I had understood the issue, but I think I lost it again. My understanding is that FORCE_COLOR={1,2,3...} forces color and that FORCE_COLOR={,0} should do nothing (as opposed to deliberately disable color).

jorgebucaran avatar Jun 26 '20 12:06 jorgebucaran

To answer my own question, I think it is because when we pipe colorette, color should be disabled.

jorgebucaran avatar Jun 26 '20 12:06 jorgebucaran

The "no color" in those cases is because of the pipe. The FORCE_COLOR is ignored in those cases i.e. it doesn't/shouldn't force color on or force color off. (It's kind of a red("herring") :-)

chocolateboy avatar Jun 26 '20 12:06 chocolateboy

🎉 @chocolateboy I got it! But...


Now that I finally understand the issue, I'd like to advocate against implementing FORCE_COLOR=0 as described in this comment. Why? I think presence of FORCE_COLOR in process.env, regardless of its value, should force color. But why? For consistency with its evil twin NO_COLOR which we also support.

All command-line software which outputs text with ANSI color added should check for the presence of a NO_COLOR environment variable that, when present (regardless of its value), prevents the addition of ANSI color.

It's also easier to reason about FORCE_COLOR and NO_COLOR as boolean pairs, rather than using FORCE_COLOR=0 to mean don't force color, which I find super cryptic. And if we went that route, I guess we'd also have to implement NO_COLOR=0. But what does that even mean? My brain exploded while trying to parse that. 😄

Now, I get it, maybe you have a script where you set FORCE_COLOR's value. Well, instead of switching from a 0 to a 1 (or was it 1 to 0?) use the right variable.

Here are two ways to do it that come to mind:

I use the fish shell and we need to place env before declaring environment variables, but you should be able to figure out the equivalent in your own shell.

set -l COLOR_MODE = NO_COLOR # or FORCE_COLOR

env $COLOR_MODE= ./test.js

or a one-liner:

env (test $condition && echo FORCE_COLOR || echo NO_COLOR)= ./test.js

jorgebucaran avatar Jun 26 '20 17:06 jorgebucaran

I think this is an awful lot of work to avoid adding a few bytes to the source code :-/

As you mention, the behavior of NO_COLOR= and NO_COLOR=0 (both equivalent to NO_COLOR=1) is non-standard and evil. No other environment variables behave that way.

It's also easier to reason about FORCE_COLOR and NO_COLOR as boolean pairs

I think it's easy enough to reason about the semantics if the code is written clearly, which is why I ungolfed it. Either way, as mentioned before:

I don't think anything else interprets FORCE_COLOR in this way (i.e. FORCE_COLOR=0 forces color), not even chalk, which supports FORCE_COLOR=0, FORCE_COLOR=1, FORCE_COLOR=2 and FORCE_COLOR=3 (!).

chocolateboy avatar Jun 26 '20 17:06 chocolateboy

@chocolateboy I respect your opinion, but I don't consider that to be a lot of work. And that's only work you'd do only if you wanted to switch between FORCE_COLOR and NO_COLOR (I don't think anyone would normally do that).

Frankly, I don't care about the bytes, this is Node after all—your node_modules are heavier than the universe. My reason to go against this is not bytes. I just find things like FORCE_COLOR=0 or FORCE_COLOR=1 to be confusing. And I didn't mean NO_COLOR is literally evil, haha, that was just a way to refer to NO_COLOR and FORCE_COLOR as counterparts.

I don't think there's a standard governing any of this, so I wouldn't claim the behavior of NO_COLOR to be non-standard. But I'm not an expert either. It's what it is, at least they have a website https://no-color.org.


I don't think anything else interprets FORCE_COLOR in this way (i.e. FORCE_COLOR=0 forces color), not even chalk, which supports FORCE_COLOR=0, FORCE_COLOR=1, FORCE_COLOR=2 and FORCE_COLOR=3 (!).

My eyes!!! 👀🩸

When I first encountered chalk way back in the day, I was frustrated by its complexity. Today, it's even more complex. It's complex any way you look at it. Some people are more adept at dealing with complexity. Others don't care. I am very particular about this. I just want colorette to be simple, whether you are using it or looking under the covers. It was probably already a bad idea enabling/disabling color out of the box, but I can't roll that back now.

jorgebucaran avatar Jun 26 '20 18:06 jorgebucaran

AFAICT, no other JS implementations of FORCE_COLOR behave in the way you're proposing (i.e. like NO_COLOR), and only NO_COLOR (mis)behaves in the way you're proposing, so I don't think adopting these two non-standards makes things simpler or easier to reason about.

chocolateboy avatar Jun 26 '20 18:06 chocolateboy

@chocolateboy What if we could do better than them? By the way, do you have an example of FORCE_COLOR used in the wild?

jorgebucaran avatar Jun 27 '20 03:06 jorgebucaran

What if we could do better than them?

I don't think "everyone else is doing it wrong - let's innovate!" is a better approach. FORCE_COLOR may not have its own website, but it's still an informal standard which no other JS library I can find interprets in the way you're proposing. Yes, FORCE_COLOR=0 may be interpreted in two different ways, but I can't see how introducing a new, third way is an improvement.

do you have an example of FORCE_COLOR used in the wild?

My original use case is here:

I'm trying to pipe colored output through a shell pipeline in a TTY, but setting FORCE_COLOR to a truthy value doesn't seem to work

If you mean an example of how it's interpreted in Node.js libraries/apps, see here. I couldn't find an example in the first 10 pages which behaves in the way you're proposing (i.e. like NO_COLOR), but someone with more time may be able to find one.

chocolateboy avatar Jun 27 '20 13:06 chocolateboy

Hey sorry to comment on an old issue, but Node.js itself now disables color then FORCE_COLOR is set to 0: https://nodejs.org/api/tty.html#writestreamgetcolordepthenv

It'd be great for JS libraries to align with the Node.js behavior.

nicolo-ribaudo avatar Mar 19 '24 11:03 nicolo-ribaudo

Thanks for the heads-up, @nicolo-ribaudo! 💯

jorgebucaran avatar Mar 28 '24 22:03 jorgebucaran