After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 168251 - add support for 256 colors terminals
add support for 256 colors terminals
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-02-23 12:33 UTC by dann
Modified: 2006-03-31 04:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Implement support for 256 colors (5.12 KB, patch)
2006-02-15 04:22 UTC, dann
none Details | Review
updated patch (5.99 KB, patch)
2006-02-15 06:59 UTC, dann
none Details | Review
updated patch, deal with DIM (7.59 KB, patch)
2006-02-16 22:16 UTC, dann
accepted-commit_after_freeze Details | Review
Fix VTE_DEF_{FG,BG} (8.13 KB, patch)
2006-02-21 18:19 UTC, dann
none Details | Review
Updated patch for 256 colors support (8.51 KB, patch)
2006-03-27 02:42 UTC, dann
committed Details | Review

Description dann 2005-02-23 12:33:59 UTC
It would be nice if vte supported 256 color terminals.

The CVS version of Emacs has support for 256 color terminals, with that support
there's little difference in appearance of text between an X11 and terminal
emacs window. 

xterm, rxvt and putty support 256 color terminals.
Comment 1 dann 2006-02-15 04:22:19 UTC
Created attachment 59389 [details] [review]
Implement support for 256 colors

The patch works. 
You can test it with the 256colors2.pl script from the xterm distribution.
Seems to work fine with CVS emacs (that has support for 256 color terminals).

Support for the xterm-256color setaf, setab terminfo entries was added.

It's unclear what to do with VTE_BOLD_FG VTE_DEF_HL and VTE_DIM_FG
They might even be dropped. Why support changing colors for "bold", when bold fonts are supported?
sizeof vte_charcell has increased. Can we steal 6 bits from "columns"?
Comment 2 Behdad Esfahbod 2006-02-15 04:35:24 UTC
Yes, we definitely can steal 6 bits from columns.  For columns, 1 bit is enough.  In fact there's absolutely no harm in making columns just as much as it needs to be.  About changing colors on bold, I tend to agree.  I'm not sure about highlight though.  I'll take a look and let you know.
Comment 3 Behdad Esfahbod 2006-02-15 04:52:44 UTC
I don't quite understand this part:

+	      b++;
+	      if (b > 5) { g++; b = 0;}
+	      if (g > 5) { r++; g = 0;}
+	      if (r > 5)        r = 0;

And also these:

-#define VTE_DEF_FG			24
-#define VTE_DEF_BG			25
+#define VTE_DEF_FG			0
+#define VTE_DEF_BG			15
 #define VTE_BOLD_FG			26
 #define VTE_DIM_FG			27
 #define VTE_DEF_HL			28
-#define VTE_CUR_BG			29
+#define VTE_CUR_BG			255

