Mark Lodato <lodatom@xxxxxxxxx> writes:
> On Sat, Feb 27, 2010 at 3:51 AM, Jeff King <peff@xxxxxxxx> wrote:
>> I am not against this patch if it gets us some flexibility that is not
>> otherwise easy to attain,
>
> Besides disallowing multiple attributes (e.g. bold blink), the current
> parser does not have a way to specify colors for 16-color mode colors
> 8-15, 256-color mode colors 0-7, or any 88-color mode colors. There
> are also other esoteric attributes [1] that some user might want to
> use, such as italic or franktur. I don't know if anyone will ever use
> this feature, but it wasn't hard to implement.
The purist side of me has been hoping that we could later wean off this
ANSI centric view of the terminal attribute handling and move us to
something based on terminfo. This patch makes it even harder by going
quite the opposite way.
But the pragmatic side of me has long held a feeling that nobody who would
use git uses real terminals these days anymore, and there is no terminal
emulator that does not grok ANSI sequences. msysgit folks have even done
their own ANSI color emulation in their "Console" interface layer, so that
may be another reason that we are practically married to ANSI sequence and
there is not much gained by introducing terminfo as another layer of
abstraction to build GIT_COLOR_* on top of.
What I am saying is that the purist in me actively hates [PATCH 1/5], but
the pragmatist in me admits it would not hurt in practice.
>> but wouldn't it be more user friendly for us
>> to support "red blink bold ul italic"?
>
> Yes, I think this should be done whether or not the patch in question
> is accepted.
Hmm, I do not care much about italic, blink nor ul, but perhaps other
people do. Combining attributes like "reverse bold" would probably make
sense.
color.c | 35 +++++++++++++++++++++++++++--------
1 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/color.c b/color.c
index 62977f4..f210d94 100644
--- a/color.c
+++ b/color.c
@@ -47,7 +47,8 @@ void color_parse_mem(const char *value, int value_len, const char *var,
{
const char *ptr = value;
int len = value_len;
- int attr = -1;
+ int attr[20];
+ int attr_idx = 0;
int fg = -2;
int bg = -2;
@@ -56,7 +57,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
return;
}
- /* [fg [bg]] [attr] */
+ /* [fg [bg]] [attr]... */
while (len > 0) {
const char *word = ptr;
int val, wordlen = 0;
@@ -85,19 +86,37 @@ void color_parse_mem(const char *value, int value_len, const char *var,
goto bad;
}
val = parse_attr(word, wordlen);
- if (val < 0 || attr != -1)
+ if (0 <= val && attr_idx < ARRAY_SIZE(attr))
+ attr[attr_idx++] = val;
+ else
goto bad;
- attr = val;
}
- if (attr >= 0 || fg >= 0 || bg >= 0) {
+ if (attr_idx > 0 || fg >= 0 || bg >= 0) {
int sep = 0;
+ int i;
+
+ if (COLOR_MAXLEN <=
+ /* Number of bytes to denote colors and attributes */
+ (attr_idx
+ + (fg < 0 ? 0 :
+ ((fg < 8) ? 2 : 8)) /* "3x" or "38;5;xxx" */
+ + (bg < 0 ? 0 :
+ ((bg < 8) ? 2 : 8)) /* "4x" or "48;5;xxx" */
+ ) +
+ /* Number of semicolons between the above */
+ (attr_idx + (0 <= fg) + (0 <= bg) - 1) +
+ /* ESC '[', terminating 'm' and NUL */
+ 4)
+ goto bad;
*dst++ = '\033';
*dst++ = '[';
- if (attr >= 0) {
- *dst++ = '0' + attr;
- sep++;
+
+ for (i = 0; i < attr_idx; i++) {
+ if (sep++)
+ *dst++ = ';';
+ *dst++ = '0' + attr[i];
}
if (fg >= 0) {
if (sep++)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html