patchGNU nano - Patches: patch #10131, add support for specifying RGB...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #10131: add support for specifying RGB colors

Submitted by:  Brad Town <townba>
Submitted on:  Tue 02 Nov 2021 03:57:29 AM UTC
 
Priority:  5 - Normal Status:  Done
Privacy:  Public Assigned to:  bens
Open/Closed:  Closed Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

( Jump to the original submission )

Tue 23 Nov 2021 11:18:01 AM UTC, comment #15: 

The patch has been applied in commit 8d6b468c.

Benno Schulenberg <bens>
Project Administrator
Mon 15 Nov 2021 09:40:58 AM UTC, comment #14: 

The signed-off patch looks good.  Ready to post.

Indeed, adding a 3 to the shebang line works better.  Thanks.

(I've tried with also showing blocks of background color, but... they are too loud: they "drown" the #xxx codes, and these codes are what matter.  The blocks also give a wrong impression of the color: for example, the #40x colors as blocks look quite good, but for coloring text, for which these colors are primarily meant, they are unusable: too dark -- on a dark background, that is.)

Benno Schulenberg <bens>
Project Administrator
Sun 14 Nov 2021 06:33:18 PM UTC, comment #13: 

Signed-off patch attached with message format similar to the ones you've used. (Haven't done this with git before, so please forgive me if I missed something.)

The script is a nice idea. Looks good to me, though you might want to change the shebang line to specify python3 (some systems, like macOS Big Sur, still default to python2). You could also display the colors as backgrounds as well, but I think it's fine as it is.

Let me know if I made the patch correctly and I'll post it to bug #56445 and send it to nano-devel.

