| Message ID | 20200118145318.5177-2-shawarmakarma@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] color.c: Refactor color_output to use enums | expand |
Eyal Soha <shawarmakarma@gmail.com> writes: > These colors are the bright variants of the 3-bit colors. OK, so this round the design is to reuse the ANSI mode instead of introducing a new AIX mode that sits next to ANSI, 256 and RGB? For this to work, not just the 90-97 range for bright-ansi orders the colors the same way as 30-37 range (only brighter), but also the differences between corresponding fore- and background colors must also be 10 just like the regular ANSI colors. So perhaps an additional sentence or two deserve to be there, e.g. ... of the 3-bit colors. Instead of 30-37 range for the foreground and 40-47 range for the background, they live in 90-97 and 100-107 range, respectively. or something like that, perhaps? > The basic colors accepted are `normal`, `black`, `red`, `green`, `yellow`, > `blue`, `magenta`, `cyan` and `white`. The first color given is the > -foreground; the second is the background. > +foreground; the second is the background. All the basic colors except > +`normal` have a bright variant that can be speficied by prefixing the > +color with `bright`, like `brightred`. Nicely and readably written. I have to wonder if spelling "bright<color>", i.e. two words smashed together without anything in between words, is in widespread use (in other words, are we following an established practice, or are we inventing our own), or if we need to prepare for synonyms? HTML/CSS folks seem to use words-smashed-without-anything-in-betwen so they should be fine with this design; I no longer recall what X did ;-) Looking good. Thanks.
On Sat, Jan 18, 2020 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote: > > OK, so this round the design is to reuse the ANSI mode instead of > introducing a new AIX mode that sits next to ANSI, 256 and RGB? Right. Previously I had it with a new AIX enum parallel to ANSI, 256, etc, but it just made the code longer for no good reason. > For this to work, not just the 90-97 range for bright-ansi orders > the colors the same way as 30-37 range (only brighter), but also > the differences between corresponding fore- and background colors > must also be 10 just like the regular ANSI colors. Yes. It's a happy coincidence that the background version is always 10 greater than the foreground version, for ANSI, for AIX, and even for the 256-bit colors. The code takes advantage of that. If that later proves to be not true, color_output needs to be modified. However!, the modification would be just in color_output because the input is now a boolean "background" instead of the previous char "type". I think that's a good improvement so that the caller of color_output doesn't need to know that, ie, '3' is foreground and '4' is background. > > So perhaps an additional sentence or two deserve to be there, e.g. > > ... of the 3-bit colors. Instead of 30-37 range for the > foreground and 40-47 range for the background, they live in > 90-97 and 100-107 range, respectively. Will do. > > or something like that, perhaps? > > > The basic colors accepted are `normal`, `black`, `red`, `green`, `yellow`, > > `blue`, `magenta`, `cyan` and `white`. The first color given is the > > -foreground; the second is the background. > > +foreground; the second is the background. All the basic colors except > > +`normal` have a bright variant that can be speficied by prefixing the > > +color with `bright`, like `brightred`. > > Nicely and readably written. Thanks. I tried to keep the voice in line with the rest of the text. > > I have to wonder if spelling "bright<color>", i.e. two words smashed > together without anything in between words, is in widespread use (in > other words, are we following an established practice, or are we > inventing our own), or if we need to prepare for synonyms? HTML/CSS > folks seem to use words-smashed-without-anything-in-betwen so they > should be fine with this design; I no longer recall what X did ;-) /usr/local/lib/X11/rgb.txt often uses smashed together: https://github.com/vim/vim/blob/master/runtime/rgb.txt Wikipedia calls them "bright" consistently: https://en.wikipedia.org/wiki/ANSI_escape_code#Colors . So we've got a vote for smashing them together and a vote for "bright". Seems okay by me! Eyal
Eyal Soha <shawarmakarma@gmail.com> writes: >> I have to wonder if spelling "bright<color>", i.e. two words smashed >> together without anything in between words, is in widespread use (in >> other words, are we following an established practice, or are we >> inventing our own), or if we need to prepare for synonyms? HTML/CSS >> folks seem to use words-smashed-without-anything-in-betwen so they >> should be fine with this design; I no longer recall what X did ;-) > > /usr/local/lib/X11/rgb.txt often uses smashed together: > https://github.com/vim/vim/blob/master/runtime/rgb.txt Wikipedia > calls them "bright" consistently: > https://en.wikipedia.org/wiki/ANSI_escape_code#Colors . So we've got > a vote for smashing them together and a vote for "bright". Seems okay > by me! OK. By synonym, I did not mean any word other than "bright"; in the context of my response, "brightred", "bright red" and "bright-red" would have been synonyms, but I think it is sufficient to support only the first one. Of course, somebody may come up with a bright idea to treat the "bright" adjective just like "underline" and "bold" and that might improve the end user experience slightly better, but I dunno ;-)
diff --git a/Documentation/config.txt b/Documentation/config.txt index 83e7bba872..08b13ba72b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -263,7 +263,9 @@ color:: + The basic colors accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and `white`. The first color given is the -foreground; the second is the background. +foreground; the second is the background. All the basic colors except +`normal` have a bright variant that can be speficied by prefixing the +color with `bright`, like `brightred`. + Colors may also be given as numbers between 0 and 255; these use ANSI 256-color mode (but note that not all terminals may support this). If diff --git a/color.c b/color.c index 3b734ccffd..66d32e1191 100644 --- a/color.c +++ b/color.c @@ -29,6 +29,7 @@ enum { COLOR_FOREGROUND_ANSI = 30, COLOR_FOREGROUND_RGB = 38, COLOR_FOREGROUND_256 = 38, + COLOR_FOREGROUND_BRIGHT_ANSI = 90, }; /* Ignore the RESET at the end when giving the size */ @@ -68,13 +69,36 @@ static int get_hex_color(const char *in, unsigned char *out) return 0; } -static int parse_color(struct color *out, const char *name, int len) +/* + * If an ANSI color is recognized in "name", fill "out" and return 0. + * Otherwise, leave out unchanged and return -1. + */ +static int parse_ansi_color(struct color *out, const char *name, int len) { /* Positions in array must match ANSI color codes */ static const char * const color_names[] = { "black", "red", "green", "yellow", "blue", "magenta", "cyan", "white" }; + + int color_offset = COLOR_FOREGROUND_ANSI; + if (strncasecmp(name, "bright", 6) == 0) { + color_offset = COLOR_FOREGROUND_BRIGHT_ANSI; + name += 6; + len -= 6; + } + for (int i = 0; i < ARRAY_SIZE(color_names); i++) { + if (match_word(name, len, color_names[i])) { + out->type = COLOR_ANSI; + out->value = i + color_offset; + return 0; + } + } + return -1; +} + +static int parse_color(struct color *out, const char *name, int len) +{ char *end; int i; long val; @@ -96,12 +120,8 @@ static int parse_color(struct color *out, const char *name, int len) } /* Then pick from our human-readable color names... */ - for (i = 0; i < ARRAY_SIZE(color_names); i++) { - if (match_word(name, len, color_names[i])) { - out->type = COLOR_ANSI; - out->value = i + COLOR_FOREGROUND_ANSI; - return 0; - } + if (parse_ansi_color(out, name, len) == 0) { + return 0; } /* And finally try a literal 256-color-mode number */ diff --git a/t/t4026-color.sh b/t/t4026-color.sh index 671e951ee5..78c69de90a 100755 --- a/t/t4026-color.sh +++ b/t/t4026-color.sh @@ -30,6 +30,14 @@ test_expect_success 'attribute before color name' ' color "bold red" "[1;31m" ' +test_expect_success 'aixterm bright fg color' ' + color "brightred" "[91m" +' + +test_expect_success 'aixterm bright bg color' ' + color "green brightblue" "[32;104m" +' + test_expect_success 'color name before attribute' ' color "red bold" "[1;31m" '
These colors are the bright variants of the 3-bit colors. Signed-off-by: Eyal Soha <shawarmakarma@gmail.com> --- Documentation/config.txt | 4 +++- color.c | 34 +++++++++++++++++++++++++++------- t/t4026-color.sh | 8 ++++++++ 3 files changed, 38 insertions(+), 8 deletions(-)