nodejs / help

:sparkles: Need help with Node.js? File an Issue here. :rocket:
1.48k stars 284 forks source link

Node inspect color choice clashes with terminal background #2991

Closed veganaize closed 4 years ago

veganaize commented 4 years ago

The Array.prototype.forEach()'s index value doesn't seem to be handled properly when passed as a comma-separated parameter to console.log().

The index value is echoed in yellow unless concatenated with the string addition operator (+).

What steps will reproduce the bug?

var my_array = ['a', 'b', 'c'];

function my_func(value, index, array) {
  console.log(index, value);
}

my_array.forEach(my_func);

How often does it reproduce? Is there a required condition?

It happens every time in a Windows CMD console window.

What is the expected behavior?

//Expected console output (all text with default foreground color):

0 a
1 b
2 c

//Similar to output produced by this code:

var my_array = ['a', 'b', 'c'];

function my_func(value, index, array) {
  console.log(index +'  '+ value);
}

my_array.forEach(my_func);

What do you see instead?

I see bright yellow index numbers and normally colored values.

Additional information

//When invoked in a console with a yellow background, as my code editor does, the output appears like this:

  a
  b
  c

//But I can copy & paste the output and it actually contains the index values.

*When I run it with GraalJS, at the cmd.exe prompt, the behavior works as expected.

devsnek commented 4 years ago

To be clear, the issue you're reporting is that you made your terminal the same color of yellow as node's number formatting, and so numbers are effectively invisible, correct?

veganaize commented 4 years ago

Ultimately, yes.

Additionally, it is automatically applying emphasis to the index's value regardless of the terminal's background color.

devsnek commented 4 years ago

You can set NODE_DISABLE_COLORS=1 in your env if you wish. Not sure there's anything actionable here.

BridgeAR commented 4 years ago

It is also possible to change the default number style by setting util.inspect.number to something else than yellow.

veganaize commented 4 years ago

Could it make more sense to default to colorless output?

I've never known a command-line interpreter / compiler to output color by default.

Sometimes Node also outputs bright green values for array listings, as well.

This issue has caused me days of frustration.

BridgeAR commented 4 years ago

The default is to detect color support of the terminal and if it does support colors, to use them. Deactivating colors by default is not ideal as it is rare to cause issues but it does help to differentiate types.

veganaize commented 4 years ago

Maybe Node could check if there is a color clash in the terminal and invert the afflicted color property?

devsnek commented 4 years ago

It is not trivial to detect background color in terminals. In some cases it even involves waiting for long periods of time for the terminal to report information to you (in gnome terminal, for example, you can wait for as long as a tenth of a second).

veganaize commented 4 years ago

How about always setting both the background & foreground color together, as a pair -- to entirely avoid any issues?

This would coincide with the World Wide Web Consortium (W3C) "Web Content Accessibility Guidelines" (WCAG).

Specifically, their recommendation to specify foreground colors with background colors.

devsnek commented 4 years ago

Many terminals apply custom themes by translating requested colors into the theme colors. Since we can't know what those mappings are, there's no way of knowing what background color to choose. Along this line of logic, it's generally up to the person theming (you in this case) to change the terminal's foreground colors (yellow in this case) to not clash.

veganaize commented 4 years ago

I understand that terminals may apply custom color schemes which override the application defaults. However, what I am suggesting is that if & when Node.js selects a particular foreground color that Node.js also selects an appropriately contrasting background color. Whether or not those colors are later overridden by some other application or terminal isn't really relevant.

devsnek commented 4 years ago

It wouldn't fix the "person is overriding colors" problem, because the person could have overridden them in a way which makes that background/foreground color combination unreadable. The appropriate solution is for you to remap your terminal's ansi colors to not conflict with your background. Any cli program which attempts to display yellow text, not just node, will have this problem in your terminal.

veganaize commented 4 years ago

Take, for example: if Node.js selected "black" as a background color, then my terminal's use of a yellow background wouldn't have mattered. If I found the black background disturbing then, at that point, I could make an effort to customize the color outputs, either through Node.js, the terminal, or both. [The text would have always been readable.]

