Add bright (4-bit) colors to TextColor.ANSI

[keithkml]

Fixes #475

[keithkml]

[avl42]

I'd be curious, how well that works with "real" terminals, such as gnome-terminal, xterm, putty, tn3270, ...

I unfortunately won't get to try it until next week...

Keith Lea notifications@github.com schrieb am Sa., 9. Mai 2020 21:21:

[image: image] https://user-images.githubusercontent.com/7430512/81482976-c2928f80-9208-11ea-8ca7-bd637a41048b.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/476#issuecomment-626223686, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMSDHQXLIXTLAFIOUKTRQWUMXANCNFSM4M43F55A .

[mabe02]

If it's supported by most terminals, then why not. I think I didn't add this initially because it seemed like it wasn't well supported and for the ANSI colors, I wanted to make sure you could rely on them. But terminals implementing bright colors through the bold SGR state should reasonably also support this style of bright colors. Let's test it with a few terminals and see.

[keithkml]

FYI I tested with gnome-terminal and IntelliJ’s terminal toolwindow.

[avl42]

One detail I'd want changed, is that the new colors (the "_BRIGHT" ones) would be added AFTER all the existing ones. Inserting new enum-values is (in practice) even more of a potential incompatibility, than merely adding new ones at the end. (In theory, Java has some mechanisms to retain compatibility on the "ABI" level, but as soon as some piece of code gets recompiled, problems may still show up.)

On Sun, May 10, 2020 at 1:57 PM Keith Lea notifications@github.com wrote:

FYI I tested with gnome-terminal and IntelliJ’s terminal toolwindow.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/476#issuecomment-626316891, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMRLSNSQFPD52V32BBDRQ2JD5ANCNFSM4M43F55A .

[avl42]

And some more minor details: instead of casting first argument of each enum-item-construction to byte, the constructor could be changed to accept an "int" and only cast it on assignement to "index".

Then, there are a few cases of using constant 48 and explaining its meaning in an endofline comment. instead, one could just use '0' which also has value 48, but is self-explaining.

Both of these points were already in the original code, but if we're going to touch these parts, we could as well improve them some more.

Something like:

BLACK(0, 0, 0, 0),
RED(1, 170, 0, 0),
...
WHITE(7, 170, 170, 170),
DEFAULT(9, 0, 0, 0);

BLACK_BRIGHT(0, true, 85, 85, 85),
 RED_BRIGHT(1, true, 255, 85, 85),
...
WHITE_BRIGHT(7, true, 255, 255, 255),

ANSI(int index, int red, int green, int blue) {
   this.index = (byte)index;
   ...
}

On Sun, May 10, 2020 at 9:56 PM Andreas Leitgeb avlplus42@gmail.com wrote:

One detail I'd want changed, is that the new colors (the "_BRIGHT" ones) would be added AFTER all the existing ones. Inserting new enum-values is (in practice) even more of a potential incompatibility, than merely adding new ones at the end. (In theory, Java has some mechanisms to retain compatibility on the "ABI" level, but as soon as some piece of code gets recompiled, problems may still show up.)

On Sun, May 10, 2020 at 1:57 PM Keith Lea notifications@github.com wrote:

FYI I tested with gnome-terminal and IntelliJ’s terminal toolwindow.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/476#issuecomment-626316891, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMRLSNSQFPD52V32BBDRQ2JD5ANCNFSM4M43F55A .

[keithkml]

I agree with all that. Didn’t want to refactor the existing code and risk a controversial PR :) will make the changes you suggested.

[avl42]

I checked the escape sequences on a few more terminals (putty and xterm) and they all seemed to have the intended effect.

Also, I think it's good that the sequences are now pre-calculated.

[mabe02]

Let me try with Linux native terminal too. If that all works then let's do it.

[mabe02]

Yep, looks good!