Does this mean that the colors in range 0 to 31 have changed now?
Comment 4 dann 2006-02-15 05:09:03 UTC
(In reply to comment #3)
> I don't quite understand this part:
> 
> +             b++;
> +             if (b > 5) { g++; b = 0;}
> +             if (g > 5) { r++; g = 0;}
> +             if (r > 5)        r = 0;

Ignore, dead code the I forgot to delete.

> And also these:
> 
> -#define VTE_DEF_FG                     24
> -#define VTE_DEF_BG                     25
> +#define VTE_DEF_FG                     0
> +#define VTE_DEF_BG                     15
>  #define VTE_BOLD_FG                    26
>  #define VTE_DIM_FG                     27
>  #define VTE_DEF_HL                     28
> -#define VTE_CUR_BG                     29
> +#define VTE_CUR_BG                     255
> 
> Does this mean that the colors in range 0 to 31 have changed now?

Yes they did. But the colors now are the ones used by xterm-256color

I don't think that colors over 16 were correct before. 
All xterm* terminfo entries have 8, 16, 88 or 256 colors, so those colors were not defined according to terminfo, they seemed to have been artifacts to support 
bold colors, etc.

Comment 5 dann 2006-02-15 05:20:19 UTC
(In reply to comment #3)
> -#define VTE_DEF_FG                     24
> -#define VTE_DEF_BG                     25
> +#define VTE_DEF_FG                     0

This should be OK. 

> +#define VTE_DEF_BG                     15

This might be needed to be changed to 7. 
The code that was using those 2 #defines that I disabled with an #if 0 would need to be changed a bit to allow for the "foreground" and "background" parameters to be used to set the default foreground and background color.


Comment 6 Behdad Esfahbod 2006-02-15 05:37:06 UTC
Can you attach a final patch please?  I really try to minimize my interaction with vte.  The code is scary huge...
Comment 7 dann 2006-02-15 06:59:13 UTC
Created attachment 59393 [details] [review]
updated patch

Updated patch.
Still not sure what to do about VTE_BOLD_FG  VTE_DIM_FG  VTE_DEF_HL
The code to handle them in vte_terminal_set_colors has been deleted in order to be able to create the 256 colors in the same way xterm does it.
The xterm terminfo entry does not support the "dim" capability, so I am not sure how the dim code is supposed to be used. Maybe when TERM is set to something else? If xterm does not need to support dim, IMHO vte shouldn't bother with it either.
Not sure what VTE_DEF_HL is supposed to do...
Comment 8 Behdad Esfahbod 2006-02-15 07:08:45 UTC
Yes, I think the dim capability is for other emulations that support it.  We cannot simply remove it!
Comment 9 dann 2006-02-15 07:40:11 UTC
(In reply to comment #8)
> Yes, I think the dim capability is for other emulations that support it.  We
> cannot simply remove it!

It seems that this code:  
	if (cell && cell->half) {
		if (*fore == VTE_DEF_FG) {
			*fore = VTE_DIM_FG;
		} else
		if ((*fore < VTE_COLOR_SET_SIZE)) {
			*fore += VTE_COLOR_DIM_OFFSET;
		}
	}

says that dim is only implemented for colors 0...7
Then creating an 8 element array that maps these colors to their dim versions
in the 256 color set should be equivalent with what was there before the patch.
Before the patch the dim colors where elements 16...23 in the palette array.
Comment 10 dann 2006-02-16 22:16:56 UTC
Created attachment 59533 [details] [review]
updated patch, deal with DIM

This is an updated patch. 
Some explanations: before this patch vte was creating 29 colors. 
The first 8 were the standard colors. The next 8 (at VTE_COLOR_BRIGHT_OFFSET) were the bright version of the 8 standard colors. These first 16 colors are the same after the patch too.
The colors 16 -> 23 (at VTE_COLOR_DIM_OFFSET) were the dim versions of the standard colors.
Now that we have 256 colors there's no point in creating extra dim colors, just use the existing 256 colors and use "corresponding_dim_index" to map the 8 standard colors to their dim versions.
The colors VTE_DEF_FG VTE_DEF_BG VTE_BOLD_FG and VTE_DIM_FG (the old 24->28 colors) don't need to be separate duplicate colors, just point them to the corresponding place in the 256 colors array.

VTE_COLOR_SET_SIZE was renamed to VTE_LEGACY_COLOR_SET_SIZE

I am happy with the patch now, it should be ready to be applied.
Comment 11 Behdad Esfahbod 2006-02-16 22:49:51 UTC
Looks good to me.  I'm going to commit this soon after we branch for gnome-2-14.
Comment 12 dann 2006-02-21 18:19:13 UTC
Created attachment 59871 [details] [review]
Fix VTE_DEF_{FG,BG}

It seems that in the GUI is possible to change the default foreground and background to arbitrary values, with the previous version of the patch this would have changed the color definitions for the colors VTE_DEF_FG and VTE_DEF_BG, so it these need to be distinct colors. 
Adding them increases the size of the palette array to over 256, so the size of  vte_charcell.{fore,back} needs to be increased.

This version of the patch also restores VTE_DIM_FG, VTE_BOLD_FG and VTE_DEF_HL
as independent colors because they can be changed with functions declared in vte.h. Those functions do not seem to be used anywhere, so maybe they can be removed at some point...
Comment 13 Behdad Esfahbod 2006-02-21 23:21:10 UTC
Those functions are part of the public API of vte and applications (gnome-terminal for example) can call them.
Comment 14 dann 2006-03-17 18:02:29 UTC
Is there anything I could do to help have this patch committed? 
Thx. 
Comment 15 Behdad Esfahbod 2006-03-17 18:19:16 UTC
Man, we are not even 48 hours past GNOME 2.14.0 release yet!
Comment 16 dann 2006-03-23 02:46:01 UTC
(In reply to comment #15)
> Man, we are not even 48 hours past GNOME 2.14.0 release yet!

I'm sorry, I didn't want to sound pushy. I have some free time now, and I might not have any in the near future. So if there's anything that I'd have to do, I should do it now. 
Note the I don't use vte (or gnome-terminal). I implemented a part of the 256 colors support in Emacs CVS, so my interest in this comes just from trying to make 256 color support available to as many people as possible.
Comment 17 Behdad Esfahbod 2006-03-23 11:21:21 UTC
Ok, I'll ask peope to test it.
Comment 18 Ed Catmur 2006-03-27 00:44:52 UTC
Patch does not apply against 0.12.0. Dann, could you fix this?
Comment 19 dann 2006-03-27 02:42:51 UTC
Created attachment 62083 [details] [review]
Updated patch for 256 colors support

Well, it looks like some parts of vte.c have moved to other files...
I attached an updated patch.
Comment 20 Ed Catmur 2006-03-27 04:54:36 UTC
Works well, only one bug: gnome-terminal spews "WARNING **: No handler for control sequence `device-control-string' defined." to stderr, seemingly every time a color control sequence is used.

Since the terminfo entry still has only 8 colors, in vim I have to :set t_Co=256 to be able to use 256 color schemes. What would be the best way to get the message across that vte/gnome-terminal is a 256-color terminal? Seeing as 2.14 is the first gnome-terminal to default TERM="gnome", could we get ncurses & distributors to fix the terminfo entry?

Of course, xterm has capability reporting... I'm guessing that would be a fair amount of work to implement.
Comment 21 dann 2006-03-27 17:59:50 UTC
(In reply to comment #20)
> Works well, only one bug: gnome-terminal spews "WARNING **: No handler for
> control sequence `device-control-string' defined." to stderr, seemingly every
> time a color control sequence is used.

I tried this on my FC5 system and I didn't get any warning with either of a 256 color enabled emacs, or vim-6.4. 

I then installed the vim7 rpm from http://people.redhat.com/karsten/ and I got the above warning for an unpatched gnome-terminal when starting vim7. 
So the above warning has nothing to do with this patch as it occurs with stock vte-0.12.

Comment 22 Ed Catmur 2006-03-27 18:22:33 UTC
Right, yes, sorry. It only happens when starting vim with TERM="xterm", actually.
Comment 23 Behdad Esfahbod 2006-03-31 03:59:47 UTC
Ok, trying to test it and then commit.
Comment 24 Behdad Esfahbod 2006-03-31 04:16:02 UTC
Committed, thanks!

2006-03-30  Behdad Esfahbod  <behdad@gnome.org>

        Bug 168251 – add support for 256 colors terminals
        Patch from dann@godzilla.ics.uci.edu.

        * src/vte-private.h, src/vte.c, src/vteseq.c: Implement support
        for 256 colors.