Obsolete CPU_NICE_TEXT and MEMORY_BUFFERS_TEXT colors

[Explorer09]

These two color constants existed only because in the default color scheme, the meter colors (CPU_NICE and MEMORY_BUFFERS) are defined to ColorPair(Blue, Black) are were difficult to read for terminal text. There is no need for the color exceptions if CPU_NICE and MEMORY_BUFFERS are A_BOLD | ColorPair(Blue, Black). This commit defines the said meter colors to bold blue and obsoletes the CPU_NICE_TEXT and MEMORY_BUFFERS_TEXT color constants.

Update: screenshot

[BenBE]

Due you have a pair of screenshots to illustrate the difference? TIA.

[Explorer09]

@BenBE I capture these on a VirtualBox machine. The upper half is before and the lower half is after,

Text terminal (Ctrl-Alt-F[1-6]) (This screenshot is outdated)

Xfce terminal (my Linux distribution is Devuan GNU/Linux 3 (beowulf)) (This screenshot is outdated)

[BenBE]

Okay, that much for the bar-part of the change. Do you also have a screenshot showing the text-part of that change?

But from the current screenshots this much: I don't quite like this visually as the bold includes some highlight that makes this stand out quite a bit.

[Explorer09]

@BenBE The text part is identical to the original. It remains light/bold blue color.

My purpose of this patch is to unite the two colors into one for both pairs (CPU_NICE_TEXT -> CPU_NICE and MEMORY_BUFFERS_TEXT -> MEMORY_BUFFERS), so as to increase consistency on the color usage. The blue & light blue color distinction is used only in two color schemes: Default and Black Night. Other schemes don't utilize the distinction and define the color to the same.

[BenBE]

While I see a good reason for the text meter mode for the brighter colors I don't see much benefit for the bar meter mode. IMHO it even looks strange to the point where it distracts as it causes an emphasis on a detail of usually little relevancy.

[fasterit]

-1 from DLange as well. The blue on black looks less "alerting" than the bold blue on black.

[Explorer09]

I've added a commit to handle bold blue on black background as a special case so that bars would draw without the bold.

And there is a side effect for the change which I consider a feature: (screenshot)

From top to bottom: Xfce terminal - before Xfce terminal - after Text console (Ctrl-Alt-F1) - before Text console (Ctrl-Alt-F1) - after

[BenBE]

I think this looks a bit more reasonable. Let's see what others think about it.

[Explorer09]

Additional note: I intend the new function CRT_getBarGraphicColor to be also applied to the colored graph display (#714), and that's why it's a separate function and the logic is not merged in BarMeterMode_draw.

[Explorer09]

Here is one question: When I rewrote the BarMeterMode_draw() routine, I noticed the blockSizes array was defined with a magic number length of 10. I think the number should be a macro constant. Is it okay if I name the macro MAX_METER_ITEM_COUNT, or MAX_METER_ITEMCOUNT (no underscore between last two words)? For now it won't be needed in the rewritten BarMeterMode_draw() routine, but I might need it in some assertion statements in the new code of (#714).

[BenBE]

Is it okay if I name the macro MAX_METER_ITEM_COUNT?

Sounds reasonable, go ahead.

[BenBE]

Please don't mix those variables, but instead introduce a new color for the highlighted text in bar mode of the meter. For most color schemes this will be the usual color also used for the bar graph, except for the color schemes where the bar graph is blue; in that case the bar text color should read as bright blue on background instead.

Mixing those variables as is currently done unnecessarily mangles things that should not be mixed together.

[Explorer09]

Please don't mix those variables, but instead introduce a new color for the highlighted text in bar mode of the meter. For most color schemes this will be the usual color also used for the bar graph, except for the color schemes where the bar graph is blue; in that case the bar text color should read as bright blue on background instead.

Mixing those variables as is currently done unnecessarily mangles things that should not be mixed together.

I don't know what you mean. Did you mean keeping the CPU_NICE_TEXT and MEMORY_BUFFERS_TEXT color constants so that new {METERITEM}_TEXT color constants can potentially be introduced? Or do you mean introducing a constant like BAR_HIGHLIGHT_BLUE that would apply to all bar items with blue colors (regardless of item meaning)?

[BenBE]

Please don't mix those variables, but instead introduce a new color for the highlighted text in bar mode of the meter. For most color schemes this will be the usual color also used for the bar graph, except for the color schemes where the bar graph is blue; in that case the bar text color should read as bright blue on background instead. Mixing those variables as is currently done unnecessarily mangles things that should not be mixed together.

I don't know what you mean. Did you mean keeping the CPU_NICE_TEXT and MEMORY_BUFFERS_TEXT color constants so that new {METERITEM}_TEXT color constants can potentially be introduced?

Yes.

Or do you mean introducing a constant like BAR_HIGHLIGHT_BLUE that would apply to all bar items with blue colors (regardless of item meaning)?

No. Have a new {METERITEM}_TEXT which defines the color used for the text overlaying the colored portion of the bar.

[Explorer09]

Have a new {METERITEM}_TEXT which defines the color used for the text overlaying the colored portion of the bar.

Well, this is tricky.

1. The request Is totally the opposite of what my PR tries to achieve. (I think I'll close this one and open another PR to implement the request.)
2. The MeterClass currently only has one member (attributes) for specifying the item colors. Does this mean there would be two members (e.g. attributesForText and attributesForGraph) for the colors of text and graphic separately? I'm not sure if this is a good idea, but if @BenBE wants to implement things this way, so be it.

[BenBE]

The idea came about in an discussion with @fasterit today.

Background is that merging the flags as you did with that patch causes some localized special treatment, that isn't obvious from the location where it's implemented alone.

Thus while the intention of having the dark blue text more readable is agreed upon, the way to reach this differs a bit. I think @fasterit can present a few more details if necessary; in particular from the coding point of view.