{"thread":{"id":"22841","subject":"[PATCH 1/5] Allow explicit ANSI codes for colors","startedAt":"2010-02-27T04:57:45Z","lastAt":"2010-03-03T04:49:27Z","messageCount":25,"participants":["Mark Lodato","Jeff King","René Scharfe","Junio C Hamano","Michael Witten","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"135811","messageId":"1267246670-19118-1-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":null,"subject":"[PATCH 0/5] color enhancements, particularly for grep","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:45Z","receivedAt":"2010-02-27T04:57:45Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"The main purpose of this patch series is to add color to git grep a la\nGNU grep. The only change to the default is to colorize the separator\nbetween filename, line number, and match (':', '-', or '=') and between\nhunks ('--'). This improves readability immensely without being\ndistracting. However, the filename, line number, function line (-p),\nand non-matching text can also be colored, if desired.\n\nThe first three patches are each independent of any other patch, but\nthey seem like a good idea.\n\nMark Lodato (5):\n Allow explicit ANSI codes for colors\n Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*\n Remove reference to GREP_COLORS from documentation\n grep: Colorize filename, line number, and separator\n grep: Colorize selected, context, and function lines\n\n Documentation/config.txt | 32 +++++++++++++++++++---\n builtin-grep.c | 42 +++++++++++++++++++++-------\n color.c | 16 +++++++++++\n color.h | 11 +++++++\n graph.c | 12 ++++----\n grep.c | 66 ++++++++++++++++++++++++++++++---------------\n grep.h | 6 ++++\n t/t4026-color.sh | 18 ++++++++++++\n 8 files changed, 159 insertions(+), 44 deletions(-)\n"},{"id":"135808","messageId":"1267246670-19118-2-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":"1267246670-19118-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 1/5] Allow explicit ANSI codes for colors","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:46Z","receivedAt":"2010-02-27T04:57:46Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Allow explicit ANSI codes to be used in configuration options expecting\na color. The form is \"[...m\", where \"...\" are characters in the ASCII\nrange 0x30 to 0x3f. This allows users to specify more complex colors\n(generically, SGR attributes) than our color language allows. For\nexample, to get blinking, bold, underlined, italic, red text,\nuse \"[5;1;4;3;31m\".\n\nSigned-off-by: Mark Lodato \n---\n Documentation/config.txt | 4 ++++\n color.c | 16 ++++++++++++++++\n t/t4026-color.sh | 18 ++++++++++++++++++\n 3 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 664de6b..fed18cb 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -663,6 +663,10 @@ accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`,\n `blink` and `reverse`. The first color given is the foreground; the\n second is the background. The position of the attribute, if any,\n doesn't matter.\n++\n+Alternatively, a raw ANSI color code (SGR attribute) may be specified, in the\n+form `[...m` (no escape character). The `...` is a set of characters in the\n+ASCII range 0x30 to 0x3f.\n \n color.diff::\n \tWhen set to `always`, always use colors in patch.\ndiff --git a/color.c b/color.c\nindex 62977f4..1b42d9a 100644\n--- a/color.c\n+++ b/color.c\n@@ -56,6 +56,22 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n \t\treturn;\n \t}\n \n+\t/* If \"[...m\", use this as the attribute string directly */\n+\tif (value_len >= 2 && value[0] == '[' && value[value_len-1] == 'm') {\n+\t\tint i;\n+\t\tfor (i = 1; i < value_len-1; i++)\n+\t\t\tif (value[i] < 0x30 || value[i] >= 0x40)\n+\t\t\t\tgoto parse;\n+\t\tif (value_len + 2 > COLOR_MAXLEN)\n+\t\t\tdie(\"color value '%.*s' too long for variable '%s'\",\n+\t\t\t value_len, value, var);\n+\t\t*dst = '\\033';\n+\t\tmemcpy(dst+1, value, value_len);\n+\t\t*(dst+1+value_len) = 0;\n+\t\treturn;\n+\t}\n+\n+parse:\n \t/* [fg [bg]] [attr] */\n \twhile (len > 0) {\n \t\tconst char *word = ptr;\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex 5ade44c..754716f 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -46,6 +46,14 @@ test_expect_success '256 colors' '\n \tcolor \"254 bold 255\" \"[1;38;5;254;48;5;255m\"\n '\n \n+test_expect_success 'explicit attribute' '\n+\tcolor \"[123;456;7890m\" \"[123;456;7890m\"\n+'\n+\n+test_expect_success 'explicit attribute maximum length' '\n+\tcolor \"[00000000000000000000m\" \"[00000000000000000000m\"\n+'\n+\n test_expect_success 'color too small' '\n \tinvalid_color \"-2\"\n '\n@@ -66,6 +74,16 @@ test_expect_success 'extra character after attribute' '\n \tinvalid_color \"dimX\"\n '\n \n+test_expect_success 'explicit attribute invalid characters' '\n+\tinvalid_color \"[/m\" &&\n+\tinvalid_color \"[@m\" &&\n+\tinvalid_color \"[mm\"\n+'\n+\n+test_expect_success 'explicit attribute too long' '\n+\tinvalid_color \"[000000000000000000000m\"\n+'\n+\n test_expect_success 'unknown color slots are ignored (diff)' '\n \tgit config --unset diff.color.new\n \tgit config color.diff.nosuchslotwilleverbedefined white &&\n-- \n1.7.0\n"},{"id":"135813","messageId":"1267246670-19118-3-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":"1267246670-19118-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 2/5] Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:47Z","receivedAt":"2010-02-27T04:57:47Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Add GIT_COLOR_BOLD_* macros to set both bold and the color in one\nsequence. This saves two characters of output (\"ESC [ m\", minus \";\")\nand makes the code more readable.\n\nAdd the remaining GIT_COLOR_BG_* macros to make the list complete.\nThe white and black colors are not included since they look bad on most\nterminals.\n\nSigned-off-by: Mark Lodato \n---\n builtin-grep.c | 2 +-\n color.h | 11 +++++++++++\n graph.c | 12 ++++++------\n 3 files changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 552ef1f..dcc3d48 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -871,7 +871,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \topt.regflags = REG_NEWLINE;\n \topt.max_depth = -1;\n \n-\tstrcpy(opt.color_match, GIT_COLOR_RED GIT_COLOR_BOLD);\n+\tstrcpy(opt.color_match, GIT_COLOR_BOLD_RED);\n \topt.color = -1;\n \tgit_config(grep_config, &opt);\n \tif (opt.color == -1)\ndiff --git a/color.h b/color.h\nindex 3cb4b7f..bfeea1f 100644\n--- a/color.h\n+++ b/color.h\n@@ -18,7 +18,18 @@\n #define GIT_COLOR_BLUE\t\t\"\\033[34m\"\n #define GIT_COLOR_MAGENTA\t\"\\033[35m\"\n #define GIT_COLOR_CYAN\t\t\"\\033[36m\"\n+#define GIT_COLOR_BOLD_RED\t\"\\033[1;31m\"\n+#define GIT_COLOR_BOLD_GREEN\t\"\\033[1;32m\"\n+#define GIT_COLOR_BOLD_YELLOW\t\"\\033[1;33m\"\n+#define GIT_COLOR_BOLD_BLUE\t\"\\033[1;34m\"\n+#define GIT_COLOR_BOLD_MAGENTA\t\"\\033[1;35m\"\n+#define GIT_COLOR_BOLD_CYAN\t\"\\033[1;36m\"\n #define GIT_COLOR_BG_RED\t\"\\033[41m\"\n+#define GIT_COLOR_BG_GREEN\t\"\\033[42m\"\n+#define GIT_COLOR_BG_YELLOW\t\"\\033[43m\"\n+#define GIT_COLOR_BG_BLUE\t\"\\033[44m\"\n+#define GIT_COLOR_BG_MAGENTA\t\"\\033[45m\"\n+#define GIT_COLOR_BG_CYAN\t\"\\033[46m\"\n \n /*\n * This variable stores the value of color.ui\ndiff --git a/graph.c b/graph.c\nindex 6746d42..e6bbcaa 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -80,12 +80,12 @@ static char column_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_BLUE,\n \tGIT_COLOR_MAGENTA,\n \tGIT_COLOR_CYAN,\n-\tGIT_COLOR_BOLD GIT_COLOR_RED,\n-\tGIT_COLOR_BOLD GIT_COLOR_GREEN,\n-\tGIT_COLOR_BOLD GIT_COLOR_YELLOW,\n-\tGIT_COLOR_BOLD GIT_COLOR_BLUE,\n-\tGIT_COLOR_BOLD GIT_COLOR_MAGENTA,\n-\tGIT_COLOR_BOLD GIT_COLOR_CYAN,\n+\tGIT_COLOR_BOLD_RED,\n+\tGIT_COLOR_BOLD_GREEN,\n+\tGIT_COLOR_BOLD_YELLOW,\n+\tGIT_COLOR_BOLD_BLUE,\n+\tGIT_COLOR_BOLD_MAGENTA,\n+\tGIT_COLOR_BOLD_CYAN,\n };\n \n #define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors))\n-- \n1.7.0\n"},{"id":"135809","messageId":"1267246670-19118-4-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":"1267246670-19118-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 3/5] Remove reference to GREP_COLORS from documentation","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:48Z","receivedAt":"2010-02-27T04:57:48Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"There is no longer support for external grep, as per bbc09c22b9, so\nremove the reference to it in the documentation.\n\nSigned-off-by: Mark Lodato \n---\n Documentation/config.txt | 4 +---\n 1 files changed, 1 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex fed18cb..791b065 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -689,9 +689,7 @@ color.grep::\n \n color.grep.match::\n \tUse customized color for matches. The value of this variable\n-\tmay be specified as in color.branch.. It is passed using\n-\tthe environment variables 'GREP_COLOR' and 'GREP_COLORS' when\n-\tcalling an external 'grep'.\n+\tmay be specified as in color.branch..\n \n color.interactive::\n \tWhen set to `always`, always use colors for interactive prompts\n-- \n1.7.0\n"},{"id":"135810","messageId":"1267246670-19118-5-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":"1267246670-19118-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:49Z","receivedAt":"2010-02-27T04:57:49Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Colorize the filename, line number, and separator in git grep output, as\nGNU grep does. The colors are customizable through color.grep..\nThe default is to only color the separator (in cyan), since this gives\nthe biggest legibility increase without overwhelming the user with\ncolors. GNU grep also defaults cyan for the separator, but defaults to\nmagenta for the filename and to green for the line number, as well.\n\nThere are a few differences from GNU grep:\n\n1. With --name-only, GNU grep colors the filenames, but we do not. I do\n not see any point to making everything the same color.\n\n2. When a binary file matches without -a, GNU grep does not color the\n in \"Binary file matches\", but we do. The point of\n colorization is to highlight important parts, and the filename is an\n important part.\n\nLike GNU grep, if --null is given, the null separators are not colored.\n\nFor config.txt, use a a sub-list to describe the slots, rather than\na single paragraph with parentheses, since this is much more readable.\n\nSigned-off-by: Mark Lodato \n---\n Documentation/config.txt | 20 ++++++++++++++--\n builtin-grep.c | 31 +++++++++++++++++--------\n grep.c | 55 +++++++++++++++++++++++++++++----------------\n grep.h | 3 ++\n 4 files changed, 76 insertions(+), 33 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 791b065..154bc02 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -687,9 +687,23 @@ color.grep::\n \t`never`), never. When set to `true` or `auto`, use color only\n \twhen the output is written to the terminal. Defaults to `false`.\n \n-color.grep.match::\n-\tUse customized color for matches. The value of this variable\n-\tmay be specified as in color.branch..\n+color.grep.::\n+\tUse customized color for grep colorization. `` specifies which\n+\tpart of the line to use the specified color, and is one of\n++\n+--\n+`filename`:::\n+\tfilename prefix (when not using `-h`)\n+`linenumber`:::\n+\tline number prefix (when using `-n`)\n+`match`:::\n+\tmatching text\n+`separator`:::\n+\tseparators between fields on a line (`:`, `-`, and `=`)\n+\tand between hunks (`--`)\n+\n+The values of these variables may be specified as in color.branch..\n+--\n \n color.interactive::\n \tWhen set to `always`, always use colors for interactive prompts\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex dcc3d48..43b952b 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -289,6 +289,7 @@ static int wait_all(void)\n static int grep_config(const char *var, const char *value, void *cb)\n {\n \tstruct grep_opt *opt = cb;\n+\tchar *color = NULL;\n \n \tswitch (userdiff_config(var, value)) {\n \tcase 0: break;\n@@ -296,17 +297,24 @@ static int grep_config(const char *var, const char *value, void *cb)\n \tdefault: return 0;\n \t}\n \n-\tif (!strcmp(var, \"color.grep\")) {\n+\tif (!strcmp(var, \"color.grep\"))\n \t\topt->color = git_config_colorbool(var, value, -1);\n-\t\treturn 0;\n-\t}\n-\tif (!strcmp(var, \"color.grep.match\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tcolor_parse(value, var, opt->color_match);\n-\t\treturn 0;\n-\t}\n-\treturn git_color_default_config(var, value, cb);\n+\telse if (!strcmp(var, \"color.grep.filename\"))\n+\t\tcolor = opt->color_filename;\n+\telse if (!strcmp(var, \"color.grep.linenumber\"))\n+\t\tcolor = opt->color_lineno;\n+\telse if (!strcmp(var, \"color.grep.match\"))\n+\t\tcolor = opt->color_match;\n+\telse if (!strcmp(var, \"color.grep.separator\"))\n+\t\tcolor = opt->color_sep;\n+\telse\n+\t\treturn git_color_default_config(var, value, cb);\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\tcolor_parse(value, var, color);\n+\tif (!strcmp(color, GIT_COLOR_RESET))\n+\t\tcolor[0] = '\\0';\n+\treturn 0;\n }\n \n /*\n@@ -871,7 +879,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \topt.regflags = REG_NEWLINE;\n \topt.max_depth = -1;\n \n+\tstrcpy(opt.color_filename, \"\");\n+\tstrcpy(opt.color_lineno, \"\");\n \tstrcpy(opt.color_match, GIT_COLOR_BOLD_RED);\n+\tstrcpy(opt.color_sep, GIT_COLOR_CYAN);\n \topt.color = -1;\n \tgit_config(grep_config, &opt);\n \tif (opt.color == -1)\ndiff --git a/grep.c b/grep.c\nindex a0864f1..132798d 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -506,35 +506,52 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\n \treturn hit;\n }\n \n+static void output_color(struct grep_opt *opt, const void *data, size_t size,\n+\t\t\t const char *color)\n+{\n+\tif (opt->color && color && color[0]) {\n+\t\topt->output(opt, color, strlen(color));\n+\t\topt->output(opt, data, size);\n+\t\topt->output(opt, GIT_COLOR_RESET, strlen(GIT_COLOR_RESET));\n+\t}\n+\telse\n+\t\topt->output(opt, data, size);\n+}\n+\n+static void output_sep(struct grep_opt *opt, char sign)\n+{\n+\tif (opt->null_following_name) {\n+\t\tsign = '\\0';\n+\t\topt->output(opt, &sign, 1);\n+\t} else\n+\t\toutput_color(opt, &sign, 1, opt->color_sep);\n+}\n+\n static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t const char *name, unsigned lno, char sign)\n {\n \tint rest = eol - bol;\n-\tchar sign_str[1];\n \n-\tsign_str[0] = sign;\n \tif (opt->pre_context || opt->post_context) {\n \t\tif (opt->last_shown == 0) {\n \t\t\tif (opt->show_hunk_mark)\n-\t\t\t\topt->output(opt, \"--\\n\", 3);\n+\t\t\t\toutput_color(opt, \"--\\n\", 3, opt->color_sep);\n \t\t\telse\n \t\t\t\topt->show_hunk_mark = 1;\n \t\t} else if (lno > opt->last_shown + 1)\n-\t\t\topt->output(opt, \"--\\n\", 3);\n+\t\t\toutput_color(opt, \"--\\n\", 3, opt->color_sep);\n \t}\n \topt->last_shown = lno;\n \n-\tif (opt->null_following_name)\n-\t\tsign_str[0] = '\\0';\n \tif (opt->pathname) {\n-\t\topt->output(opt, name, strlen(name));\n-\t\topt->output(opt, sign_str, 1);\n+\t\toutput_color(opt, name, strlen(name), opt->color_filename);\n+\t\toutput_sep(opt, sign);\n \t}\n \tif (opt->linenum) {\n \t\tchar buf[32];\n \t\tsnprintf(buf, sizeof(buf), \"%d\", lno);\n-\t\topt->output(opt, buf, strlen(buf));\n-\t\topt->output(opt, sign_str, 1);\n+\t\toutput_color(opt, buf, strlen(buf), opt->color_lineno);\n+\t\toutput_sep(opt, sign);\n \t}\n \tif (opt->color) {\n \t\tregmatch_t match;\n@@ -548,12 +565,9 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t\t\tbreak;\n \n \t\t\topt->output(opt, bol, match.rm_so);\n-\t\t\topt->output(opt, opt->color_match,\n-\t\t\t\t strlen(opt->color_match));\n-\t\t\topt->output(opt, bol + match.rm_so,\n-\t\t\t\t (int)(match.rm_eo - match.rm_so));\n-\t\t\topt->output(opt, GIT_COLOR_RESET,\n-\t\t\t\t strlen(GIT_COLOR_RESET));\n+\t\t\toutput_color(opt, bol + match.rm_so,\n+\t\t\t\t (int)(match.rm_eo - match.rm_so),\n+\t\t\t\t opt->color_match);\n \t\t\tbol += match.rm_eo;\n \t\t\trest -= match.rm_eo;\n \t\t\teflags = REG_NOTBOL;\n@@ -823,7 +837,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t\t\t\treturn 1;\n \t\t\tif (binary_match_only) {\n \t\t\t\topt->output(opt, \"Binary file \", 12);\n-\t\t\t\topt->output(opt, name, strlen(name));\n+\t\t\t\toutput_color(opt, name, strlen(name),\n+\t\t\t\t\t opt->color_filename);\n \t\t\t\topt->output(opt, \" matches\\n\", 9);\n \t\t\t\treturn 1;\n \t\t\t}\n@@ -882,9 +897,9 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t */\n \tif (opt->count && count) {\n \t\tchar buf[32];\n-\t\topt->output(opt, name, strlen(name));\n-\t\tsnprintf(buf, sizeof(buf), \"%c%u\\n\",\n-\t\t\t opt->null_following_name ? '\\0' : ':', count);\n+\t\toutput_color(opt, name, strlen(name), opt->color_filename);\n+\t\toutput_sep(opt, ':');\n+\t\tsnprintf(buf, sizeof(buf), \"%u\\n\", count);\n \t\topt->output(opt, buf, strlen(buf));\n \t}\n \treturn !!last_hit;\ndiff --git a/grep.h b/grep.h\nindex 9703087..36919ee 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -84,7 +84,10 @@ struct grep_opt {\n \tint color;\n \tint max_depth;\n \tint funcname;\n+\tchar color_filename[COLOR_MAXLEN];\n+\tchar color_lineno[COLOR_MAXLEN];\n \tchar color_match[COLOR_MAXLEN];\n+\tchar color_sep[COLOR_MAXLEN];\n \tint regflags;\n \tunsigned pre_context;\n \tunsigned post_context;\n-- \n1.7.0\n"},{"id":"135812","messageId":"1267246670-19118-6-git-send-email-lodatom@gmail.com","threadId":"22841","inReplyTo":"1267246670-19118-1-git-send-email-lodatom@gmail.com","subject":"[PATCH 5/5] grep: Colorize selected, context, and function lines","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T04:57:50Z","receivedAt":"2010-02-27T04:57:50Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"Colorize non-matching text of selected lines, context lines, and\nfunction name lines. The default for all three is no color, but they\ncan be configured using color.grep.. The first two are similar\nto the corresponding options in GNU grep, except that GNU grep applies\nthe color to the entire line, not just non-matching text.\n\nSigned-off-by: Mark Lodato \n---\nTo me, the biggest benefit is the function line color. I don't find the other\ntwo useful, but they were trivial to implement.\n\n Documentation/config.txt | 6 ++++++\n builtin-grep.c | 9 +++++++++\n grep.c | 11 +++++++++--\n grep.h | 3 +++\n 4 files changed, 27 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 154bc02..999b1bd 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -692,12 +692,18 @@ color.grep.::\n \tpart of the line to use the specified color, and is one of\n +\n --\n+`context`:::\n+\tnon-matching text in context lines (when using `-A`, `-B`, or `-C`)\n `filename`:::\n \tfilename prefix (when not using `-h`)\n+`function`:::\n+\tfunction name lines (when using `-p`)\n `linenumber`:::\n \tline number prefix (when using `-n`)\n `match`:::\n \tmatching text\n+`selected`:::\n+\tnon-matching text in selected lines\n `separator`:::\n \tseparators between fields on a line (`:`, `-`, and `=`)\n \tand between hunks (`--`)\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 43b952b..2ae25c0 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -299,12 +299,18 @@ static int grep_config(const char *var, const char *value, void *cb)\n \n \tif (!strcmp(var, \"color.grep\"))\n \t\topt->color = git_config_colorbool(var, value, -1);\n+\telse if (!strcmp(var, \"color.grep.context\"))\n+\t\tcolor = opt->color_context;\n \telse if (!strcmp(var, \"color.grep.filename\"))\n \t\tcolor = opt->color_filename;\n+\telse if (!strcmp(var, \"color.grep.function\"))\n+\t\tcolor = opt->color_function;\n \telse if (!strcmp(var, \"color.grep.linenumber\"))\n \t\tcolor = opt->color_lineno;\n \telse if (!strcmp(var, \"color.grep.match\"))\n \t\tcolor = opt->color_match;\n+\telse if (!strcmp(var, \"color.grep.selected\"))\n+\t\tcolor = opt->color_selected;\n \telse if (!strcmp(var, \"color.grep.separator\"))\n \t\tcolor = opt->color_sep;\n \telse\n@@ -879,9 +885,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \topt.regflags = REG_NEWLINE;\n \topt.max_depth = -1;\n \n+\tstrcpy(opt.color_context, \"\");\n \tstrcpy(opt.color_filename, \"\");\n+\tstrcpy(opt.color_function, \"\");\n \tstrcpy(opt.color_lineno, \"\");\n \tstrcpy(opt.color_match, GIT_COLOR_BOLD_RED);\n+\tstrcpy(opt.color_selected, \"\");\n \tstrcpy(opt.color_sep, GIT_COLOR_CYAN);\n \topt.color = -1;\n \tgit_config(grep_config, &opt);\ndiff --git a/grep.c b/grep.c\nindex 132798d..adc93b0 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -531,6 +531,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t const char *name, unsigned lno, char sign)\n {\n \tint rest = eol - bol;\n+\tchar *line_color = NULL;\n \n \tif (opt->pre_context || opt->post_context) {\n \t\tif (opt->last_shown == 0) {\n@@ -559,12 +560,18 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\tint ch = *eol;\n \t\tint eflags = 0;\n \n+\t\tif (sign == ':')\n+\t\t\tline_color = opt->color_selected;\n+\t\telse if (sign == '-')\n+\t\t\tline_color = opt->color_context;\n+\t\telse if (sign == '=')\n+\t\t\tline_color = opt->color_function;\n \t\t*eol = '\\0';\n \t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n \t\t\tif (match.rm_so == match.rm_eo)\n \t\t\t\tbreak;\n \n-\t\t\topt->output(opt, bol, match.rm_so);\n+\t\t\toutput_color(opt, bol, match.rm_so, line_color);\n \t\t\toutput_color(opt, bol + match.rm_so,\n \t\t\t\t (int)(match.rm_eo - match.rm_so),\n \t\t\t\t opt->color_match);\n@@ -574,7 +581,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t}\n \t\t*eol = ch;\n \t}\n-\topt->output(opt, bol, rest);\n+\toutput_color(opt, bol, rest, line_color);\n \topt->output(opt, \"\\n\", 1);\n }\n \ndiff --git a/grep.h b/grep.h\nindex 36919ee..2c4bdac 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -84,9 +84,12 @@ struct grep_opt {\n \tint color;\n \tint max_depth;\n \tint funcname;\n+\tchar color_context[COLOR_MAXLEN];\n \tchar color_filename[COLOR_MAXLEN];\n+\tchar color_function[COLOR_MAXLEN];\n \tchar color_lineno[COLOR_MAXLEN];\n \tchar color_match[COLOR_MAXLEN];\n+\tchar color_selected[COLOR_MAXLEN];\n \tchar color_sep[COLOR_MAXLEN];\n \tint regflags;\n \tunsigned pre_context;\n-- \n1.7.0\n"},{"id":"135817","messageId":"20100227085144.GD27191@coredump.intra.peff.net","threadId":"22841","inReplyTo":"1267246670-19118-2-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 1/5] Allow explicit ANSI codes for colors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-27T08:51:45Z","receivedAt":"2010-02-27T08:51:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 26, 2010 at 11:57:46PM -0500, Mark Lodato wrote:\n\n> Allow explicit ANSI codes to be used in configuration options expecting\n> a color. The form is \"[...m\", where \"...\" are characters in the ASCII\n> range 0x30 to 0x3f. This allows users to specify more complex colors\n> (generically, SGR attributes) than our color language allows. For\n> example, to get blinking, bold, underlined, italic, red text,\n> use \"[5;1;4;3;31m\".\n\nI am not against this patch if it gets us some flexibility that is not\notherwise easy to attain, but wouldn't it be more user friendly for us\nto support \"red blink bold ul italic\"? AFAICT, the only things standing\nthe way of that are:\n\n - we don't support the italic attribute yet (are there are a lot of\n others that we are missing?)\n\n - the parser in color_parse_mem already understands how to parse\n multiple attributes, but it just complains after the first one\n\n-Peff\n"},{"id":"135819","messageId":"4B890572.5040604@lsrfire.ath.cx","threadId":"22841","inReplyTo":"1267246670-19118-5-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-02-27T11:43:46Z","receivedAt":"2010-02-27T11:43:46Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 27.02.2010 05:57, schrieb Mark Lodato:\n> Colorize the filename, line number, and separator in git grep output, as\n> GNU grep does. The colors are customizable through color.grep..\n> The default is to only color the separator (in cyan), since this gives\n> the biggest legibility increase without overwhelming the user with\n> colors. GNU grep also defaults cyan for the separator, but defaults to\n> magenta for the filename and to green for the line number, as well.\n> \n> There are a few differences from GNU grep:\n> \n> 1. With --name-only, GNU grep colors the filenames, but we do not. I do\n> not see any point to making everything the same color.\n\nI guess they did it for consistency, so when you see \"magenta\" you think\n\"filename\", and because it can be turned off with a switch. With your\npatch all filenames are coloured the same, too, by the way: using the\ndefault foreground colour. :)\n\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index dcc3d48..43b952b 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -289,6 +289,7 @@ static int wait_all(void)\n> static int grep_config(const char *var, const char *value, void *cb)\n> {\n> \tstruct grep_opt *opt = cb;\n> +\tchar *color = NULL;\n> \n> \tswitch (userdiff_config(var, value)) {\n> \tcase 0: break;\n> @@ -296,17 +297,24 @@ static int grep_config(const char *var, const char *value, void *cb)\n> \tdefault: return 0;\n> \t}\n> \n> -\tif (!strcmp(var, \"color.grep\")) {\n> +\tif (!strcmp(var, \"color.grep\"))\n> \t\topt->color = git_config_colorbool(var, value, -1);\n> -\t\treturn 0;\n> -\t}\n> -\tif (!strcmp(var, \"color.grep.match\")) {\n> -\t\tif (!value)\n> -\t\t\treturn config_error_nonbool(var);\n> -\t\tcolor_parse(value, var, opt->color_match);\n> -\t\treturn 0;\n> -\t}\n> -\treturn git_color_default_config(var, value, cb);\n> +\telse if (!strcmp(var, \"color.grep.filename\"))\n> +\t\tcolor = opt->color_filename;\n> +\telse if (!strcmp(var, \"color.grep.linenumber\"))\n> +\t\tcolor = opt->color_lineno;\n> +\telse if (!strcmp(var, \"color.grep.match\"))\n> +\t\tcolor = opt->color_match;\n> +\telse if (!strcmp(var, \"color.grep.separator\"))\n> +\t\tcolor = opt->color_sep;\n> +\telse\n> +\t\treturn git_color_default_config(var, value, cb);\n> +\tif (!value)\n> +\t\treturn config_error_nonbool(var);\n\ncolor.grep without a value used to turn on colourization, now it seems\nto error out.\n\n> +\tcolor_parse(value, var, color);\n> +\tif (!strcmp(color, GIT_COLOR_RESET))\n> +\t\tcolor[0] = '\\0';\n\nThis turns off colouring if the user specified \"reset\" as the colour,\nright? Interesting optimization, but is it really needed? Perhaps it's\njust me, but I'd give the user the requested \"text\"\nsequence if she asked for it, even if it's longer than and looks the\nsame as \"text\" alone.\n\n> diff --git a/grep.c b/grep.c\n> index a0864f1..132798d 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -506,35 +506,52 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\n> \treturn hit;\n> }\n> \n> +static void output_color(struct grep_opt *opt, const void *data, size_t size,\n> +\t\t\t const char *color)\n> +{\n> +\tif (opt->color && color && color[0]) {\n> +\t\topt->output(opt, color, strlen(color));\n> +\t\topt->output(opt, data, size);\n> +\t\topt->output(opt, GIT_COLOR_RESET, strlen(GIT_COLOR_RESET));\n> +\t}\n> +\telse\n\n\t} else\n\n> +\t\topt->output(opt, data, size);\n> +}\n> +\n> +static void output_sep(struct grep_opt *opt, char sign)\n> +{\n> +\tif (opt->null_following_name) {\n> +\t\tsign = '\\0';\n> +\t\topt->output(opt, &sign, 1);\n> +\t} else\n\n\tif (opt->null_following_name)\n\t\topt->output(opt, \"\", 1);\n\telse\n\n> +\t\toutput_color(opt, &sign, 1, opt->color_sep);\n> +}\n"},{"id":"135821","messageId":"4B89079C.8030206@lsrfire.ath.cx","threadId":"22841","inReplyTo":"1267246670-19118-5-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-02-27T11:53:00Z","receivedAt":"2010-02-27T11:53:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Forgot one thing in my earlier reply:\n\nAm 27.02.2010 05:57, schrieb Mark Lodato:\n> @@ -548,12 +565,9 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n> \t\t\t\tbreak;\n> \n> \t\t\topt->output(opt, bol, match.rm_so);\n> -\t\t\topt->output(opt, opt->color_match,\n> -\t\t\t\t strlen(opt->color_match));\n> -\t\t\topt->output(opt, bol + match.rm_so,\n> -\t\t\t\t (int)(match.rm_eo - match.rm_so));\n> -\t\t\topt->output(opt, GIT_COLOR_RESET,\n> -\t\t\t\t strlen(GIT_COLOR_RESET));\n> +\t\t\toutput_color(opt, bol + match.rm_so,\n> +\t\t\t\t (int)(match.rm_eo - match.rm_so),\n> +\t\t\t\t opt->color_match);\n\nThe third parameter of output_color() (and of ->output(), so you didn't\nintroduce this, of course) is a size_t, so why cast to int? Is a cast\nneeded at all?\n"},{"id":"135844","messageId":"7vy6ie1u9a.fsf@alter.siamese.dyndns.org","threadId":"22841","inReplyTo":"4B89079C.8030206@lsrfire.ath.cx","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-27T17:08:33Z","receivedAt":"2010-02-27T17:08:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>> -\t\t\topt->output(opt, bol + match.rm_so,\n>> -\t\t\t\t (int)(match.rm_eo - match.rm_so));\n>\n> The third parameter of output_color() (and of ->output(), so you didn't\n> introduce this, of course) is a size_t, so why cast to int? Is a cast\n> needed at all?\n\nI don't think so.\n\nEarlier in 747a322 (grep: cast printf %.*s \"precision\" argument explicitly\nto int, 2009-03-08), I casted the difference between two regoff_t you were\nfeeding to printf's \"%.*s\" as a length, introduced by 7e8f59d (grep: color\npatterns in output, 2009-03-07), and 5b594f4 (Threaded grep, 2010-01-25)\ncarried that cast over without thinking.\n"},{"id":"135845","messageId":"ca433831002271024t5af1dba9m6ca719c114e54892@mail.gmail.com","threadId":"22841","inReplyTo":"20100227085144.GD27191@coredump.intra.peff.net","subject":"Re: [PATCH 1/5] Allow explicit ANSI codes for colors","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-27T18:24:53Z","receivedAt":"2010-02-27T18:24:53Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sat, Feb 27, 2010 at 3:51 AM, Jeff King wrote:\n> I am not against this patch if it gets us some flexibility that is not\n> otherwise easy to attain,\n\nBesides disallowing multiple attributes (e.g. bold blink), the current\nparser does not have a way to specify colors for 16-color mode colors\n8-15, 256-color mode colors 0-7, or any 88-color mode colors. There\nare also other esoteric attributes [1] that some user might want to\nuse, such as italic or franktur. I don't know if anyone will ever use\nthis feature, but it wasn't hard to implement.\n\n> but wouldn't it be more user friendly for us\n> to support \"red blink bold ul italic\"?\n\nYes, I think this should be done whether or not the patch in question\nis accepted.\n\n> AFAICT, the only things standing\n> the way of that are:\n>\n>  - we don't support the italic attribute yet (are there are a lot of\n>    others that we are missing?)\n\nWikipedia [1] lists a whole bunch of codes, including italic, but I\ndoubt anyone uses them. My thought was if someone really wanted to\nuse one of these obscure codes, they could do it with the patch in\nquestion. I don't think it's worth allowing users to type \"italic\".\n\n>  - the parser in color_parse_mem already understands how to parse\n>    multiple attributes, but it just complains after the first one\n\nIt seems like this restriction should be lifted. However, if this is\ndone, then COLOR_MAXLEN should be increased to 32 or so, and there\nmust be explicit checks to make sure the buffer does not overflow.\nTechnically, VT500 terminals accept up to 16 parameters up to 5 digits\neach [2], which would be 98 bytes, but this is overkill.\n\n[1] http://en.wikipedia.org/wiki/ANSI_escape_code\n[2] http://vt100.net/emu/dec_ansi_parser#ACPAR\n"},{"id":"135853","messageId":"7vfx4mv0h9.fsf@alter.siamese.dyndns.org","threadId":"22841","inReplyTo":"ca433831002271024t5af1dba9m6ca719c114e54892@mail.gmail.com","subject":"Re: [PATCH 1/5] Allow explicit ANSI codes for colors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-27T21:21:22Z","receivedAt":"2010-02-27T21:21:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato writes:\n\n> On Sat, Feb 27, 2010 at 3:51 AM, Jeff King wrote:\n>> I am not against this patch if it gets us some flexibility that is not\n>> otherwise easy to attain,\n>\n> Besides disallowing multiple attributes (e.g. bold blink), the current\n> parser does not have a way to specify colors for 16-color mode colors\n> 8-15, 256-color mode colors 0-7, or any 88-color mode colors. There\n> are also other esoteric attributes [1] that some user might want to\n> use, such as italic or franktur. I don't know if anyone will ever use\n> this feature, but it wasn't hard to implement.\n\nThe purist side of me has been hoping that we could later wean off this\nANSI centric view of the terminal attribute handling and move us to\nsomething based on terminfo. This patch makes it even harder by going\nquite the opposite way.\n\nBut the pragmatic side of me has long held a feeling that nobody who would\nuse git uses real terminals these days anymore, and there is no terminal\nemulator that does not grok ANSI sequences. msysgit folks have even done\ntheir own ANSI color emulation in their \"Console\" interface layer, so that\nmay be another reason that we are practically married to ANSI sequence and\nthere is not much gained by introducing terminfo as another layer of\nabstraction to build GIT_COLOR_* on top of.\n\nWhat I am saying is that the purist in me actively hates [PATCH 1/5], but\nthe pragmatist in me admits it would not hurt in practice.\n\n>> but wouldn't it be more user friendly for us\n>> to support \"red blink bold ul italic\"?\n>\n> Yes, I think this should be done whether or not the patch in question\n> is accepted.\n\nHmm, I do not care much about italic, blink nor ul, but perhaps other\npeople do. Combining attributes like \"reverse bold\" would probably make\nsense.\n\n color.c | 35 +++++++++++++++++++++++++++--------\n 1 files changed, 27 insertions(+), 8 deletions(-)\n\ndiff --git a/color.c b/color.c\nindex 62977f4..f210d94 100644\n--- a/color.c\n+++ b/color.c\n@@ -47,7 +47,8 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n {\n \tconst char *ptr = value;\n \tint len = value_len;\n-\tint attr = -1;\n+\tint attr[20];\n+\tint attr_idx = 0;\n \tint fg = -2;\n \tint bg = -2;\n \n@@ -56,7 +57,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n \t\treturn;\n \t}\n \n-\t/* [fg [bg]] [attr] */\n+\t/* [fg [bg]] [attr]... */\n \twhile (len > 0) {\n \t\tconst char *word = ptr;\n \t\tint val, wordlen = 0;\n@@ -85,19 +86,37 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n \t\t\tgoto bad;\n \t\t}\n \t\tval = parse_attr(word, wordlen);\n-\t\tif (val < 0 || attr != -1)\n+\t\tif (0 <= val && attr_idx < ARRAY_SIZE(attr))\n+\t\t\tattr[attr_idx++] = val;\n+\t\telse\n \t\t\tgoto bad;\n-\t\tattr = val;\n \t}\n \n-\tif (attr >= 0 || fg >= 0 || bg >= 0) {\n+\tif (attr_idx > 0 || fg >= 0 || bg >= 0) {\n \t\tint sep = 0;\n+\t\tint i;\n+\n+\t\tif (COLOR_MAXLEN <=\n+\t\t /* Number of bytes to denote colors and attributes */\n+\t\t (attr_idx\n+\t\t + (fg < 0 ? 0 :\n+\t\t\t((fg < 8) ? 2 : 8)) /* \"3x\" or \"38;5;xxx\" */\n+\t\t + (bg < 0 ? 0 :\n+\t\t\t((bg < 8) ? 2 : 8)) /* \"4x\" or \"48;5;xxx\" */\n+\t\t\t ) +\n+\t\t /* Number of semicolons between the above */\n+\t\t (attr_idx + (0 <= fg) + (0 <= bg) - 1) +\n+\t\t /* ESC '[', terminating 'm' and NUL */\n+\t\t 4)\n+\t\t\tgoto bad;\n \n \t\t*dst++ = '\\033';\n \t\t*dst++ = '[';\n-\t\tif (attr >= 0) {\n-\t\t\t*dst++ = '0' + attr;\n-\t\t\tsep++;\n+\n+\t\tfor (i = 0; i < attr_idx; i++) {\n+\t\t\tif (sep++)\n+\t\t\t\t*dst++ = ';';\n+\t\t\t*dst++ = '0' + attr[i];\n \t\t}\n \t\tif (fg >= 0) {\n \t\t\tif (sep++)\n"},{"id":"135866","messageId":"1267325798-8280-1-git-send-email-gitster@pobox.com","threadId":"22841","inReplyTo":"7vfx4mv0h9.fsf@alter.siamese.dyndns.org","subject":"[PATCH] color: allow multiple attributes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-28T02:56:38Z","receivedAt":"2010-02-28T02:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In configuration files (and \"git config --color\" command line), we\nsupported one and only one attribute after foreground and background\ncolor. Accept combinations of attributes, e.g.\n\n [diff.color]\n old = red reverse bold\n\nSigned-off-by: Junio C Hamano \n---\n Junio C Hamano writes:\n\n >>> but wouldn't it be more user friendly for us\n >>> to support \"red blink bold ul italic\"?\n >>\n >> Yes, I think this should be done whether or not the patch in question\n >> is accepted.\n\n This time with a bit of test updates as well for real inclusion.\n\n Also I realized that we can stuff them in an unsigned flag word as\n bitfields (\"red bold\" and \"red bold bold bold\" would give the same\n boldness anyway) to lift the artificial limit of number of attribute\n words.\n\n color.c | 47 +++++++++++++++++++++++++++++++++++++++--------\n t/t4026-color.sh | 15 +++++++++++----\n 2 files changed, 50 insertions(+), 12 deletions(-)\n\ndiff --git a/color.c b/color.c\nindex db4dccf..17eb3ec 100644\n--- a/color.c\n+++ b/color.c\n@@ -44,12 +44,23 @@ void color_parse(const char *value, const char *var, char *dst)\n \tcolor_parse_mem(value, strlen(value), var, dst);\n }\n \n+static int count_bits(unsigned flag)\n+{\n+\tint cnt = 0;\n+\twhile (flag) {\n+\t\tif (flag & 01)\n+\t\t\tcnt++;\n+\t\tflag >>= 1;\n+\t}\n+\treturn cnt;\n+}\n+\n void color_parse_mem(const char *value, int value_len, const char *var,\n \t\tchar *dst)\n {\n \tconst char *ptr = value;\n \tint len = value_len;\n-\tint attr = -1;\n+\tunsigned int attr = 0;\n \tint fg = -2;\n \tint bg = -2;\n \n@@ -58,7 +69,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n \t\treturn;\n \t}\n \n-\t/* [fg [bg]] [attr] */\n+\t/* [fg [bg]] [attr]... */\n \twhile (len > 0) {\n \t\tconst char *word = ptr;\n \t\tint val, wordlen = 0;\n@@ -87,19 +98,39 @@ void color_parse_mem(const char *value, int value_len, const char *var,\n \t\t\tgoto bad;\n \t\t}\n \t\tval = parse_attr(word, wordlen);\n-\t\tif (val < 0 || attr != -1)\n+\t\tif (0 <= val)\n+\t\t\tattr |= (1 << val);\n+\t\telse\n \t\t\tgoto bad;\n-\t\tattr = val;\n \t}\n \n-\tif (attr >= 0 || fg >= 0 || bg >= 0) {\n+\tif (attr || fg >= 0 || bg >= 0) {\n \t\tint sep = 0;\n+\t\tint i;\n+\t\tint num_attrs = count_bits(attr);\n+\n+\t\tif (COLOR_MAXLEN <=\n+\t\t /* Number of bytes to denote colors and attributes */\n+\t\t num_attrs\n+\t\t + (fg < 0 ? 0 : (fg < 8) ? 2 : 8) /* \"3x\" or \"38;5;xxx\" */\n+\t\t + (bg < 0 ? 0 : (bg < 8) ? 2 : 8) /* \"4x\" or \"48;5;xxx\" */\n+\t\t /* Number of semicolons between the above elements */\n+\t\t + (num_attrs + (0 <= fg) + (0 <= bg) - 1)\n+\t\t /* ESC '[', terminating 'm' and NUL */\n+\t\t + 4)\n+\t\t\tgoto bad;\n \n \t\t*dst++ = '\\033';\n \t\t*dst++ = '[';\n-\t\tif (attr >= 0) {\n-\t\t\t*dst++ = '0' + attr;\n-\t\t\tsep++;\n+\n+\t\tfor (i = 0; attr; i++) {\n+\t\t\tunsigned bit = (1 << i);\n+\t\t\tif (!(attr & bit))\n+\t\t\t\tcontinue;\n+\t\t\tattr &= ~bit;\n+\t\t\tif (sep++)\n+\t\t\t\t*dst++ = ';';\n+\t\t\t*dst++ = '0' + i;\n \t\t}\n \t\tif (fg >= 0) {\n \t\t\tif (sep++)\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex b61e516..c3af190 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -8,14 +8,13 @@ test_description='Test diff/status color escape codes'\n \n color()\n {\n-\tgit config diff.color.new \"$1\" &&\n-\ttest \"`git config --get-color diff.color.new`\" = \"\u001b$2\"\n+\tactual=$(git config --get-color no.such.slot \"$1\") &&\n+\ttest \"$actual\" = \"\u001b$2\"\n }\n \n invalid_color()\n {\n-\tgit config diff.color.new \"$1\" &&\n-\ttest -z \"`git config --get-color diff.color.new 2>/dev/null`\"\n+\ttest_must_fail git config --get-color no.such.slot \"$1\"\n }\n \n test_expect_success 'reset' '\n@@ -42,6 +41,14 @@ test_expect_success 'fg bg attr' '\n \tcolor \"blue red ul\" \"[4;34;41m\"\n '\n \n+test_expect_success 'fg bg attr...' '\n+\tcolor \"blue bold dim ul blink reverse\" \"[1;2;4;5;7;34m\"\n+'\n+\n+test_expect_success 'color specification too long' '\n+\tinvalid_color \"254 255 bold dim ul blink reverse\" \"[1;2;4;5;7;38;5;254;48;5;255m\"\n+'\n+\n test_expect_success '256 colors' '\n \tcolor \"254 bold 255\" \"[1;38;5;254;48;5;255m\"\n '\n-- \n1.7.0.270.g320aa\n"},{"id":"135875","messageId":"20100228122019.GB24247@coredump.intra.peff.net","threadId":"22841","inReplyTo":"1267325798-8280-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH] color: allow multiple attributes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-28T12:20:19Z","receivedAt":"2010-02-28T12:20:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 27, 2010 at 06:56:38PM -0800, Junio C Hamano wrote:\n\n> >>> but wouldn't it be more user friendly for us\n> >>> to support \"red blink bold ul italic\"?\n> >>\n> >> Yes, I think this should be done whether or not the patch in question\n> >> is accepted.\n> \n> This time with a bit of test updates as well for real inclusion.\n\nLooks OK to me, but...\n\n> Also I realized that we can stuff them in an unsigned flag word as\n> bitfields (\"red bold\" and \"red bold bold bold\" would give the same\n> boldness anyway) to lift the artificial limit of number of attribute\n> words.\n\nI also had this thought, but shouldn't that mean:\n\n> +\t\tint i;\n> +\t\tint num_attrs = count_bits(attr);\n> +\n> +\t\tif (COLOR_MAXLEN <=\n> +\t\t /* Number of bytes to denote colors and attributes */\n> +\t\t num_attrs\n> +\t\t + (fg < 0 ? 0 : (fg < 8) ? 2 : 8) /* \"3x\" or \"38;5;xxx\" */\n> +\t\t + (bg < 0 ? 0 : (bg < 8) ? 2 : 8) /* \"4x\" or \"48;5;xxx\" */\n> +\t\t /* Number of semicolons between the above elements */\n> +\t\t + (num_attrs + (0 <= fg) + (0 <= bg) - 1)\n> +\t\t /* ESC '[', terminating 'm' and NUL */\n> +\t\t + 4)\n> +\t\t\tgoto bad;\n\nWe don't need this, because the length of what can be specified is\nbounded, and we simply need to set COLOR_MAXLEN high enough to handle\nthe longest case? Though I suppose it doesn't hurt to be paranoid.\n\n> +test_expect_success 'fg bg attr...' '\n> +\tcolor \"blue bold dim ul blink reverse\" \"[1;2;4;5;7;34m\"\n> +'\n\nHmm. Just a thought on the bit-setting approach, but does the order of\nattributes ever matter? We are going to lose the ordering information\nthe user specifies, obviously.\n\n-Peff\n"},{"id":"135884","messageId":"7vhbp1cjkc.fsf@alter.siamese.dyndns.org","threadId":"22841","inReplyTo":"20100228122019.GB24247@coredump.intra.peff.net","subject":"Re: [PATCH] color: allow multiple attributes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-28T18:16:19Z","receivedAt":"2010-02-28T18:16:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +\t\tif (COLOR_MAXLEN <=\n>> +\t\t /* Number of bytes to denote colors and attributes */\n>> +\t\t num_attrs\n>> +\t\t + (fg < 0 ? 0 : (fg < 8) ? 2 : 8) /* \"3x\" or \"38;5;xxx\" */\n>> +\t\t + (bg < 0 ? 0 : (bg < 8) ? 2 : 8) /* \"4x\" or \"48;5;xxx\" */\n>> +\t\t /* Number of semicolons between the above elements */\n>> +\t\t + (num_attrs + (0 <= fg) + (0 <= bg) - 1)\n>> +\t\t /* ESC '[', terminating 'm' and NUL */\n>> +\t\t + 4)\n>> +\t\t\tgoto bad;\n>\n> We don't need this, because the length of what can be specified is\n> bounded, and we simply need to set COLOR_MAXLEN high enough to handle\n> the longest case?\n\nYes, I think we are now bounded and don't need this; I just thought it\nwould have an educational value to show how to comment a complex\nexpression in a readable way ;-)\n\n>> +test_expect_success 'fg bg attr...' '\n>> +\tcolor \"blue bold dim ul blink reverse\" \"[1;2;4;5;7;34m\"\n>> +'\n>\n> Hmm. Just a thought on the bit-setting approach, but does the order of\n> attributes ever matter? We are going to lose the ordering information\n> the user specifies, obviously.\n\nTrue, I don't know if it matters. I don't know if \"blue bold bold\" would\nresult in bolder blue than \"blue bold\" on some terminal emulators, either.\n\nI'd suggest that we ignore the issue for now, and when somebody complains\nwith an actual non-working case, we would assess the damage that comes\nfrom this reordering to decide what to do next. Parhaps a \"non-working\ncase\" could be \"'blink ul' blinks letter without blinking underline, but\n'ul blink' makes both letter and underline blink\". At that point we can\nsay \"Ok, you found a case the order changes the results. But does that\ndifference matter in practice?\" and move forward, either by fixing it, or\ndeclaring it doesn't matter in practice.\n\nWe were already losing the order by emitting attr then fg then bg even\nthough attr can come before any colors (an undocumented side effect of a\nsloppy parsing logic, but some of the existing tests insist on kepping it\nworking), by the way.\n"},{"id":"135885","messageId":"20100228183349.GA24273@coredump.intra.peff.net","threadId":"22841","inReplyTo":"7vhbp1cjkc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] color: allow multiple attributes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-28T18:33:49Z","receivedAt":"2010-02-28T18:33:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 28, 2010 at 10:16:19AM -0800, Junio C Hamano wrote:\n\n> > Hmm. Just a thought on the bit-setting approach, but does the order of\n> > attributes ever matter? We are going to lose the ordering information\n> > the user specifies, obviously.\n> \n> True, I don't know if it matters. I don't know if \"blue bold bold\" would\n> result in bolder blue than \"blue bold\" on some terminal emulators, either.\n> \n> I'd suggest that we ignore the issue for now, and when somebody complains\n> with an actual non-working case, we would assess the damage that comes\n> from this reordering to decide what to do next. Parhaps a \"non-working\n\nI'm fine with that. FWIW, I tested and blink-before-ul and\nul-before-blink look identical in an xterm. Certainly that's not the\nonly terminal emulator people will use, but I expect it to be\nrepresentative of the behavior of most emulators. I can dig my VT100\nout of the attic if we want a real answer. ;)\n\n> We were already losing the order by emitting attr then fg then bg even\n> though attr can come before any colors (an undocumented side effect of a\n> sloppy parsing logic, but some of the existing tests insist on kepping it\n> working), by the way.\n\nTrue. I also tested attr-before-color, then color-before-attr for both\nreverse and bold, and they look the same in an xterm. So I suspect it\ndoesn't matter, and I'm too lazy to do more research unless somebody\nfinds something that actually doesn't work.\n\n-Peff\n"},{"id":"135886","messageId":"7vd3zp88gn.fsf@alter.siamese.dyndns.org","threadId":"22841","inReplyTo":"1267246670-19118-5-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-28T19:29:44Z","receivedAt":"2010-02-28T19:29:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato writes:\n\n> +color.grep.::\n> +\tUse customized color for grep colorization. `` specifies which\n> +\tpart of the line to use the specified color, and is one of\n> ++\n> +--\n> +`filename`:::\n> +\tfilename prefix (when not using `-h`)\n> +`linenumber`:::\n\nWhy do I get a feeling that I already said something about three colons?\n\n ... goes and looks ...\n\nAh, it wasn't to you. Please see:\n\n http://thread.gmane.org/gmane.comp.version-control.git/139014/focus=139343\n\nBUT.\n\nI tried the three-colons notation with AsciiDoc 8.2.7 and it seems to take\nit as enumeration items that are nested a level deeper, so this might be\nsafe.\n\nBut the last sentence about color.branch. is indented as if it is a\npart of description for \"separator\" slot, which you may want to fix\nregardless.\n"},{"id":"135891","messageId":"ca433831002281214q14e6e62bj54cf7227cd32873b@mail.gmail.com","threadId":"22841","inReplyTo":"4B890572.5040604@lsrfire.ath.cx","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-28T20:14:40Z","receivedAt":"2010-02-28T20:14:40Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sat, Feb 27, 2010 at 6:43 AM, René Scharfe\n wrote:\n> Am 27.02.2010 05:57, schrieb Mark Lodato:\n>> 1. With --name-only, GNU grep colors the filenames, but we do not.  I do\n>>    not see any point to making everything the same color.\n>\n> I guess they did it for consistency, so when you see \"magenta\" you think\n> \"filename\", and because it can be turned off with a switch.  With your\n> patch all filenames are coloured the same, too, by the way: using the\n> default foreground colour. :)\n\nYes, I think I understand the reasoning, but to me it is very\nannoying. However, if there is a consensus that we should follow GNU\ngrep in this regard, I will do it.\n\n>> diff --git a/builtin-grep.c b/builtin-grep.c\n>> +     if (!value)\n>> +             return config_error_nonbool(var);\n>\n> color.grep without a value used to turn on colourization, now it seems\n> to error out.\n\nOops, that should be \"if (color && !value)\". I will fix in next respin.\n\n>> +     color_parse(value, var, color);\n>> +     if (!strcmp(color, GIT_COLOR_RESET))\n>> +             color[0] = '\\0';\n>\n> This turns off colouring if the user specified \"reset\" as the colour,\n> right?\n\nYes.\n\n> Interesting optimization, but is it really needed? Perhaps it's\n> just me, but I'd give the user the requested \"text\"\n> sequence if she asked for it, even if it's longer than and looks the\n> same as \"text\" alone.\n\nThe problem is that there's no way to say \"no color\". A blank value\nand \"reset\" both come to the same thing. I would rather have as\nlittle markup as possible in the output, and this tweak is very\nsimple. While this is not strictly necessary, it does make the output\nidentical to the pre-patch output if you disable all the new colors\n(just grep.color.separator, by default.)\n\n>> +     }\n>> +     else\n>\n>        } else\n\nOops, thanks.\n\n>> +     if (opt->null_following_name) {\n>> +             sign = '\\0';\n>> +             opt->output(opt, &sign, 1);\n>> +     } else\n>\n>        if (opt->null_following_name)\n>                opt->output(opt, \"\", 1);\n>        else\n\nPersonally, I find your suggestion less readable. My version is only\none line longer but makes the code completely obvious, whereas the\none-liner requires a second of thought. Anyone else care to comment\non this?\n"},{"id":"135893","messageId":"ca433831002281215i66af2401n221813466f2ffa85@mail.gmail.com","threadId":"22841","inReplyTo":"7vy6ie1u9a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-28T20:15:40Z","receivedAt":"2010-02-28T20:15:40Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sat, Feb 27, 2010 at 12:08 PM, Junio C Hamano wrote:\n> René Scharfe writes:\n>\n>>> -                    opt->output(opt, bol + match.rm_so,\n>>> -                                (int)(match.rm_eo - match.rm_so));\n>>\n>> The third parameter of output_color() (and of ->output(), so you didn't\n>> introduce this, of course) is a size_t, so why cast to int?  Is a cast\n>> needed at all?\n>\n> I don't think so.\n>\n> Earlier in 747a322 (grep: cast printf %.*s \"precision\" argument explicitly\n> to int, 2009-03-08), I casted the difference between two regoff_t you were\n> feeding to printf's \"%.*s\" as a length, introduced by 7e8f59d (grep: color\n> patterns in output, 2009-03-07), and 5b594f4 (Threaded grep, 2010-01-25)\n> carried that cast over without thinking.\n\nOk. I'll remove the cast. Should I note this in the commit message?\n"},{"id":"135896","messageId":"ca433831002281239j2fc17033xfa013bf896dcda2c@mail.gmail.com","threadId":"22841","inReplyTo":"7vd3zp88gn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-02-28T20:39:31Z","receivedAt":"2010-02-28T20:39:31Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sun, Feb 28, 2010 at 2:29 PM, Junio C Hamano wrote:\n> Mark Lodato writes:\n>\n>> +color.grep.::\n>> +     Use customized color for grep colorization.  `` specifies which\n>> +     part of the line to use the specified color, and is one of\n>> ++\n>> +--\n>> +`filename`:::\n>> +     filename prefix (when not using `-h`)\n>> +`linenumber`:::\n>\n> Why do I get a feeling that I already said something about three colons?\n>\n>  ... goes and looks ...\n>\n> Ah, it wasn't to you.  Please see:\n>\n>  http://thread.gmane.org/gmane.comp.version-control.git/139014/focus=139343\n>\n> BUT.\n>\n> I tried the three-colons notation with AsciiDoc 8.2.7 and it seems to take\n> it as enumeration items that are nested a level deeper, so this might be\n> safe.\n\nWhen I wrote the patch originally, I tried to find the difference\nbetween triple colons and double semi-colons but failed. Now that I\nlook at the changelog, double semi-colons was introduced in 5.0.9, but\nthere is no indication whatsoever of triple colons. The wording\nimplies that double-semicolon is older, so it's probably safer to use.\n I'll switch to that.\n\n> But the last sentence about color.branch. is indented as if it is a\n> part of description for \"separator\" slot, which you may want to fix\n> regardless.\n>\n\nMan! It worked in AsciiDoc 8.4.4, but evidently not in 8.2.7. What a\npain. I just installed 8.2.7 so hopefully I won't run into these\nproblems in the future. I'll fix this.\n"},{"id":"135916","messageId":"b4087cc51002281426m126a0c07l9f4a38088d0146b1@mail.gmail.com","threadId":"22841","inReplyTo":"ca433831002281214q14e6e62bj54cf7227cd32873b@mail.gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2010-02-28T22:26:30Z","receivedAt":"2010-02-28T22:26:30Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"On Sun, Feb 28, 2010 at 14:14, Mark Lodato wrote:\n> On Sat, Feb 27, 2010 at 6:43 AM, René Scharfe\n> wrote:\n>> Am 27.02.2010 05:57, schrieb Mark Lodato:\n>>> 1. With --name-only, GNU grep colors the filenames, but we do not.  I do\n>>>    not see any point to making everything the same color.\n>>\n>> I guess they did it for consistency, so when you see \"magenta\" you think\n>> \"filename\", and because it can be turned off with a switch.  With your\n>> patch all filenames are coloured the same, too, by the way: using the\n>> default foreground colour. :)\n>\n> Yes, I think I understand the reasoning, but to me it is very\n> annoying.  However, if there is a consensus that we should follow GNU\n> grep in this regard, I will do it.\n\nI'm in favor of colorizing the output even when just one piece of\ninformation is presented. If I turn on colorization, then there should\nbe colorization; my brain would expect it, especially when I first\ngrep without --name-only and then turn on --name-only after getting\nresults that I like.\n\nOf course, I bet you find colorizing the filenames a nuisance because\nyou don't care to pipe the relevant escape sequences to other\ncommands. On that note, it would be nice to have something like GNU's\n--color=(auto|yes|no) with `auto' as the default for a plain --color.\n\nAs a compromise (and perhaps as an improvement), perhaps only the\nbasename of the filename should be colorized when --name-only is used;\nthat way, colorization is still being used to differentiate different\ndata, and the rest of the path is usually not that interesting anyway.\nHowever, for consistency, I would still think it wise to colorize the\ndirname portion with `color.grep.filename', but color the basename\nportion with `color.grep.match' (as though the basename portion is the\ntext being matched).\n\nSincerely,\nMichael Witten\n"},{"id":"135997","messageId":"ca433831003011749h43293f80kd4ec18bd796dea7c@mail.gmail.com","threadId":"22841","inReplyTo":"b4087cc51002281426m126a0c07l9f4a38088d0146b1@mail.gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-02T01:49:08Z","receivedAt":"2010-03-02T01:49:08Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sun, Feb 28, 2010 at 5:26 PM, Michael Witten wrote:\n> Of course, I bet you find colorizing the filenames a nuisance because\n> you don't care to pipe the relevant escape sequences to other\n> commands.\n\nI'm not quite sure what you mean here, but my reason has nothing to do\nwith piping. If the output is entirely in a single color, I would\nprefer that color to be my terminal's default. The color adds no\nvalue.\n\n> On that note, it would be nice to have something like GNU's\n> --color=(auto|yes|no) with `auto' as the default for a plain --color.\n\nSomething like [1]? By the way, the default should be 'always', not\n'auto', to be consistent with GNU tools, and to be backwards\ncompatible with the old --color behavior.\n\n[1] http://permalink.gmane.org/gmane.comp.version-control.git/139864\n\n> As a compromise (and perhaps as an improvement), perhaps only the\n> basename of the filename should be colorized when --name-only is used;\n> that way, colorization is still being used to differentiate different\n> data, and the rest of the path is usually not that interesting anyway.\n> However, for consistency, I would still think it wise to colorize the\n> dirname portion with `color.grep.filename', but color the basename\n> portion with `color.grep.match' (as though the basename portion is the\n> text being matched).\n\nPersonally, I am not a fan of this, but if it is implemented, it\nshould be an option, and should be turned off by default. Instead of\nhighlighting the name, it may be better to simply highlight the\nslashes so the reader can more easily parse the path. But still, I\ndon't think this is worth the trouble.\n"},{"id":"136000","messageId":"4b8cb38b.870fcc0a.7ebc.1a83@mx.google.com","threadId":"22841","inReplyTo":"ca433831003011749h43293f80kd4ec18bd796dea7c@mail.gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2010-03-02T06:43:23Z","receivedAt":"2010-03-02T06:43:23Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"> Something like [1]?\n\nAh! Good!\n\n>> with `auto' as the default for a plain --color.\n>\n> By the way, the default should be 'always', not\n> 'auto', to be consistent with GNU tools, and to be backwards\n> compatible with the old --color behavior.\n\nWell, I've got:\n\n GNU grep 2.5.4\n\nand the default for a plain `--color' seems to be `auto', whereby\ncolorization is turned on when stdout is attached to a tty capable\nof color, but turned off otherwise:\n\n echo t > t\n\n /bin/grep t t --color # t is colored red\n /bin/grep t t --color | cat # t is not colored\n /bin/grep t t --color=always | cat # t is colored\n\nLet's see how the GNU grep source sets up colorization:\n\n case COLOR_OPTION:\n if(optarg) {\n if(!strcasecmp(optarg, \"always\") || !strcasecmp(optarg, \"yes\") ||\n !strcasecmp(optarg, \"force\"))\n color_option = 1;\n else if(!strcasecmp(optarg, \"never\") || !strcasecmp(optarg, \"no\") ||\n !strcasecmp(optarg, \"none\"))\n color_option = 0;\n else if(!strcasecmp(optarg, \"auto\") || !strcasecmp(optarg, \"tty\") ||\n !strcasecmp(optarg, \"if-tty\"))\n color_option = 2;\n else\n show_help = 1;\n } else\n color_option = 2; /* <--------------- default */\n /* the rest of this case statement (see below) */\n\nFirstly, note that GNU understands a wide set of option arguments:\n\n 1: always , yes , force\n 0: never , no , none\n 2: auto , tty , if-tty\n\nSecondly, note that the default mode is that which is selected by\nauto/tty/if-tty:\n\n color_option = 2;\n\nHowever, there's a little code left that specially processes this\n'auto' mode to transform it into either 'always' or 'never':\n\n if(color_option == 2) {\n if(isatty(STDOUT_FILENO) && getenv(\"TERM\") &&\n strcmp(getenv(\"TERM\"), \"dumb\"))\n color_option = 1;\n else\n color_option = 0;\n }\n break;\n\nThus, if stdout is attached to a tty that understands color, then\ncolorization is turned on; otherwise, colorization is turned off.\n\nIn my opinion, Git grep should follow GNU grep's conventions, not\nonly to be consistent, but also because they are better.\n\n>> Of course, I bet you find colorizing the filenames a nuisance because\n>> you don't care to pipe the relevant escape sequences to other\n>> commands.\n>\n> I'm not quite sure what you mean here, but my reason has nothing to do\n> with piping. If the output is entirely in a single color, I would\n> prefer that color to be my terminal's default. The color adds no\n> value.\n\nUnfortunately, Git grep interprets a plain `--color' the same way\nthat GNU grep interprets `--color=always', so that the color\nescape sequences get piped to everything.\n\n $ cd $clean_repo_for_git_source\n $ grep ':-)' -R . --exclude-dir=.git --color | cut -c 3- > smilies-gnu\n $ git grep --color ':-)' > smilies-git\n\n $ ls -l smilies* # Note the size difference\n -rw-r--r-- 1 mfwitten mfwitten 813 Mar 1 05:43 smilies-git\n -rw-r--r-- 1 mfwitten mfwitten 717 Mar 1 05:43 smilies-gnu\n \n $ cat -t smilies-gnu\n Documentation/glossary-content.txt:^IThe list you get with \"ls\" :-)\n Documentation/technical/pack-heuristics.txt: have to build up a certain level of gumption first :-)\n Documentation/technical/pack-heuristics.txt: even realize how much I wasn't realizing :-)\n Documentation/technical/pack-heuristics.txt: the cases where you might have to wander, don't do that :-)\n Documentation/technical/pack-heuristics.txt: I'm getting lost in all these orders, let me re-read :-)\n Documentation/technical/pack-heuristics.txt: can just read what you said there :-)\n Documentation/technical/pack-heuristics.txt: :-)\n Documentation/technical/pack-heuristics.txt: details on git packs :-)\n \n $ cat -t smilies-git\n Documentation/glossary-content.txt:^IThe list you get with \"ls\" ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: have to build up a certain level of gumption first ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: even realize how much I wasn't realizing ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: the cases where you might have to wander, don't do that ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: I'm getting lost in all these orders, let me re-read ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: can just read what you said there ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: ^[[31m^[[1m:-)^[[m\n Documentation/technical/pack-heuristics.txt: details on git packs ^[[31m^[[1m:-)^[[m\n\nIf you just run a plain `cat smilies-git', your terminal should\nstill render the colorization, as the escape sequences are\npreserved. This is generally a nuisance because normally it\nis desirable to lose that information when piping it to places\nother than the screen.\n\n> If the output is entirely in a single color, I would prefer that\n> color to be my terminal's default. The color adds no value.\n\nI would prefer whatever color to which I've become accustomed; the\ncolor is also a quick indication of what you're viewing. I usually\nkeep altering the search pattern until I get the right set of\nmatches and THEN issue `--name-only' to get just the paths; when\nI do that, I expect the output to change just by cutting out the\nstuff to the right of the paths, but your suggestion would ALSO\ncause the color to change. In my opinion, that's jarring.\n\nMoreover, with a plain `--color' being interpreted as `--color=auto',\nI would also have the benefit of piping the output anywhere else and\nnever having to bother with `--no-color' or `--color=never' or just\nremoving `--color'.\n\nIn short, I think the GNU strategy is the most intuitive and\nstreamlined approach.\n\n>> As a compromise (and perhaps as an improvement), perhaps\n>> only the basename of the filename should be colorized when\n>> --name-only is used; that way, colorization is still being used\n>> to differentiate different data, and the rest of the path is\n>> usually not that interesting anyway. However, for consistency,\n>> I would still think it wise to colorize the dirname portion\n>> with `color.grep.filename', but color the basename portion with\n>> `color.grep.match' (as though the basename portion is the text\n>> being matched).\n>\n> Personally, I am not a fan of this, but if it is implemented, it\n> should be an option, and should be turned off by default. Instead\n> of highlighting the name, it may be better to simply highlight the\n> slashes so the reader can more easily parse the path. But still, I\n> don't think this is worth the trouble.\n\nThe basenames of paths are almost always the most important piece of\ndata. Still, my suggestion is to leave the whole path colorized as\nusual when colorization is active.\n\nSincerely,\nMichael Witten\n"},{"id":"136059","messageId":"ca433831003022026pbc172d6ocb5ff2aefe29f462@mail.gmail.com","threadId":"22841","inReplyTo":"4b8cb38b.870fcc0a.7ebc.1a83@mx.google.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-03T04:26:36Z","receivedAt":"2010-03-03T04:26:36Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Tue, Mar 2, 2010 at 1:43 AM, Michael Witten wrote:\n>> By the way, the default should be 'always', not\n>> 'auto', to be consistent with GNU tools, and to be backwards\n>> compatible with the old --color behavior.\n>\n> Well, I've got:\n>\n>    GNU grep 2.5.4\n>\n> and the default for a plain `--color' seems to be `auto', whereby\n> colorization is turned on when stdout is attached to a tty capable\n> of color, but turned off otherwise:\n\nSorry, that was my mistake about GNU grep. However, GNU ls (coreutils\n7.4) defaults to 'always'. So, GNU tools are not consistent in this\nregard. Furthermore, the current behavior of all git tools is to make\n--color turn on color always, so I imagine you would have to make an\nextremely compelling argument to break backwards compatibility. I'll\nadd that this behavior makes the most sense, since most folks who use\ncolor have done `git config color.ui auto'. This is why no one have\ncreated this patch until now. The [=] part is nice, but the git\nconfig infrastructure usually obviates the need for --color=auto.\n\n> Firstly, note that GNU understands a wide set of option arguments:\n>\n>    1: always , yes , force\n>    0: never  , no  , none\n>    2: auto   , tty , if-tty\n\nI guess this is okay, but I don't see a need for it. If we allow\nthese other synonyms, then we'll have to support them forever. I say\njust stick with always/never/auto for now, and we could add the others\nlater if there's a big demand.\n\n> In my opinion, Git grep should follow GNU grep's conventions, not\n> only to be consistent, but also because they are better.\n\nIt is more important to be consistent with the other git tools, so\nthat is why --color is a synonym for --color=always.\n"},{"id":"136060","messageId":"buod3zmj9go.fsf@dhlpc061.dev.necel.com","threadId":"22841","inReplyTo":"ca433831003022026pbc172d6ocb5ff2aefe29f462@mail.gmail.com","subject":"Re: [PATCH 4/5] grep: Colorize filename, line number, and separator","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-03-03T04:49:27Z","receivedAt":"2010-03-03T04:49:27Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Mark Lodato writes:\n> Furthermore, the current behavior of all git tools is to make\n> --color turn on color always, so I imagine you would have to make an\n> extremely compelling argument to break backwards compatibility.\n\nI don't think the usual backwards-compatibility arguments should really\napply to edge-cases in frippery like --color...\n\n-Miles\n\n-- \nGenerous, adj. Originally this word meant noble by birth and was rightly\napplied to a great multitude of persons. It now means noble by nature and is\ntaking a bit of a rest.\n"}]}