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 735245 - Dim colors handled differently than in xterm, xterm makes more sense
Dim colors handled differently than in xterm, xterm makes more sense
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.37.x
Other Linux
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-22 20:25 UTC by Egmont Koblinger
Modified: 2014-08-25 20:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a handy test script (1.04 KB, text/plain)
2014-08-22 22:51 UTC, Egmont Koblinger
  Details
Fix (7.51 KB, patch)
2014-08-22 22:59 UTC, Egmont Koblinger
none Details | Review
Fix (7.39 KB, patch)
2014-08-22 23:05 UTC, Egmont Koblinger
committed Details | Review

Description Egmont Koblinger 2014-08-22 20:25:52 UTC
vte:  The dim (a.k.a. half) attribute is mutually exclusive with the bold attribute, setting one automatically clears the other.  Only the first 8 colors have dim counterparts (and as said, they can't be rendered in bold).  There's a hardwired corresponding_dim_index array, mapping the dim versions of the first 8 colors to other entries of the 256-color palette.  This has the weird consequence that e.g. redefining color 90 also redefines the dimmed version of color 5.

xterm:  The dim and bold attributes are orthogonal.  There's no mapping index, the dimmed color is automatically computed from the base RGB color using some hardwired formula, resulting in effectively 512 foreground colors.  And of course each of them can be made bold.

xterm's behavior is much cleaner and makes much more sense to me.  We already have the "half" attribute in the crowded VteCellAttr structure, so this is not an issue.  Also, this would make bug 678765 obsolete.
Comment 1 Egmont Koblinger 2014-08-22 22:51:35 UTC
Created attachment 284266 [details]
a handy test script
Comment 2 Egmont Koblinger 2014-08-22 22:59:04 UTC
Created attachment 284267 [details] [review]
Fix

This fixes the problem.

Note that an API call is removed because it no longer makes sense.  Can I squeeze this change into master before 3.14 now that we break the API anyways?  I wouldn't want to deprecate for years :)
Comment 3 Egmont Koblinger 2014-08-22 23:05:31 UTC
Created attachment 284268 [details] [review]
Fix
Comment 4 Christian Persch 2014-08-23 08:21:16 UTC
Comment on attachment 284268 [details] [review]
Fix

Put the test script somewhere in git, too.
Comment 5 Egmont Koblinger 2014-08-25 20:40:36 UTC
(In reply to comment #4)
> Put the test script somewhere in git, too.

I put it under 'perf' where we have other test scripts too.  We should rename or create another directory like 'tests' or so :)  Nevermind.