My foreground / text color has always been, and remains, a (dark) color which contrasts with the (light) background color. I have never had this problem with any other command-line program. I've even written such programs myself, in the past. I fail to understand why Node.js can't simply set a dark (e.g. black) background color whenever it sets a light (e.g. white) text color (and vice versa).

devsnek commented 4 years ago

Node isn't selecting a specific light or dark color to use. It just tells the terminal "i want yellow". Maybe the yellow that the terminal selects is a dark yellow that should go on a light background. Maybe it's a light yellow that should go on a dark background. Maybe the terminal decides that yellow corresponds to a shade of purple. It all depends on your terminal's configuration.

veganaize commented 4 years ago

Yellow always needs a dark background. Just like white always needs a dark background.

If the terminal overrides yellow, and turns it purple, then it's obviously no longer specifically a Node.js issue.

If I had overridden Node.js' foreground color selection then I wouldn't have bothered to bring any of this up. The fact is that I didn't. Node.js overrode the foreground color and therefore it should override the background color, as well. If Node.js can't change the background color with any confidence then how can it do so with the foreground color?

I'm simply asking for a best effort -- in accordance with long accepted accessibility practices. This ISN'T a request to override every possible color trap the a user could possibly get themselves into.

How easy is it for users to read the output if they have (otherwise) dark text over a white background (as many traditional IDEs do by default)?

I really feel like you're grasping at straws in order to shoot this down.

devsnek commented 4 years ago

You should adjust your terminal's configuration so that ANSI 4 bit colors are readable.

veganaize commented 4 years ago

I'm using cmd.exe on Windows 8.1. It doesn't understand ANSI escape codes. All I can do is set the default foreground and background colors.

Node.js should set the background color if it alters the foreground color.

I didn't ask Node.js to change the color value of index, in the Array.prototype.forEach() callback, when echoed via console.log. But you don't seem to care about making software accessible.

Is this how you treat black people which the Node.js project claims to care so much about? You guys claim to be systematically complicit in perpetuating the issues that led us to where we are yet you take no action to the contrary. You continue to assert your dominance as (mostly) white men even though you say you'll change how you build it, moving forward. Was that nothing more than virtue signalling? I don't feel that I've had access to collaboration nor amplification -- just shut down.

Trott commented 4 years ago

Might be good to get some other opinions in here as to whether Node.js could and should do more to keep background/foreground contrast adequate in the terminal on Windows. @nodejs/console @nodejs/repl @nodejs/platform-windows

mildsunrise commented 4 years ago

For context (no windows knowledge here, but on every other OS):

I don't think it's actually possible to query background or graphical attributes from the terminal, you can only set them.

Like @devsnek said, most applications don't typically specify exact colors, they use the 8-color palette that is known to work everywhere. We do that, too. So when users change the terminal's colors, as long as they don't choose clashing colors for their palette, things are good.

veganaize commented 4 years ago

It is possible to query colors but that isn't necessary. The plain and simple fact is that, when setting either the foreground or background color, one should also attempt to set the other. So, keeping it simple, if Node.js selects a color that one would typically describe as "light" then the background should be set to a color typically described as "dark". (And vice versa)

Otherwise Node.js should (ideally) default to no color and allow color to be enabled separately (e.g. like most GNU/Linux terminals).

veganaize commented 4 years ago

Here are some sample colorings using QBasic, in DOSBox, on Windows. I think it's a nice example of the variations (from the stated color) that @devsnek was referring to. From my experience the light over dark & dark over light thing works well enough. But if we really can't be certain then I'm still behind no color as the default. (And I'll shut up now ☺) image

mildsunrise commented 4 years ago

The plain and simple fact is that, when setting either the foreground or background color, one should also attempt to set the other

Not sure what you mean, but I know no programs that output colored text (i.e. grep, git diff, less) that explicitely set a background when changing foreground :thinking: Demo with grep:

Captura de 2020-10-01 13-54-05

