[Home]

Summary:ASTERISK-12582: [patch] Optionally use black on white for the terminal settings
Reporter:Tilghman Lesher (tilghman)Labels:
Date Opened:2008-08-14 15:14:10Date Closed:2008-08-25 18:04:30
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20080814__bug13306__2.diff.txt
( 1) 20080814__bug13306__3.diff.txt
( 2) 20080814__bug13306.diff.txt
( 3) 20080814__optional_white_background.diff.txt
( 4) termcolors.patch.txt
Description:Jared Smith schrieb:
> On Thu, 2008-08-14 at 20:35 +0200, Philipp Kempgen wrote:
>> Whenever something spits out lines with a different background
>> color (of a varying runlength!) my eyes start to hurt.
>
> You can turn of the ANSI color support completely by adding
> "nocolor=yes" to the [options] section of asterisk.conf and then
> restarting Asterisk.

Sure. But I want colored output on my default background color.
:-)

ls with dircolors works perfectly. So I was curious if there
is a reason for Asterisk to behave differently (forcing the
background color to black).

Grüße,
Philipp Kempgen
Comments:By: Tilghman Lesher (tilghman) 2008-08-14 16:22:44

Here, this new patch should provide better colors.
By: pkempgen (pkempgen) 2008-08-14 17:11:53

I don't think the patch provides the behavior I had in mind. I mean: what
if somebody sets their terminal's default color to a dark red? or a light
blue? While int opposite(int color) looks like a good thing, ls or vim
don't do that. They just leave the background color as is and do not try
to find the opposite foreground color. I don't have to tell ls/dircolors
that whitebackground=yes - it works out of the box with the terminal set
to white background, black foreground.
By: pkempgen (pkempgen) 2008-08-14 17:14:14

OTOH I don't want to be a pain in the neck. :-)
By: Tilghman Lesher (tilghman) 2008-08-14 17:37:28

Well, the issue is that I couldn't find (in the 2 minutes that I looked) how to determine the color code of the native background.  If you can find that information, I'll be happy to see how I can make that work.

The issue that I'm dealing with on colors is that colors are embedded in many places throughout the code, and I chose to make the colors work on the two most common background colors.  In other words, the 'opposite' colors are not true color opposites, but rather tuned to what is legible on the screen.  If you pick another color, like red, for the background color, it's difficult to know how to translate the various existing colors in a way that makes them legible in a portable way, without hardcoding every single possible combination.
By: Tilghman Lesher (tilghman) 2008-08-14 20:42:41

I just checked and confirmed that ls(1) is not checking anything other than the environmental variable LS_COLORS to determine what colors to use.  So it does not change behavior whether the terminal is black on white or white on black.
By: Tilghman Lesher (tilghman) 2008-08-14 21:35:32

Okay, so I did a bit more research on ls(1) and came up with this solution.  It preserves the background color by not specifying it at all.
By: pkempgen (pkempgen) 2008-08-18 05:38:45

My termcolors.patch.txt works for me. :-) It's based on Corydon's
20080814__bug13306__3.diff.txt with minor modifications.
By: Tilghman Lesher (tilghman) 2008-08-25 11:00:44

pkempgen:  could you specify please what changes you made?
By: Tilghman Lesher (tilghman) 2008-08-25 11:09:32

Okay, nevermind.  I think I'm just going to go with my original changeset.  While there are sure to be many opinions on what the "correct" colors should be, we'll need to select a single standard one, and given the amount of time I've spent coming up with this solution, it might as well be mine.  ;-)
By: pkempgen (pkempgen) 2008-08-25 11:29:23

Wait a second and I'll try to explain. Actually it almost *is* your
changeset.
By: pkempgen (pkempgen) 2008-08-25 11:38:00

Here you go:

I changed the colors not to be the opposite in most cases because
notices and warnings come out in really strange colors then instead
of yellow and red.

I changed the
attr = ast_opt_light_background ? ATTR_DIM : ATTR_BRIGHT;
from your patch to
attr = ATTR_BRIGHT;  /* bold */
because not many terminals understand dim anyway but many implement bright
as bold (so a bold uppercase "[NOTICE]" is perfectly readable in yellow).

I restored the previous behavior: Output attributes only if there are any.

I undid the change to
printf("   -s <socket>     Connect to Asterisk via socket <socket> (only valid with -r)\n");
(you changed the order) which is completely unrelated.
By: pkempgen (pkempgen) 2008-08-25 15:07:26

The reason for not using opposite foreground colors on white background
is that other software (like ls or vim) doesn't do that either (except
for white->black of course). So currently that's the best thing to do.

I can confirm that my patch works in Apple's Terminal.app which defaults
to "xterm-color" emulation.
With Corydon's patch some "colored" text is rendered in black because the
terminal doesn't support the dim attribute (at least mine doesn't).

Any other testers?
By: Tilghman Lesher (tilghman) 2008-08-25 15:16:46

A couple of points:

1) Vim DOES change the colors, based upon the value of the "background" attribute.  The two values are settable to "dark" or "light", which is exactly how I modeled my colors.

2) Yellow on white is wholly unreadable, which is why I changed that.

3) If ATTR_DIM is unreadable on your terminal, I'm willing to change that to no attribute at all, but ATTR_BRIGHT on a light-colored background creates text that is difficult to read.  I want to improve readability, not decrease it.
By: pkempgen (pkempgen) 2008-08-25 15:39:01

> If ATTR_DIM is unreadable on your terminal

You don't have to make the patch so it looks best on *my* terminal. :-)
It's just that according to http://en.wikipedia.org/wiki/ANSI_escape_sequences
dim (faint) is "not widely supported" and all I can say is that it
holds true for my terminal.

> I'm willing to change that to no attribute at all

Ok.

> but ATTR_BRIGHT on a light-colored background creates text that is
> difficult to read. I want to improve readability, not decrease it.

True if code 1 was rendered as bright. According to the Wikipedia page
some terminals render it bright, some bold. Mine does bold text which
increases readability of short light-colored strings (like "[NOTICE]").

But Terminal.app might not be the most popular terminal. So if it works
on a normal X.org terminal, go ahead. Your patch improves the color
handling and fixes the main issue of this bug. Secondly you can't make
everyone happy, and terminal colors are not the most important thing in
the world. :-)
By: Digium Subversion (svnbot) 2008-08-25 18:04:29

Repository: asterisk
Revision: 139981

U   trunk/Makefile
U   trunk/doc/asterisk.8
U   trunk/include/asterisk/options.h
U   trunk/main/asterisk.c
U   trunk/main/term.c

------------------------------------------------------------------------
r139981 | tilghman | 2008-08-25 18:04:28 -0500 (Mon, 25 Aug 2008) | 7 lines

Optional light colored background, for those who use black on white terminals.
(closes issue ASTERISK-12582)
Reported by: Corydon76
Patches:
      20080814__bug13306__3.diff.txt uploaded by Corydon76 (license 14)
Tested by: Corydon76, pkempgen

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=139981