{"thread":{"id":"22944","subject":"[PATCHv2 1/3] Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*","startedAt":"2010-03-07T16:52:44Z","lastAt":"2010-03-07T20:47:00Z","messageCount":6,"participants":["Mark Lodato","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"136343","messageId":"1267980767-12617-1-git-send-email-lodatom@gmail.com","threadId":"22944","inReplyTo":null,"subject":"[PATCHv2 0/3] grep color enhancements","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-07T16:52:44Z","receivedAt":"2010-03-07T16:52:44Z","isPatch":false,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"This is a re-spin of the second half of Gmane 141186,  \"color\nenhancements, particularly for grep\", which is currently in pu as\n'ml/color-grep'.  The following changes were made, all to the second\npatch:\n\n* Fix the parsing of color.grep.  The old code caused a segfault if this\n  option was set, and it also disallowed setting this option without\n  a value.\n\n* Color the filename in --name-only output.  I still don't like this,\n  but I understand that makes the coloring more consistent.  As Junio\n  said, if there's no strong argument one way or the other, it's\n  probably best to follow GNU's lead.\n\n* Style changes: Remove the cast to int for \"rm_eo - rm_so\", put else\n  on same line as curly brace, and remove an unnecessary variable\n  assignment.\n\n* Asciidoc fixes: Use \";;\" instead of \":::\" for nested lists, and\n  un-indent the last sentence for color.grep.<slot>.\n\nMark Lodato (3):\n  Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*\n  grep: Colorize filename, line number, and separator\n  grep: Colorize selected, context, and function lines\n\n Documentation/config.txt |   26 ++++++++++++++++--\n builtin-grep.c           |   38 +++++++++++++++++++++-----\n color.h                  |   11 +++++++\n graph.c                  |   12 ++++----\n grep.c                   |   66 ++++++++++++++++++++++++++++++----------------\n grep.h                   |    6 ++++\n 6 files changed, 119 insertions(+), 40 deletions(-)\n"},{"id":"136341","messageId":"1267980767-12617-2-git-send-email-lodatom@gmail.com","threadId":"22944","inReplyTo":"1267980767-12617-1-git-send-email-lodatom@gmail.com","subject":"[PATCHv2 1/3] Add GIT_COLOR_BOLD_* and GIT_COLOR_BG_*","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-07T16:52:45Z","receivedAt":"2010-03-07T16:52:45Z","isPatch":false,"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 <lodatom@gmail.com>\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 2697957..2a876c4 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -847,7 +847,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":"136342","messageId":"1267980767-12617-3-git-send-email-lodatom@gmail.com","threadId":"22944","inReplyTo":"1267980767-12617-1-git-send-email-lodatom@gmail.com","subject":"[PATCHv2 2/3] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-07T16:52:46Z","receivedAt":"2010-03-07T16:52:46Z","isPatch":false,"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.<slot>.\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 is one difference from GNU grep: When a binary file matches\nwithout -a, GNU grep does not color the <file> in \"Binary file <file>\nmatches\", but we do.\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\nRemove the cast to int for `rm_eo - rm_so` since it is not necessary.\n\nSigned-off-by: Mark Lodato <lodatom@gmail.com>\n---\n Documentation/config.txt |   20 ++++++++++++++--\n builtin-grep.c           |   27 ++++++++++++++++------\n grep.c                   |   55 ++++++++++++++++++++++++++++-----------------\n grep.h                   |    3 ++\n 4 files changed, 74 insertions(+), 31 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2dc3a05..711519e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -678,9 +678,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.<slot>.\n+color.grep.<slot>::\n+\tUse customized color for grep colorization.  `<slot>` 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++\n+The values of these variables may be specified as in color.branch.<slot>.\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 2a876c4..0c8115d 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -288,6 +288,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@@ -295,17 +296,26 @@ 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+\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 (color) {\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\tcolor_parse(value, var, color);\n+\t\tif (!strcmp(color, GIT_COLOR_RESET))\n+\t\t\tcolor[0] = '\\0';\n \t}\n-\treturn git_color_default_config(var, value, cb);\n+\treturn 0;\n }\n \n /*\n@@ -847,7 +857,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..3a03060 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -270,9 +270,28 @@ static int word_char(char ch)\n \treturn isalnum(ch) || ch == '_';\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} else\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\topt->output(opt, \"\\0\", 1);\n+\telse\n+\t\toutput_color(opt, &sign, 1, opt->color_sep);\n+}\n+\n static void show_name(struct grep_opt *opt, const char *name)\n {\n-\topt->output(opt, name, strlen(name));\n+\toutput_color(opt, name, strlen(name), opt->color_filename);\n \topt->output(opt, opt->null_following_name ? \"\\0\" : \"\\n\", 1);\n }\n \n@@ -510,31 +529,27 @@ 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 +563,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     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 +835,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 +895,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":"136344","messageId":"1267980767-12617-4-git-send-email-lodatom@gmail.com","threadId":"22944","inReplyTo":"1267980767-12617-1-git-send-email-lodatom@gmail.com","subject":"[PATCHv2 3/3] grep: Colorize selected, context, and function lines","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-07T16:52:47Z","receivedAt":"2010-03-07T16:52:47Z","isPatch":false,"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.<slot>.  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 <lodatom@gmail.com>\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 711519e..38bc729 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -683,12 +683,18 @@ color.grep.<slot>::\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 0c8115d..e423eac 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -298,12 +298,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@@ -857,9 +863,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 3a03060..1e0b1e6 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -529,6 +529,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@@ -557,12 +558,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     match.rm_eo - match.rm_so,\n \t\t\t\t     opt->color_match);\n@@ -572,7 +579,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":"136348","messageId":"7v1vfwrm51.fsf@alter.siamese.dyndns.org","threadId":"22944","inReplyTo":"1267980767-12617-3-git-send-email-lodatom@gmail.com","subject":"Re: [PATCHv2 2/3] grep: Colorize filename, line number, and separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-07T19:02:02Z","receivedAt":"2010-03-07T19:02:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Lodato <lodatom@gmail.com> writes:\n\n> +\tif (color) {\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\tcolor_parse(value, var, color);\n> +\t\tif (!strcmp(color, GIT_COLOR_RESET))\n> +\t\t\tcolor[0] = '\\0';\n\nI don't know this \"optimization\" is warranted.  I can understand that you\nare trying to help the user to save a handful of useless bytes per line of\noutput in the normal case, but doesn't \"color.cmd.foo = normal\" set the\nvalue to an empty string already if that is what the user wants?\n\nThe user may know about his contents better than this code does; perhaps\nthe payload sometimes has funny ANSI sequence in it and the user may be\ntrying to explicitly reset at the beginning of the line for the \"plain\"\npart of the output by saying \"reset\".  Wouldn't this make it impossible to\ndo so?\n"},{"id":"136354","messageId":"ca433831003071247j489177ceke9c3394fdc6d2372@mail.gmail.com","threadId":"22944","inReplyTo":"7v1vfwrm51.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 2/3] grep: Colorize filename, line number, and separator","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-03-07T20:47:00Z","receivedAt":"2010-03-07T20:47:00Z","isPatch":false,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sun, Mar 7, 2010 at 2:02 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mark Lodato <lodatom@gmail.com> writes:\n>\n>> +     if (color) {\n>>               if (!value)\n>>                       return config_error_nonbool(var);\n>> -             color_parse(value, var, opt->color_match);\n>> -             return 0;\n>> +             color_parse(value, var, color);\n>> +             if (!strcmp(color, GIT_COLOR_RESET))\n>> +                     color[0] = '\\0';\n>\n> I don't know this \"optimization\" is warranted.  I can understand that you\n> are trying to help the user to save a handful of useless bytes per line of\n> output in the normal case, but doesn't \"color.cmd.foo = normal\" set the\n> value to an empty string already if that is what the user wants?\n\nAh, I didn't know about the \"normal\" color.  I tried using a blank\nvalue, which defaulted to \"reset\", so I thought there was no way to\nsay \"no ANSI sequence\".  The above was my workaround to get no\nsequence.  Since the \"normal\" color exists, this workaround should be\nremoved.\n\nThe squash-able patch below removes the workaround, and also fixes\nanother bug: color was not reset at the end of the line for hunk\nseparators (\"--\").\n\nAs an aside, perhaps a blank color value should default to no ANSI\nsequence, rather than the reset sequence?\n\n\ndiff --git i/builtin-grep.c w/builtin-grep.c\nindex e423eac..c519fcf 100644\n--- i/builtin-grep.c\n+++ w/builtin-grep.c\n@@ -318,8 +318,6 @@\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tcolor_parse(value, var, color);\n-\t\tif (!strcmp(color, GIT_COLOR_RESET))\n-\t\t\tcolor[0] = '\\0';\n \t}\n \treturn 0;\n }\ndiff --git i/grep.c w/grep.c\nindex 1e0b1e6..b641305 100644\n--- i/grep.c\n+++ w/grep.c\n@@ -533,12 +533,15 @@\n\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\toutput_color(opt, \"--\\n\", 3, opt->color_sep);\n-\t\t\telse\n+\t\t\tif (opt->show_hunk_mark) {\n+\t\t\t\toutput_color(opt, \"--\", 2, opt->color_sep);\n+\t\t\t\topt->output(opt, \"\\n\", 1);\n+\t\t\t} else\n \t\t\t\topt->show_hunk_mark = 1;\n-\t\t} else if (lno > opt->last_shown + 1)\n-\t\t\toutput_color(opt, \"--\\n\", 3, opt->color_sep);\n+\t\t} else if (lno > opt->last_shown + 1) {\n+\t\t\toutput_color(opt, \"--\", 2, opt->color_sep);\n+\t\t\topt->output(opt, \"\\n\", 1);\n+\t\t}\n \t}\n \topt->last_shown = lno;\n"}]}