Some of them use CSI m to reset the foreground color, which has the side-effect of also resetting the background to its default color. This can be seen above, notice the ! doesn't have red background.

veganaize commented 4 years ago

@mildsunrise grep does NOT default to using color. You can find out if grep is being aliased by running the following command:

$ alias grep

Otherwise try echoing the following grep environment variables:

$ echo $GREP_OPTIONS $GREP_COLORS $GREP_COLOR

The culprit is probably the alias and it's probably set in .bashrc:

$ grep -i grep .bash*

Even when color is enabled grep also fails to set the background color, as you've demonstrated --but color output is disabled by default.

My specific issue is surrounded by the fact the node is changing the output color produced by MY program, an effect which I do not desire, in a confusing manner, without my specifying any such behavior..

aqilc commented 4 years ago

I think this can maybe be solved if you just set the background color to something else yourself, maybe by logging a code to change the background before execution.

This can definitely get taken into more consideration if more people support it by the way, but I don't feel it as necessary to go through and recode how node colors its output even though you can just change your background. I love my solid black terminal and the nice contrast I get from node's output without any tweaks at all xP

if you really want to push this feature, just contribute to node yourself :D

aqilc commented 4 years ago

then again, someone could just have a theme where your suggestion looks really messed up... which would omit the whole point of this xd

veganaize commented 4 years ago

@AqilCont am I not part of "everyone"? I suppose I'm just some nobody. I don't matter?

Node.js is welcome to break the decades old tradition of robust Unix-like utilities. That's fine. But I never asked Node.js to color MY index value in the console.log() statement in MY program. That's the primary problem.

That's unexpected behavior. That's obviously confusing. Even to nobodies, whose opinions don't matter, like me.

Node.js can go against the accessibility standards set forth by the world-wide standards body (W3C) when displaying it's own output -- but it should not do so with the output of individual programs unless specifically asked to do so.

With that, I rest my case.

veganaize commented 4 years ago

What a bunch of haters.

aqilc commented 4 years ago

nononono you're taking me too seriously! I meant to say that most people I have met, and most other devs on the internet are fine with with the current output, and I even brought up the fact that if more people support this, we can obviously bring a change. I was just saying that changing the whole colorization of the output is a bit of an undertaking, as there are obviously many things that node tries to bring out. I even brought up the fact that anyone can contribute, including you, to change this to your liking and add a sought out feature if it sits well with the team.

Also, with your most recent point, I think I misunderstood what you meant, because you brought up detecting the backgroind color and other things in the conversation, and I thought you were talking about entirely recoding the way Node output works. I had no intention of degrading or hurting you, and I am really sorry for the misunderstanding! With your recent argument, I think you should rephrase your original question to a simple, "Is there a way to stop Node from colorizing my output?", to which there might be more obvious and viable solutions all parties can agree upon.

veganaize commented 4 years ago

@devsnek decided to close the issue for no apparent reason. So obviously the "team" could care less about what I think or how I feel. Thank you for the actionable advice @AqilCont.

Major consultants are actually advising that people avoid using Node. I think I am beginning to understand why.

aqilc commented 4 years ago

I love node, and I can't recommend completely quitting just for this reason. I guess there's no point in staying if you really hate it, but keep in mind that there is a very large and supportive community behind Node outside of this repository too. You might not see it here, as not many people really come here and a lot of these issues are really unnecessary so the people who handle these repositories are just tired of dealing with people xd. I still apreciate them, as they do whatever they can most of the time and don't get any payment for for their efforts.

If you want more real and friendlier help, I would suggest asking a larger part of the community(maybe like reddit) or researching something yourself. This repository has never really ever really helped me with any real issues either(at least the one I opened), but I still come here when I want to help some people out sometimes.

aqilc commented 4 years ago

Oh btw, check out Deno too, it's really cool :D https://deno.land/

aqilc commented 4 years ago

this was closed because it's extremely off topic by the way(my best guess), not because anyone hates you or anything. I should stop commenting here too ngl...