(file #52278)

Brad Town <townba>
Sun 14 Nov 2021 12:24:49 PM UTC, comment #12: 

Thanks for the snippet of color-slices code.  It's amazing how many shades of color my eye can distinguish when they are right next to each other.  (The direct color seems to work fine on my Xfce terminal, but shows just mostly blue and greenish vertical bars on an xterm, version 330.)

Your new patch...  I don't like the 'lv/2 & 0x07' parts -- the idea was to do no computation and just let the array do the work.  So attached patch restores the 16-element array.  I also don't like the uint8_t, and have used shorts instead, and have changed a comment and reshuffled another.  If you can agree with this version, please provide a signed-off patch with a relevant commit message.  Post the patch also on bug #56445.  And post it to nano-devel too, if you will, so people will know what is coming up.  (Or if you want, I can post it there.)

When announcing the next version of nano (in about a month), I intend to attach the attached little Python script to the announcement.  What do you think?  Any improvements that can be made?

(file #52273, file #52274)

Benno Schulenberg <bens>
Project Administrator
Sat 13 Nov 2021 10:38:00 PM UTC, comment #11: 

Just realized I was multiplying by the wrong number to get from [0, 1] to the offset thresholds -- I should have been using (n – 1) instead of n. So the correct offsets for the eight-item array are … exactly what I have in my patch. Huh.

(For the record, I made a similar error with the previous 16-item array, which should have been [0, 0, 0, 0, 1, 1, 1, 2, 2, 2, 3, 3, 4, 4, 5, 5]. By chance, it also looks better from a code standpoint.)

(Getting the halfway "tipping point" index between offset 0 and offset 1 for an eight-item array:

>>> linear_to_sRGB((sRGB_to_linear(0x00 / 0xFF) + sRGB_to_linear(0x5F / 0xFF)) / 2) * 7
1.8570990262654143

So if the array index is below that result, like 0 or 1, we should get the lower value, and if it's higher, we should get the higher value.)

Brad Town <townba>
Sat 13 Nov 2021 08:59:48 PM UTC, comment #10: 

New patch. Dropped the variables, renamed and shortened the array, dropped a comment, dropped "static", renamed the function, and added an "&" to ensure we don't accidentally walk off the end of the array.

To get the offsets, I converted the color cube's levels to linear space, found midway points between neighbors, and converted back. Calculating using the levels directly isn't right. (An extreme example: The level between levels 255 and 0 is about 187.5, not 127.5. http://www.ericbrasseur.org/gamma.html#explanation has a decent explanation. The level midway between 0 and 0x5F is almost 0x44, so level 0x33 is closer to 0 than to 0x5F.)

However, like you pointed out, nano doesn't need precise. Assuming you'd prefer a more regular distribution of offsets and ease of user understanding to exacting color matching, I modified the array further. The change introduces additional "error" (beyond my previous patch) only in the 0xE case (so something like 0xEFE and 0xFFF are now displayed the same). It's perhaps a bit easier for users to understand when they encounter it as well. Many of the colors are now a bit brighter than they "should" be, but the array "should" be [0, 0, 0, 1, 2, 2, 3, 4], which would obviously be worse because it drops 5. My cube-slice code is below if you'd like to compare how the colors look vs. direct-color mode. I'll also attach a zip with some screenshots.

That one function was static because it was static in my test code and isn't used anywhere else. Of course it doesn't need to be static.

ANSI defined the first eight colors. AIX extended the list to 16, and the 6x6x6 color cube was a patch to XTerm with offsets chosen based on ease of calculation and what the author thought looked nice. Calling it index_color is fine by me.

As for using color_content to get the colors, it doesn't work (I tried) because nano doesn't initialize the colors to anything known: https://stackoverflow.com/a/49139882 If you'd like to see what XTerm displays (and what's been copied by other terminal emulators) for colors 16+, the levels are here: https://github.com/ThomasDickey/xterm-snapshots/blob/master/256colres.h

Here's the code I use to dump slices of the color cube to compare nano's colors to direct-color colors. (You need an emulator that supports direct-color mode, like XTerm or iTerm2).

        { /* Display cube slices for 256-color and direct-color modes. */
                printf("\r\n");
                for (int k = 0; k < 16; ++k) {
                        for (int j = 0; j < 16; ++j) {
                                for (int i = 0; i < 16; ++i) {
                                        int curses_color = closest_index_color(k, i, j);
                                        printf("\e[48;5;%dm  ", curses_color);
                                }
                                printf("\e[m  ");
                                for (int i = 0; i < 32; ++i) {
                                        printf("\e[48:2::%d:%d:%dm ", k * 16, i * 8, j * 16);
                                }
                                printf("\e[m\r\n");
                        }
                        printf("\r\n");
                }
                exit(0);
        }

(file #52270, file #52271)

Brad Town <townba>
Sat 13 Nov 2021 11:12:06 AM UTC, comment #9: 

Thanks.  Things begin to look good.  Personally I would drop the intermediate variables ro, go and bo, give the array a short name, so that it becomes a straightforward 'return 36 * level[red] + 6 * level[green] + level[blue] + 16', but that is a detail.  Other than that, some questions.

Shouldn't the "4, 4, 4, 5" be "4, 4, 5, 5"?  Because ee is closer to ff than to d7.  But even if things look better gammawise with the current values, I think having a more predictable mapping between the 0-f values and the 0-5 offsets is easier for the user.  (The first two offsets are mapped from four values each, and the other four offsets from two values each.)  (If one were to really go for closest value, then 33 is closer to 5f than to 00, and 77 closer to 87 than to 5f, and 99 closer to 87 than to af -- so the array would have to be: { 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 4, 5, 5 }.  But I prefer a more regular distribution.)

The "encroach a bit into black" comment doesn't make sense to me.  Some context seems to be missing.  But I don't think such details should be in the comments, but in the commit message (if at all).

Why is the closest_color() function declared static?  No other function of nano is declared static.  Why this one?  What does it bring?

Why do you call it curses_color?  When looking at the ANSI color stuff [1], it would seem that these 216 colors were defined by ANSI.  But there does not seem to be any reference document for it.  And if the Ncurses FAQ [2] is correct (search for "colour table"), then the ANSI documents give no clue about what this table should look like, and the actual 216-colors and 24-grays tables were invented by the maintainer of xterm.  So one could call it xterm_color.  But I prefer a more neutral name like index_color or palette_color.

[1] https://en.wikipedia.org/wiki/ANSI_escape_code#8-bit
[2] https://invisible-island.net/ncurses/ncurses.faq.html

If you already have a piece of code that tries all the #rgb values (to see how they translate to indexes and actual colors), can you modify this code to dump the actual rgb value that each indexed color contains in the 216-entries table, and post the code here?  It seems that ncurses has a function for this: color_content().

Benno Schulenberg <bens>
Project Administrator
Fri 12 Nov 2021 05:26:42 PM UTC, comment #8: 

How about this patch?

  • Dropped the multiply-by-17 bit and reduced the array down based on that.
  • Changed the offset for color level 4 from 0 to 1 to ensure we don't clip too much.
  • Dropped the level_to_offset function. Moved the array into closest_curses_color (renamed from closes_index_color).
  • Added a string length check to verify color format.

What do you think?

(For testing, I modified some nanorc files and try those. I also have some chunks of code I paste in: one dumps slices of the color cube, one dumps all the named colors, one tries all the #rgb colors, and one dumps a rainbow and gray ramp.)

(file #52262)

Brad Town <townba>
Fri 12 Nov 2021 11:52:48 AM UTC, comment #7: 

Thanks for the patch.  And thanks for including the short #rgb format -- I think that short format fits nano much better.  Plus: the #rrggbb format gives the false impression of being able to specify colors very precisely, so I would drop that entirely.

About avoiding the blue colors for color numbers 8 to 15 on a direct-color terminal...  It is also an issue in current nano and is thus unrelated to adding support for the 216 index colors.  It should therefore be a separate patch (if we want to fix this at all, as no one has complained about this yet).

About the grays...  I don't know how you test, but if I add the following lines at the end of my .nanorc:

extendsyntax default color #666 "this|that"
extendsyntax default color #888 "these|those"
extendsyntax default color #aaa "here|there"
extendsyntax default color #ccc "more|less"

extendsyntax default color #f66 "Pico|editor"
extendsyntax default color #7f7 "\<the\>"
extendsyntax default color #88f "\<(and|it|in|on|or)\>"

extendsyntax nanorc color yellow "#[0-9A-Fa-f]{3}\>"

and then paste those same lines into the README file, and then run a nano with your patch and (in a second terminal tab) a nano with the attached simpler patch, both with README as argument, and then flip back and forth between the tabs, I can see some of your grays are slightly, slightly lighter.  But... when using these colors to accentuate text (what these colors are about), would you be able to tell the difference between your #aaa gray and your #bbb gray if the affected words were two lines apart?  I can't.  And I'm pretty sure I would use at most two levels of grey (if I were to use grey at all) in a syntax definition.  In short: I think giving the user the ability to specify such precise levels of grey is unneeded.

So, I propose something like the attached patch: not changing anything that doesn't /need/ to be changed (for now).  This patch can be simplified further, because first multiplying with 0x11 and then shifting three bits right...  All that is needed is an array of 16 items, not 32.

(file #52259)

Benno Schulenberg <bens>
Project Administrator
Thu 11 Nov 2021 09:54:54 PM UTC, comment #6: 

New version of the patch attached.

  • Simplify the colors array.
  • Simplify getting the 256-color curses colors.
  • Strip out the direct-color stuff. (Now it just avoids problems -- direct-color mode only has the first eight ANSI colors, not the brights or color cube or gray ramp.)
  • Accept both #rrggbb and #rgb for color.

It doesn't include changes to other stuff like documentation or syntax highlighting. (I did verify that nanorc.nanorc can easily support the new colors, though. Moving sections "Strings" and "Comments" before "Color names", then adding "|#([0-9A-Fa-f]{3}){1,2}" after both "|gray" in two places, seemed to do the trick, but I did not test it exhaustively.)

Grays: I tried it without. The relatively chunky color cube clips a lot of darker colors to black. In the end, I put the ramp back in but used the lookup-table approach -- six of the grays fill in what would otherwise be just black. I could have limited it to just the dark colors, but it's actually simpler to just do it for all the grays.

ncursesw: The new patch doesn't use init_extended_pair. However, I looked into it. When ncurses is compiled for the version 5 ABI, it leaves out some things, including init_extended_pair, because they require changes to underlying structures.

Direct color: I don't think I'm going to bother.

(file #52241)

Brad Town <townba>
Tue 09 Nov 2021 10:31:04 AM UTC, comment #5: 

Okay.  I'm looking forward to the first patch.  :)

About the gray-scaling...  I really wouldn't bother with that.  These color specifications are meant for syntax highlighting, so people will want to see some contrast between the different colors, not very subtle different levels of gray.  The latter are nice when simulating graphics on a terminal, but I think they are pointless for coloring text.

About wide curses...  Of course I have been using UTF-8 for years, and my nano is linked against libncursesw.so.5.  Why this library does not contain 'init_extended_pair', I don't know.  On a newer machine, nano links against libncursesw.so.6, which does export 'init_extended_pair'.

Splitting the thing up in different patches sounds good: having the 256-color case work will probably satisfy more than ninety five percent of the users.  So just having that patch would be enough, I think.

Benno Schulenberg <bens>
Project Administrator
Tue 09 Nov 2021 02:34:12 AM UTC, comment #4: 

I tried the lookup-table approach. It’s visually good, easier to understand, and fast (assembly output for both ARM and x86-64 shows around three or four instructions), so I’ll go with that approach.

To get better grays, I have a similar lookup table for the gray ramp. It has 32 entries and looks pretty good. (It’s most important for grays near black to minimize clipping, but it also improves lighter grays pretty much for free.)

I’m thinking I could break it up into two patches. The first would handle 256-color terminals, and the second would add support for direct-color terminals. The second is the one that would need init_extended_pair and would be lower priority. (I believe few people are using such terminals [e.g., xterm-direct] because programs often assume that COLORS >= 256 means they can use the 256-color mapping, which results in all colors but 0–8 looking like very dark blue.) What do you think?

(By the way, rereading my previous message, I think it sounds like I was being sarcastic about ncursesw being needed for init_extended_pair. I was serious – I was very surprised that the wide-character version of the library was needed because they seem to have nothing to do with each other. It turns out the changes made to support wide characters were useful for extended color support.)

Brad Town <townba>
Mon 08 Nov 2021 06:11:30 PM UTC, comment #3: 

Yes, I will. I was out of commission last week and couldn’t work on it.

I’ll drop the documentation changes and simplify the code – I might keep some simple version of getting a better gray when ri==gi==bi (we know in those cases the ramp will always be at least very close but often much better, particularly near black). I’ll also go with the #xxxxxx format for colors, which is what I had at first (and is actually the format I added to tmux’s configuration a few years ago). I’ll try your lookup table suggestion as well.

If the ncurses library your machine is linking doesn’t export init_extended_pair, it’s probably because the ncurses library getting linked isn’t ncursesw (which – surprise! – is needed to get that function). I’ll tweak configure.ac to add a proper check for that function, but in the meantime, if you configure nano with --enable-utf8 (which appears to have nano link with ncursesw) it might work.

Brad Town <townba>
Sun 07 Nov 2021 01:54:43 PM UTC, comment #2: 

Hello Brad,

Will you post a trimmed-down patch?

Benno Schulenberg <bens>
Project Administrator
Wed 03 Nov 2021 11:20:05 AM UTC, comment #1: 

Thanks for the patch.  ...  It seems you have recreated Brand Huntman's work in a different way.  (See the long thread that starts at https://lists.gnu.org/archive/html/nano-devel/2018-02/msg00068.html, and the discussion that triggered it: https://lists.gnu.org/archive/html/nano-devel/2017-12/msg00129.html, continued in https://lists.gnu.org/archive/html/nano-devel/2018-01/msg00005.html.)

I am quite willing to add support for more colors.  But the implementation should be simple and small and straightforward.  The initial patch should do only what is strictly necessary to get the extra colors to work -- no documentation, no options, no optimizations, no syntax changes -- all those can come later, in subsequent patches.

When I look at the 216 indexed colors produced by the attached Python script, then for many hues I cannot see the difference between two adjacent ones (colorwise, not indexwise) -- for some pairs yes, but for many not.  So to me it would seem that being able to specify just these 216 colors would be enough for a small editor like nano.  However, the index numbers give no clue about what color they produce.  So it will be much nicer to allow the user to specify RGB numbers and that nano converts these numbers to the relevant indexes -- when on a 256-color terminal, which seems to be the default on the few terminals I have tried.

The conversion, however, must be fast.  There is no need to find the color that is absolutely the closest, a rough but fast approximation is good enough, as long as it's easy to hit all 216 index colors.

About the way to represent the RGB hex values...  I don't like the rgb:xx/xx/xx form -- it is too verbose.  Also, on the net, when using the word "RGB", mostly decimal numbers are used (see for example https://www.w3schools.com/colors/colors_rgb.asp), and when using the word "hex",  mostly the form #xxxxxx is used (see for example https://www.w3schools.com/colors/colors_hex.asp).  So, using that latter, concise form seems much better to me.  (It causes some trouble with getting colorized as comments by the current syntax, but that can be fixed.)

Some comments about a detail of your implementation.  The 'rgblevel_to_6x6x6' function returns two values, but the 'out' value is used only when the color appears to be a grey one, which I think will be quite seldom (we're doing color syntax highlighting, after all), so returning that value seems a waste of time in most cases.  Also, I simply wouldn't bother with the grey-scale values at all, the four #5f5f5f, #878787, #afafaf, #d7d7d7 are good enough in my opinion -- if the user really needs more nuanced grey tones, they should switch to a more capable terminal.  And when the terminal cannot do even 256 colors, then I wouldn't do any conversion at all: just drop any color that is beyond the supported number, just like is done now.

"Fast" and "short" are the words that should describe the implementation.  So, when converting, I would just drop the lower nibble of each of the three components, and use the upper nibble as an index in a short array, to convert it to a 0..5 value.  So: three masks, three shifts, three array accesses, and then two multiplications and three additions.  Done.  Would that be too inaccurate?

(By the way, your patch does not compile on my machine: color.c:60: undefined reference to `init_extended_pair'.  The ncurses patch level on this machine claims to be 20180127.)

(file #52193)

Benno Schulenberg <bens>
Project Administrator
Tue 02 Nov 2021 03:57:29 AM UTC, original submission:  

Improve color support

Overview

This addresses bug #56445 and provides significantly improved color support.

Colors can be specified using a syntax like rgb:9A/7C/5E. If ncurses is using direct-color mode such as with terminfo entry xterm-direct, it will use the color mostly as specified. If ncurses is using a 256-color mode, it will match the specified color with a color known to be part of the 256-color palette. If ncurses is using an 8- or 16-color mode, it will match the specified color with one of the 16 colors available unless disabled using a new setting or command line switch.

Example use

(I am not advocating for any of the following settings. These are merely examples.)

.nanorc line to set title to white on brighter white:

set titlecolor rgb:AA/AA/AA,rgb:EE/EE/EE

Adjustment to c.nanorc to change color of constants and labels:

# Constants.
color rgb:9D/BE/D9 "\<[A-Z_][0-9A-Z_]*\>"
# Labels.
color rgb:75/B3/DB "^[[:blank:]]*[A-Z_a-z][0-9A-Z_a-z]*:[[:blank:]]*$"
color normal ":[[:blank:]]*$"

To test nano with different terminfo entries (some of these might require a more
up-to-date terminfo database):

TERM=xterm-color nano ~/.nanorc
TERM=xterm-16color nano ~/.nanorc
TERM=xterm-256color nano ~/.nanorc
TERM=xterm-direct nano ~/.nanorc

Considerations

Use extended ncurses functions and terminfo to provide color support to keep code simple
and portable.

Avoid calculating closest colors where possible and reasonable.

Be wary about how to use RGB values and linear color space to ensure more accurate
brightness.

Changes

Code

  • Convert existing nano colors such as pink and lime to RGB equivalents to expand their availability to terminals with more than (or fewer than) 256 colors.
  • The 16 ANSI colors are not available in 16M-color terminals. For example, xterm-direct only provides the first 8 ANSI colors.
  • Match RGB colors to the closest colors and grays available for 8-, 16-, and 256-color terminals.
  • Match RGB colors to the closest colors available for 8- and 16-color terminals. Allow the user to disable such matching using new setting nocolormatchansi and switch --nocolormatchansi (*-8*) because the terminal colors may not be sufficiently close to what the user expects.

Info and manpage

  • Add descriptions of setting nocolormatchansi and switch --nocolormatchansi (*-8*).
  • Add description of specifying colors using RGB hexadecimal values.

Syntax highlighting for nanorc

  • Add highlighting for RGB colors and new option nocolormatchansi.

Testing

Testing was performed using both Xterm and iTerm2. ncurses 5.4 and 6.2 (patch 20200212) were tested as well. Several terminfo entries were used, including xterm-color, xterm-16color, xterm-256color, xterm-direct, iterm2-256color, and iterm2-direct.

Possible later work

The setting nocolormatchansi and switch --nocolormatchansi (*-8*) could be dropped in favor of never or always doing the color matching.

Use init_color and/or init_extended_color to set colors to those specified without using color matching. Note that this changes colors for the terminal itself and the colors might not be easily or reliably restored upon program exit.

Brad Town <townba>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #52273:  less-computation.patch added by bens (1KiB - text/x-patch)
file #52274:  colorcodes.py added by bens (386B - text/x-python)
file #52270:  patch1e.patch added by townba (1KiB - application/octet-stream)
file #52271:  example-cube-slices-1e.zip added by townba (68KiB - application/zip)
file #52262:  patch1d.patch added by townba (1KiB - application/octet-stream)
file #52259:  simpler.patch added by bens (1KiB - text/x-patch)
file #52241:  patch1b.patch added by townba (7KiB - application/octet-stream)
file #52193:  colors.py added by bens (711B - text/x-python)
file #52181:  0001-Improve-color-support.patch added by townba (22KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Updated the item)
  • -email is unavailable- added by townba (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 16 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-11-23 bens StatusIn Progress => Done
        Open/ClosedOpen => Closed
        SummaryAdd support for specifying RGB colors => add support for specifying RGB colors
    2021-11-14 townba Attached File- => Added 0001-rcfile-support-rgb-format-for-specifying-colors-in-2.patch, #52278
    2021-11-14 bens Attached File- => Added less-computation.patch, #52273
        Attached File- => Added colorcodes.py, #52274
    2021-11-13 townba Attached File- => Added patch1e.patch, #52270
        Attached File- => Added example-cube-slices-1e.zip, #52271
    2021-11-12 townba Attached File- => Added patch1d.patch, #52262
    2021-11-12 bens Attached File- => Added simpler.patch, #52259
    2021-11-11 townba Attached File- => Added patch1b.patch, #52241
    2021-11-09 bens StatusNeed Info => In Progress
    2021-11-03 bens Attached File- => Added colors.py, #52193
        StatusNone => Need Info
        Assigned toNone => bens
    2021-11-02 townba Attached File- => Added 0001-Improve-color-support.patch, #52181

    Back to the top


    Powered by Savane 3.9