FS#7369 - rc scripts should check the $TERM variable and disable color on "dumb" terminals

Attached to Project: Arch Linux
Opened by Arthur Danskin (wiremore) - Tuesday, 05 June 2007, 19:07 GMT
Last edited by Tobias Powalowski (tpowa) - Thursday, 25 October 2007, 05:42 GMT
Task Type Feature Request
Category System
Status Closed
Assigned To Tobias Powalowski (tpowa)
Architecture All
Severity Very Low
Priority Normal
Reported Version 2007.05 Duke
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

It's nice to have colored rc script output, but I often use the emacs shell, which does not support escape characters. In this case, it would be nice if the rc scripts automatically disabled color. This can be easily implemented by changing the line:

if [ "$USECOLOR" = "YES" -o "$USECOLOR" = "yes" ]; then

to:

if [ "$USECOLOR" = "YES" -o "$USECOLOR" = "yes" -a "$TERM" != "dumb" ]; then

in /etc/rc.d/functions
This task depends upon

Closed by  Tobias Powalowski (tpowa)
Thursday, 25 October 2007, 05:42 GMT
Reason for closing:  Fixed
Additional comments about closing:  latest initscripts
Comment by Travis Willard (Cerebral) - Friday, 19 October 2007, 12:29 GMT
Should we be doing this on a per-term basis like that, or is there a generic way to determine if the current term supports escapes? I don't know if selectively disabling it term-by-term is the proper way to go about this.

I agree that colour should be disabled on terms that don't support it though, even if USECOLOR is yes.
Comment by Aaron Griffin (phrakture) - Friday, 19 October 2007, 17:28 GMT
This is waaay too complicated to do properly by checking the $TERM value. The best we can do is to trust the terminfo entry for the terminal.

This might work:
$(tput colors) -ge 8

In place of the explicit $TERM check
Comment by Travis Willard (Cerebral) - Friday, 19 October 2007, 17:46 GMT
I like that idea - it explicitly checks that the term can do what we want. Some random testing at work here in cygwin seems to support that as a good method:

$ tput colors
8
$ echo $TERM
rxvt
$ TERM=dumb tput colors
-1
$

Comment by Aaron Griffin (phrakture) - Friday, 19 October 2007, 22:01 GMT
Yeah the only issue is that this assumes a valid and proper terminfo entry which doesn't lie. But it's FAR better than checking the $TERM value
Comment by Oleg Finkelshteyn (olegfink) - Saturday, 20 October 2007, 07:36 GMT
But as -ge assumes integer on its input, you should first check if tput has returned an integer and not an error string.
For example, trying to query color support for a terminal which doesn't have a terminfo entry results in tput printing a string `tput: unknown terminal "X"' instead of printing `-1' for no colors.
Comment by Tobias Powalowski (tpowa) - Saturday, 20 October 2007, 10:12 GMT
would you please add a suitable patch here, thanks
Comment by Oleg Finkelshteyn (olegfink) - Saturday, 20 October 2007, 11:24 GMT
I'm not a bash guru, but probably that "if" line in /etc/rc.d/functions should read something like this:

if [[ ( $USECOLOR = yes || $USECOLOR = YES ) && `tput colors` > 7 ]] 2>/dev/null; then

Note the usage of `[[' and `>' here. It looks pretty crappy, but that's all I could think of.
Comment by Thomas Bächler (brain0) - Sunday, 21 October 2007, 08:25 GMT
fixed in initscripts-git, will be released to testing today or tomorrow.
Comment by Roman Kyrylych (Romashka) - Sunday, 21 October 2007, 14:34 GMT
Please test initscripts-2007.11-1 from Testing.
Comment by Oleg Finkelshteyn (olegfink) - Sunday, 21 October 2007, 16:36 GMT
That still doesn't work, perhaps because you do nothing when ${TERM_COLORS} is empty (and it's the case when $TERM is not in terminfo database).
Comment by Oleg Finkelshteyn (olegfink) - Sunday, 21 October 2007, 16:44 GMT
Also, the same thing could be done for /etc/skel/.bashrc which currently aliases ls to 'ls --color=auto', which is kinda annoying on terminals unaware of ansi color sequences.
Comment by Thomas Bächler (brain0) - Sunday, 21 October 2007, 16:46 GMT
Damn, I had screwed up logic when I wrote it. I'll apply a correct patch to git.
Comment by Thomas Bächler (brain0) - Sunday, 21 October 2007, 16:52 GMT
http://projects.archlinux.org/git/?p=initscripts.git;a=commitdiff;h=c5cffb501763c355299e593d16089fa5e56097b2

Apply this patch to rc.sysinit and try again. Sorry for my stupidness on that one.
Comment by Oleg Finkelshteyn (olegfink) - Sunday, 21 October 2007, 17:00 GMT
Sorry, but you're wrong again. [ doesn't like its lhs to be empty, so running this code with empty ${TERM_COLORS} will result in bash: [: integer expression expected printed to stdout.
You might want to use [[ instead of [ here, as [[ accepts empty strings as operands for -lt.
Comment by Thomas Bächler (brain0) - Sunday, 21 October 2007, 17:12 GMT
That code should never be called if TERM_COLORS is empty, that is what the whole

if [ -n "${TERM_COLORS}" ]; then

is for. The left operand in line 30 can never be empty.
Comment by Oleg Finkelshteyn (olegfink) - Sunday, 21 October 2007, 17:15 GMT
Sorry, that's my fault. I've read the patch wrong.
Now it's all right.

Loading...