| Message ID | CANsz78LEZiocv_E-Hvj3iBahFFgomPb3BFNdmas2iqxqRLfRiw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for axiterm colors in parse_color | expand |
On Tue, Jan 07, 2020 at 10:36:53AM -0500, Eyal Soha wrote: > https://en.wikipedia.org/wiki/ANSI_escape_code > > ANSI color codes 90-97 and 100-107 are brighter versions of the 30-37 > and 40-47 colors. To view them, run `colortest-16`. In that > colortest, they are named with a leading "+". Maybe git config could > support those colors, too, with a leading plus in the name? They are > nice for having a different shade of a color in a diff. Depending on your terminal, you can already access these colors via 256-color mode. Generally 0-7 are the standard colors there, then 8-15 are the "bright" variants, and then an rgb cube. You can see how your terminal displays all of them with something like this: reset=$(git config --type=color --default=reset foo.bar) for i in $(seq 0 255); do color=$(git config --type=color --default="$i" foo.bar) echo "$color$i$reset" done Some terminals also show "bold red" as bright red, but many modern ones seem to actually provide a bold font. That said, I'm not entirely opposed to having more human-readable aliases. I'm not sure if it's worth using the 16-color version (e.g., "ESC[91m" versus the 256-color version "ESC[38;5;9m"). It's possible it would be compatible with more terminals, and it is slightly fewer bytes. > diff --git a/color.c b/color.c > index ebb222ec33..a39fe7d133 100644 > --- a/color.c > +++ b/color.c > @@ -33,6 +33,7 @@ struct color { > COLOR_UNSPECIFIED = 0, > COLOR_NORMAL, > COLOR_ANSI, /* basic 0-7 ANSI colors */ > + COLOR_AXITERM, /* brighter than 0-7 ANSI colors */ What's AXITERM? I couldn't find it mentioned anywhere around ANSI codes. I kind of wonder if we could just call these ANSI as well, and have color_output() just decide whether to add an offset of 60 when we see a color number between 8 and 15. Or possibly c->value should just have the 60-offset value in the first place. > * already have the ANSI escape code in it. "out" should have enough > * space in it to fit any color. > */ > -static char *color_output(char *out, int len, const struct color *c, char type) > +static char *color_output(char *out, int len, const struct color *c, > + const char *type) This "type" field was already pretty ugly, and I think your patch makes it even worse. It would probably be less bad if we just passed in the integer 30 instead of '3', and then do: /* handles ANSI; your bright colors would want to add 60 */ out += xsnprintf(out, len, "%d", type + c->value); And then likewise the 256-color and rgb paths would do: out += xsnprintf(out, len, "%d;5;%d", type + 8, c->value); Bonus points for declaring: enum { COLOR_FOREGROUND = 30, COLOR_BACKGROUND = 40, } color_ground; to make the caller more readable. > case COLOR_ANSI: > - if (len < 2) > + case COLOR_AXITERM: > + if (len < strlen(type) + 1) > BUG("color parsing ran out of space"); > - *out++ = type; > - *out++ = '0' + c->value; > + out += xsnprintf(out, len, "%s%c", type, '0' + c->value); This BUG() can go away. The point was to make sure we don't overflow "out", but xsnprintf() will check that for us (that's why there isn't one in the COLOR_256 and COLOR_RGB case arms). > @@ -279,14 +287,24 @@ int color_parse_mem(const char *value, int > value_len, char *dst) > if (!color_empty(&fg)) { > if (sep++) > OUT(';'); > - /* foreground colors are all in the 3x range */ > - dst = color_output(dst, end - dst, &fg, '3'); > + /* foreground colors are in the 3x range */ > + char *range = "3"; > + if (fg.type == COLOR_AXITERM) { > + /* axiterm fg colors are in the 9x range */ > + range = "9"; > + } > + dst = color_output(dst, end - dst, &fg, range); And if we can handle the regular/bright stuff instead of color_output(), we don't need to have this extra fg.type check. -Peff
> Depending on your terminal, you can already access these colors via > 256-color mode. Generally 0-7 are the standard colors there, then 8-15 > are the "bright" variants, and then an rgb cube. Yes. According to the wikipedia page https://en.wikipedia.org/wiki/ANSI_escape_code#Colors : 0- 7: standard colors (as in ESC [ 30–37 m) 8- 15: high intensity colors (as in ESC [ 90–97 m) 16-231: 6 × 6 × 6 cube (216 colors): 16 + 36 × r + 6 × g + b (0 ≤ r, g, b ≤ 5) 232-255: grayscale from black to white in 24 steps So 8bit 0-7 and 8bit 8-15 match the ANSI colors 30-37 and 90-97. The background color versions match 40-47 and 100-107. On my desktop, the RGB and ANSI colors match in color as described above. > That said, I'm not entirely opposed to having more human-readable > aliases. I'm not sure if it's worth using the 16-color version (e.g., > "ESC[91m" versus the 256-color version "ESC[38;5;9m"). It's possible it > would be compatible with more terminals, and it is slightly fewer bytes. My motivation for this patch was to fix the github workflow log output that doesn't support 8bit colors properly. Only the "ANSI" colors 0-15 worked. None of the 8-bit colors worked except for 30-37, 40-47, 90-97, and 100-107, which matched the ANSI colors. That is a very broken color scheme! But that's how it displayed. > What's AXITERM? I couldn't find it mentioned anywhere around ANSI codes. Oops, I misread it. The wikipedia page mentions it, it's "aixterm". > I kind of wonder if we could just call these ANSI as well, and have > color_output() just decide whether to add an offset of 60 when we see a > color number between 8 and 15. Or possibly c->value should just have the > 60-offset value in the first place. Done. > This "type" field was already pretty ugly, and I think your patch makes > it even worse. It would probably be less bad if we just passed in the > integer 30 instead of '3', and then do: Done. > Bonus points for declaring: > > enum { > COLOR_FOREGROUND = 30, > COLOR_BACKGROUND = 40, > } color_ground; Done in a slightly different way. > This BUG() can go away. The point was to make sure we don't overflow > "out", but xsnprintf() will check that for us (that's why there isn't > one in the COLOR_256 and COLOR_RGB case arms). Removed. > And if we can handle the regular/bright stuff instead of color_output(), > we don't need to have this extra fg.type check. Done. Here's a new patch! diff --git a/color.c b/color.c index ebb222ec33..efbe9a1858 100644 --- a/color.c +++ b/color.c @@ -24,6 +24,14 @@ const char *column_colors_ansi[] = { GIT_COLOR_RESET, }; +enum { + COLOR_BACKGROUND_OFFSET = 10, + 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 */ const int column_colors_ansi_max = ARRAY_SIZE(column_colors_ansi) - 1; @@ -92,7 +100,13 @@ static int parse_color(struct color *out, const char *name, int len) for (i = 0; i < ARRAY_SIZE(color_names); i++) { if (match_word(name, len, color_names[i])) { out->type = COLOR_ANSI; - out->value = i; + out->value = i + COLOR_FOREGROUND_ANSI; + return 0; + } + if (*name == '+' && + match_word(name+1, len-1, color_names[i])) { + out->type = COLOR_ANSI; + out->value = i + COLOR_FOREGROUND_BRIGHT_ANSI; return 0; } } @@ -109,10 +123,15 @@ static int parse_color(struct color *out, const char *name, int len) else if (val < 0) { out->type = COLOR_NORMAL; return 0; - /* Rewrite low numbers as more-portable standard colors. */ + /* Rewrite 0-7 as more-portable standard colors. */ } else if (val < 8) { out->type = COLOR_ANSI; - out->value = val; + out->value = val + COLOR_FOREGROUND_ANSI; + return 0; + /* Rewrite 8-15 as more-portable aixterm colors. */ + } else if (val < 16) { + out->type = COLOR_ANSI; + out->value = val - 8 + COLOR_FOREGROUND_BRIGHT_ANSI; return 0; } else if (val < 256) { out->type = COLOR_256; @@ -166,24 +185,24 @@ int color_parse(const char *value, char *dst) * already have the ANSI escape code in it. "out" should have enough * space in it to fit any color. */ -static char *color_output(char *out, int len, const struct color *c, char type) +static char *color_output(char *out, int len, const struct color *c, + int offset) { switch (c->type) { case COLOR_UNSPECIFIED: case COLOR_NORMAL: break; case COLOR_ANSI: - if (len < 2) - BUG("color parsing ran out of space"); - *out++ = type; - *out++ = '0' + c->value; + out += xsnprintf(out, len, "%d", c->value + offset); break; case COLOR_256: - out += xsnprintf(out, len, "%c8;5;%d", type, c->value); + out += xsnprintf(out, len, "%d;5;%d", COLOR_FOREGROUND_256 + offset, + c->value); break; case COLOR_RGB: - out += xsnprintf(out, len, "%c8;2;%d;%d;%d", type, - c->red, c->green, c->blue); + out += xsnprintf(out, len, "%d;2;%d;%d;%d", + COLOR_FOREGROUND_RGB + offset, + c->red, c->green, c->blue); break; } return out; @@ -279,14 +298,12 @@ int color_parse_mem(const char *value, int value_len, char *dst) if (!color_empty(&fg)) { if (sep++) OUT(';'); - /* foreground colors are all in the 3x range */ - dst = color_output(dst, end - dst, &fg, '3'); + dst = color_output(dst, end - dst, &fg, 0); } if (!color_empty(&bg)) { if (sep++) OUT(';'); - /* background colors are all in the 4x range */ - dst = color_output(dst, end - dst, &bg, '4'); + dst = color_output(dst, end - dst, &bg, COLOR_BACKGROUND_OFFSET); } OUT('m'); } diff --git a/t/t4026-color.sh b/t/t4026-color.sh index 671e951ee5..e3f11a94f9 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 'axiterm bright fg color' ' + color "+red" "[91m" +' + +test_expect_success 'axiterm bright bg color' ' + color "green +blue" "[32;104m" +' + test_expect_success 'color name before attribute' ' color "red bold" "[1;31m" ' @@ -74,6 +82,10 @@ test_expect_success '0-7 are aliases for basic ANSI color names' ' color "0 7" "[30;47m" ' +test_expect_success '8-15 are aliases for aixterm color names' ' + color "12 13" "[94;105m" +' + test_expect_success '256 colors' ' color "254 bold 255" "[1;38;5;254;48;5;255m" '
On Thu, Jan 09, 2020 at 07:20:09PM -0500, Eyal Soha wrote: > > That said, I'm not entirely opposed to having more human-readable > > aliases. I'm not sure if it's worth using the 16-color version (e.g., > > "ESC[91m" versus the 256-color version "ESC[38;5;9m"). It's possible it > > would be compatible with more terminals, and it is slightly fewer bytes. > > My motivation for this patch was to fix the github workflow log output > that doesn't support 8bit colors properly. Only the "ANSI" colors > 0-15 worked. None of the 8-bit colors worked except for 30-37, 40-47, > 90-97, and 100-107, which matched the ANSI colors. That is a very > broken color scheme! But that's how it displayed. That makes sense. I'm not too surprised to see a terminal that supports one but not the other. But I wonder if there are ones that go the other way around: supporting 256-color mode but not ANSI 90-97, etc. I doubt it, but I think it would be nice to split that change out into a separate commit in case we do run into problems. > Done. Here's a new patch! Thanks. The content here is looking pretty good (though I have a few comments below). Can you also format it as described in Documentation/SubmittingPatches and re-send? In particular, it needs a regular commit message and your signoff. > +enum { > + COLOR_BACKGROUND_OFFSET = 10, > + COLOR_FOREGROUND_ANSI = 30, > + COLOR_FOREGROUND_RGB = 38, > + COLOR_FOREGROUND_256 = 38, > + COLOR_FOREGROUND_BRIGHT_ANSI = 90, > +}; The split in this enum mostly makes sense to me. It changes the meaning of "value" for COLOR_ANSI, but this is all local to color.c, so your changes to output_color() are all that's needed. It might be easier to follow the patch if switching to this enum came in a separate preparatory patch. > @@ -92,7 +100,13 @@ static int parse_color(struct color *out, const > char *name, int len) > for (i = 0; i < ARRAY_SIZE(color_names); i++) { > if (match_word(name, len, color_names[i])) { > out->type = COLOR_ANSI; > - out->value = i; > + out->value = i + COLOR_FOREGROUND_ANSI; > + return 0; > + } > + if (*name == '+' && > + match_word(name+1, len-1, color_names[i])) { > + out->type = COLOR_ANSI; > + out->value = i + COLOR_FOREGROUND_BRIGHT_ANSI; > return 0; > } It would be slightly simpler and more efficient handle the "+" outside the loop, like: int offset = COLOR_FOREGROUND_ANSI; if (*name == '+') { offset = COLOR_FOREGROUND_BRIGHT_ANSI; name++; len--; } and then in the loop just set "out->value = i + offset". You'd probably want to hoist this out to a separate function since "name" needs to be unchanged in the later part of the function. I dunno. It's almost certainly an unmeasurable micro-optimization, but somehow the flow of it seems simpler to me. I also wondered if we'd want English aliases like "brightred" that some other systems use. It would be easy to add that to the check I showed above without having to repeat as much. > @@ -109,10 +123,15 @@ static int parse_color(struct color *out, const > char *name, int len) > else if (val < 0) { > out->type = COLOR_NORMAL; > return 0; > - /* Rewrite low numbers as more-portable standard colors. */ > + /* Rewrite 0-7 as more-portable standard colors. */ > } else if (val < 8) { > out->type = COLOR_ANSI; > - out->value = val; > + out->value = val + COLOR_FOREGROUND_ANSI; > + return 0; > + /* Rewrite 8-15 as more-portable aixterm colors. */ > + } else if (val < 16) { > + out->type = COLOR_ANSI; > + out->value = val - 8 + COLOR_FOREGROUND_BRIGHT_ANSI; And I think this 8-15 handling might want to be a separate commit on top, just because it's possible it might regress some cases (though again, I do doubt it). > case COLOR_256: > - out += xsnprintf(out, len, "%c8;5;%d", type, c->value); > + out += xsnprintf(out, len, "%d;5;%d", COLOR_FOREGROUND_256 + offset, > + c->value); Looks like some unwanted tab/space conversion (here and elsewhere). > +test_expect_success 'axiterm bright fg color' ' > + color "+red" "[91m" s/axi/aix/ (here and below). > +test_expect_success '8-15 are aliases for aixterm color names' ' > + color "12 13" "[94;105m" Makes sense. -Peff
Thanks. I think that 3 patches are in order. The first will refactor and add the enums. The second will add the aixterm colors. The last will alias RGB 8-15 to aixterm colors. What's the correct way to email those patches? I will try with git-send-email. BTW, is "git help send-email" supposed to work? Eyal On Fri, Jan 10, 2020 at 6:15 AM Jeff King <peff@peff.net> wrote: > > On Thu, Jan 09, 2020 at 07:20:09PM -0500, Eyal Soha wrote: > > > > That said, I'm not entirely opposed to having more human-readable > > > aliases. I'm not sure if it's worth using the 16-color version (e.g., > > > "ESC[91m" versus the 256-color version "ESC[38;5;9m"). It's possible it > > > would be compatible with more terminals, and it is slightly fewer bytes. > > > > My motivation for this patch was to fix the github workflow log output > > that doesn't support 8bit colors properly. Only the "ANSI" colors > > 0-15 worked. None of the 8-bit colors worked except for 30-37, 40-47, > > 90-97, and 100-107, which matched the ANSI colors. That is a very > > broken color scheme! But that's how it displayed. > > That makes sense. I'm not too surprised to see a terminal that supports > one but not the other. > > But I wonder if there are ones that go the other way around: supporting > 256-color mode but not ANSI 90-97, etc. I doubt it, but I think it would > be nice to split that change out into a separate commit in case we do > run into problems. > > > Done. Here's a new patch! > > Thanks. The content here is looking pretty good (though I have a few > comments below). > > Can you also format it as described in Documentation/SubmittingPatches > and re-send? In particular, it needs a regular commit message and your > signoff. > > > +enum { > > + COLOR_BACKGROUND_OFFSET = 10, > > + COLOR_FOREGROUND_ANSI = 30, > > + COLOR_FOREGROUND_RGB = 38, > > + COLOR_FOREGROUND_256 = 38, > > + COLOR_FOREGROUND_BRIGHT_ANSI = 90, > > +}; > > The split in this enum mostly makes sense to me. It changes the meaning > of "value" for COLOR_ANSI, but this is all local to color.c, so your > changes to output_color() are all that's needed. > > It might be easier to follow the patch if switching to this enum came in > a separate preparatory patch. > > > @@ -92,7 +100,13 @@ static int parse_color(struct color *out, const > > char *name, int len) > > for (i = 0; i < ARRAY_SIZE(color_names); i++) { > > if (match_word(name, len, color_names[i])) { > > out->type = COLOR_ANSI; > > - out->value = i; > > + out->value = i + COLOR_FOREGROUND_ANSI; > > + return 0; > > + } > > + if (*name == '+' && > > + match_word(name+1, len-1, color_names[i])) { > > + out->type = COLOR_ANSI; > > + out->value = i + COLOR_FOREGROUND_BRIGHT_ANSI; > > return 0; > > } > > It would be slightly simpler and more efficient handle the "+" outside > the loop, like: > > int offset = COLOR_FOREGROUND_ANSI; > if (*name == '+') { > offset = COLOR_FOREGROUND_BRIGHT_ANSI; > name++; > len--; > } > > and then in the loop just set "out->value = i + offset". > > You'd probably want to hoist this out to a separate function since > "name" needs to be unchanged in the later part of the function. > > I dunno. It's almost certainly an unmeasurable micro-optimization, but > somehow the flow of it seems simpler to me. > > I also wondered if we'd want English aliases like "brightred" that some > other systems use. It would be easy to add that to the check I showed > above without having to repeat as much. > > > @@ -109,10 +123,15 @@ static int parse_color(struct color *out, const > > char *name, int len) > > else if (val < 0) { > > out->type = COLOR_NORMAL; > > return 0; > > - /* Rewrite low numbers as more-portable standard colors. */ > > + /* Rewrite 0-7 as more-portable standard colors. */ > > } else if (val < 8) { > > out->type = COLOR_ANSI; > > - out->value = val; > > + out->value = val + COLOR_FOREGROUND_ANSI; > > + return 0; > > + /* Rewrite 8-15 as more-portable aixterm colors. */ > > + } else if (val < 16) { > > + out->type = COLOR_ANSI; > > + out->value = val - 8 + COLOR_FOREGROUND_BRIGHT_ANSI; > > And I think this 8-15 handling might want to be a separate commit on > top, just because it's possible it might regress some cases (though > again, I do doubt it). > > > case COLOR_256: > > - out += xsnprintf(out, len, "%c8;5;%d", type, c->value); > > + out += xsnprintf(out, len, "%d;5;%d", COLOR_FOREGROUND_256 + offset, > > + c->value); > > Looks like some unwanted tab/space conversion (here and elsewhere). > > > +test_expect_success 'axiterm bright fg color' ' > > + color "+red" "[91m" > > s/axi/aix/ (here and below). > > > +test_expect_success '8-15 are aliases for aixterm color names' ' > > + color "12 13" "[94;105m" > > Makes sense. > > -Peff
I sent out my patches. Can someone take a look? Thanks, Eyal On Fri, Jan 10, 2020 at 10:02 AM Eyal Soha <shawarmakarma@gmail.com> wrote: > > Thanks. I think that 3 patches are in order. The first will refactor > and add the enums. The second will add the aixterm colors. The last > will alias RGB 8-15 to aixterm colors. > > What's the correct way to email those patches? I will try with > git-send-email. BTW, is "git help send-email" supposed to work? > > Eyal > On Fri, Jan 10, 2020 at 6:15 AM Jeff King <peff@peff.net> wrote: > > > > On Thu, Jan 09, 2020 at 07:20:09PM -0500, Eyal Soha wrote: > > > > > > That said, I'm not entirely opposed to having more human-readable > > > > aliases. I'm not sure if it's worth using the 16-color version (e.g., > > > > "ESC[91m" versus the 256-color version "ESC[38;5;9m"). It's possible it > > > > would be compatible with more terminals, and it is slightly fewer bytes. > > > > > > My motivation for this patch was to fix the github workflow log output > > > that doesn't support 8bit colors properly. Only the "ANSI" colors > > > 0-15 worked. None of the 8-bit colors worked except for 30-37, 40-47, > > > 90-97, and 100-107, which matched the ANSI colors. That is a very > > > broken color scheme! But that's how it displayed. > > > > That makes sense. I'm not too surprised to see a terminal that supports > > one but not the other. > > > > But I wonder if there are ones that go the other way around: supporting > > 256-color mode but not ANSI 90-97, etc. I doubt it, but I think it would > > be nice to split that change out into a separate commit in case we do > > run into problems. > > > > > Done. Here's a new patch! > > > > Thanks. The content here is looking pretty good (though I have a few > > comments below). > > > > Can you also format it as described in Documentation/SubmittingPatches > > and re-send? In particular, it needs a regular commit message and your > > signoff. > > > > > +enum { > > > + COLOR_BACKGROUND_OFFSET = 10, > > > + COLOR_FOREGROUND_ANSI = 30, > > > + COLOR_FOREGROUND_RGB = 38, > > > + COLOR_FOREGROUND_256 = 38, > > > + COLOR_FOREGROUND_BRIGHT_ANSI = 90, > > > +}; > > > > The split in this enum mostly makes sense to me. It changes the meaning > > of "value" for COLOR_ANSI, but this is all local to color.c, so your > > changes to output_color() are all that's needed. > > > > It might be easier to follow the patch if switching to this enum came in > > a separate preparatory patch. > > > > > @@ -92,7 +100,13 @@ static int parse_color(struct color *out, const > > > char *name, int len) > > > for (i = 0; i < ARRAY_SIZE(color_names); i++) { > > > if (match_word(name, len, color_names[i])) { > > > out->type = COLOR_ANSI; > > > - out->value = i; > > > + out->value = i + COLOR_FOREGROUND_ANSI; > > > + return 0; > > > + } > > > + if (*name == '+' && > > > + match_word(name+1, len-1, color_names[i])) { > > > + out->type = COLOR_ANSI; > > > + out->value = i + COLOR_FOREGROUND_BRIGHT_ANSI; > > > return 0; > > > } > > > > It would be slightly simpler and more efficient handle the "+" outside > > the loop, like: > > > > int offset = COLOR_FOREGROUND_ANSI; > > if (*name == '+') { > > offset = COLOR_FOREGROUND_BRIGHT_ANSI; > > name++; > > len--; > > } > > > > and then in the loop just set "out->value = i + offset". > > > > You'd probably want to hoist this out to a separate function since > > "name" needs to be unchanged in the later part of the function. > > > > I dunno. It's almost certainly an unmeasurable micro-optimization, but > > somehow the flow of it seems simpler to me. > > > > I also wondered if we'd want English aliases like "brightred" that some > > other systems use. It would be easy to add that to the check I showed > > above without having to repeat as much. > > > > > @@ -109,10 +123,15 @@ static int parse_color(struct color *out, const > > > char *name, int len) > > > else if (val < 0) { > > > out->type = COLOR_NORMAL; > > > return 0; > > > - /* Rewrite low numbers as more-portable standard colors. */ > > > + /* Rewrite 0-7 as more-portable standard colors. */ > > > } else if (val < 8) { > > > out->type = COLOR_ANSI; > > > - out->value = val; > > > + out->value = val + COLOR_FOREGROUND_ANSI; > > > + return 0; > > > + /* Rewrite 8-15 as more-portable aixterm colors. */ > > > + } else if (val < 16) { > > > + out->type = COLOR_ANSI; > > > + out->value = val - 8 + COLOR_FOREGROUND_BRIGHT_ANSI; > > > > And I think this 8-15 handling might want to be a separate commit on > > top, just because it's possible it might regress some cases (though > > again, I do doubt it). > > > > > case COLOR_256: > > > - out += xsnprintf(out, len, "%c8;5;%d", type, c->value); > > > + out += xsnprintf(out, len, "%d;5;%d", COLOR_FOREGROUND_256 + offset, > > > + c->value); > > > > Looks like some unwanted tab/space conversion (here and elsewhere). > > > > > +test_expect_success 'axiterm bright fg color' ' > > > + color "+red" "[91m" > > > > s/axi/aix/ (here and below). > > > > > +test_expect_success '8-15 are aliases for aixterm color names' ' > > > + color "12 13" "[94;105m" > > > > Makes sense. > > > > -Peff
diff --git a/color.c b/color.c index ebb222ec33..a39fe7d133 100644 --- a/color.c +++ b/color.c @@ -33,6 +33,7 @@ struct color { COLOR_UNSPECIFIED = 0, COLOR_NORMAL, COLOR_ANSI, /* basic 0-7 ANSI colors */ + COLOR_AXITERM, /* brighter than 0-7 ANSI colors */ COLOR_256, COLOR_RGB } type; @@ -95,6 +96,12 @@ static int parse_color(struct color *out, const char *name, int len) out->value = i; return 0; } + if (*name == '+' && + match_word(name+1, len-1, color_names[i])) { + out->type = COLOR_AXITERM; + out->value = i; + return 0; + } } /* And finally try a literal 256-color-mode number */ @@ -166,23 +173,24 @@ int color_parse(const char *value, char *dst) * already have the ANSI escape code in it. "out" should have enough * space in it to fit any color. */ -static char *color_output(char *out, int len, const struct color *c, char type) +static char *color_output(char *out, int len, const struct color *c, + const char *type) { switch (c->type) { case COLOR_UNSPECIFIED: case COLOR_NORMAL: break; case COLOR_ANSI: - if (len < 2) + case COLOR_AXITERM: + if (len < strlen(type) + 1) BUG("color parsing ran out of space"); - *out++ = type; - *out++ = '0' + c->value; + out += xsnprintf(out, len, "%s%c", type, '0' + c->value); break; case COLOR_256: - out += xsnprintf(out, len, "%c8;5;%d", type, c->value); + out += xsnprintf(out, len, "%s8;5;%d", type, c->value); break; case COLOR_RGB: - out += xsnprintf(out, len, "%c8;2;%d;%d;%d", type, + out += xsnprintf(out, len, "%s8;2;%d;%d;%d", type, c->red, c->green, c->blue); break; } @@ -279,14 +287,24 @@ int color_parse_mem(const char *value, int value_len, char *dst) if (!color_empty(&fg)) { if (sep++) OUT(';'); - /* foreground colors are all in the 3x range */ - dst = color_output(dst, end - dst, &fg, '3'); + /* foreground colors are in the 3x range */ + char *range = "3"; + if (fg.type == COLOR_AXITERM) { + /* axiterm fg colors are in the 9x range */ + range = "9"; + } + dst = color_output(dst, end - dst, &fg, range); } if (!color_empty(&bg)) { if (sep++) OUT(';'); /* background colors are all in the 4x range */ - dst = color_output(dst, end - dst, &bg, '4'); + char *range = "4"; + if (bg.type == COLOR_AXITERM) { + /* axiterm bg colors are in the 10x range */ + range = "10"; + } + dst = color_output(dst, end - dst, &bg, range); } OUT('m'); } diff --git a/t/t4026-color.sh b/t/t4026-color.sh index 671e951ee5..2b8430584f 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 'axiterm bright fg color' ' + color "+red" "[91m" +' + +test_expect_success 'axiterm bright bg color' ' + color "green +blue" "[32;104m" +' + test_expect_success 'color name before attribute' ' color "red bold" "[1;31m" '