{"thread":{"id":"13649","subject":"[PATCH] git-grep: add --color to highlight matches","startedAt":"2008-05-24T04:31:19Z","lastAt":"2008-05-26T11:29:38Z","messageCount":10,"participants":["Nguyễn Thái Ngọc Duy","Jakub Narebski","Nguyen Thai Ngoc Duy","Andreas Ericsson","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77636","messageId":"20080524043118.GA23118@laptop","threadId":"13649","inReplyTo":null,"subject":"[PATCH] git-grep: add --color to highlight matches","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-05-24T04:31:19Z","receivedAt":"2008-05-24T04:31:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-grep.txt | 4 +++\n builtin-grep.c | 6 +++-\n grep.c | 62 +++++++++++++++++++++++++++++++++++++++++---\n grep.h | 1 +\n 4 files changed, 68 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex a97f055..20ef098 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -16,6 +16,7 @@ SYNOPSIS\n \t [-F | --fixed-strings] [-n]\n \t [-l | --files-with-matches] [-L | --files-without-match]\n \t [-c | --count] [--all-match]\n+\t [--color]\n \t [-A ] [-B ] [-C ]\n \t [-f ] [-e] \n \t [--and|--or|--not|(|)|-e ...] [...]\n@@ -85,6 +86,9 @@ OPTIONS\n \tInstead of showing every matched line, show the number of\n \tlines that match.\n \n+--color::\n+\tShow colored matches.\n+\n -[ABC] ::\n \tShow `context` trailing (`A` -- after), or leading (`B`\n \t-- before), or both (`C` -- context) lines, and place a\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex ef29910..118ec32 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -386,7 +386,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \t * we grep through the checked-out files. It tends to\n \t * be a lot more optimized\n \t */\n-\tif (!cached) {\n+\tif (!cached && !opt->color) {\n \t\thit = external_grep(opt, paths, cached);\n \t\tif (hit >= 0)\n \t\t\treturn hit;\n@@ -708,6 +708,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\topt.relative = 0;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--color\", arg)) {\n+\t\t\topt.color = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(\"--\", arg)) {\n \t\t\t/* later processing wants to have this at argv[1] */\n \t\t\targv--;\ndiff --git a/grep.c b/grep.c\nindex f67d671..7cc05b0 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -247,7 +247,10 @@ static int fixmatch(const char *pattern, char *line, regmatch_t *match)\n \t}\n }\n \n-static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol, enum grep_context ctx)\n+static int match_one_pattern_1(struct grep_opt *opt, struct grep_pat *p,\n+\t\t\t char *bol, char *eol,\n+\t\t\t enum grep_context ctx, int eflags,\n+\t\t\t int *rm_so, int *rm_eo)\n {\n \tint hit = 0;\n \tint at_true_bol = 1;\n@@ -261,7 +264,7 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol\n \tif (!opt->fixed) {\n \t\tregex_t *exp = &p->regexp;\n \t\thit = !regexec(exp, bol, ARRAY_SIZE(pmatch),\n-\t\t\t pmatch, 0);\n+\t\t\t pmatch, eflags);\n \t}\n \telse {\n \t\thit = !fixmatch(p->pattern, bol, pmatch);\n@@ -298,9 +301,20 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol\n \t\t\tgoto again;\n \t\t}\n \t}\n+\tif (hit) {\n+\t\tif (rm_so)\n+\t\t\t*rm_so = pmatch[0].rm_so;\n+\t\tif (rm_eo)\n+\t\t\t*rm_eo = pmatch[0].rm_eo;\n+\t}\n \treturn hit;\n }\n \n+static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol, enum grep_context ctx)\n+{\n+\treturn match_one_pattern_1(opt, p, bol, eol, ctx, 0, NULL, NULL);\n+}\n+\n static int match_expr_eval(struct grep_opt *o,\n \t\t\t struct grep_expr *x,\n \t\t\t char *bol, char *eol,\n@@ -365,6 +379,42 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \treturn 0;\n }\n \n+static void show_line_colored(struct grep_opt *opt, char *bol, char *eol,\n+\t\t\t const char *name, unsigned lno, char sign)\n+{\n+\tstruct grep_pat *p;\n+\tint rm_so, rm_eo, eflags = 0;\n+\tint ch;\n+\n+\tif (opt->pathname)\n+\t\tprintf(\"%s%c\", name, sign);\n+\tif (opt->linenum)\n+\t\tprintf(\"%d%c\", lno, sign);\n+\n+\tch = *eol;\n+\t*eol = 0;\n+\twhile (bol < eol) {\n+\t\tfor (p = opt->pattern_list; p; p = p->next) {\n+\t\t\tif (match_one_pattern_1(opt, p, bol, eol, GREP_CONTEXT_BODY, eflags, &rm_so, &rm_eo))\n+\t\t\t\tbreak;\n+\t\t}\n+\n+\t\t/* No match, break the loop */\n+\t\tif (!p ||\n+\t\t (rm_so < 0) || (eol - bol) <= rm_so ||\n+\t\t (rm_eo < 0) || (eol - bol) < rm_eo)\n+\t\t\tbreak;\n+\n+\t\tprintf(\"%.*s\\033[31m\\033[1m%.*s\\033[m\",\n+\t\t rm_so, bol,\n+\t\t rm_eo-rm_so, bol+rm_so);\n+\t\tbol += rm_eo;\n+\t\teflags = REG_NOTBOL;\n+\t}\n+\tprintf(\"%s\\n\", bol);\n+\t*eol = ch;\n+}\n+\n static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t\t\t char *buf, unsigned long size, int collect_hits)\n {\n@@ -466,8 +516,12 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t\t\t}\n \t\t\tif (last_shown && lno != last_shown + 1)\n \t\t\t\tprintf(hunk_mark);\n-\t\t\tif (!opt->count)\n-\t\t\t\tshow_line(opt, bol, eol, name, lno, ':');\n+\t\t\tif (!opt->count) {\n+\t\t\t\tif (opt->color)\n+\t\t\t\t\tshow_line_colored(opt, bol, eol, name, lno, ':');\n+\t\t\t\telse\n+\t\t\t\t\tshow_line(opt, bol, eol, name, lno, ':');\n+\t\t\t}\n \t\t\tlast_shown = last_hit = lno;\n \t\t}\n \t\telse if (last_hit &&\ndiff --git a/grep.h b/grep.h\nindex d252dd2..979f7d0 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -68,6 +68,7 @@ struct grep_opt {\n \tunsigned extended:1;\n \tunsigned relative:1;\n \tunsigned pathname:1;\n+\tunsigned color:1;\n \tint regflags;\n \tunsigned pre_context;\n \tunsigned post_context;\n-- \n1.5.5.GIT\n"},{"id":"77645","messageId":"m3ve142jkc.fsf@localhost.localdomain","threadId":"13649","inReplyTo":"20080524043118.GA23118@laptop","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-24T08:28:13Z","receivedAt":"2008-05-24T08:28:13Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> +--color::\n> +\tShow colored matches.\n> +\n\nWhy not borrow from GNU grep manpage?\n\n --colour[=WHEN], --color[=WHEN]\n Surround the matching string with the marker find in GREP_COLOR\n environment variable. WHEN may be 'never', 'always', or 'auto'\n\nWe probably would also want it configurable (via config variables),\nas `color.grep' I guess.\n\nI think compatibility with grep (using GREP_COLOR) would be good idea,\nand easily implemented.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"77648","messageId":"fcaeb9bf0805240220x78943f8ap67344a508531faa0@mail.gmail.com","threadId":"13649","inReplyTo":"m3ve142jkc.fsf@localhost.localdomain","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-05-24T09:20:39Z","receivedAt":"2008-05-24T09:20:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> +--color::\n>> + Show colored matches.\n>> +\n>\n> Why not borrow from GNU grep manpage?\n>\n> --colour[=WHEN], --color[=WHEN]\n> Surround the matching string with the marker find in GREP_COLOR\n> environment variable. WHEN may be 'never', 'always', or 'auto'\n>\n> We probably would also want it configurable (via config variables),\n> as `color.grep' I guess.\n\nYes. But this patch was really to scratch my itch and I felt good\nafter having it colorized so did not bother with configuration system.\nI will probably make it a config var when I get tired of typing\n--color too often ;)\n\n> I think compatibility with grep (using GREP_COLOR) would be good idea,\n> and easily implemented.\n\nI had no idea that variable existed.\n\n> --\n> Jakub Narebski\n> Poland\n> ShadeHawk on #git\n>\n\n\n\n-- \nDuy\n"},{"id":"77650","messageId":"200805241259.13132.jnareb@gmail.com","threadId":"13649","inReplyTo":"fcaeb9bf0805240220x78943f8ap67344a508531faa0@mail.gmail.com","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-24T10:59:12Z","receivedAt":"2008-05-24T10:59:12Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 24 May 2008, Nguyen Thai Ngoc Duy wrote:\n> On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski wrote:\n>> Nguyen Thai Ngoc Duy writes:\n>>\n>>> +--color::\n>>> + Show colored matches.\n>>> +\n>>\n>> Why not borrow from GNU grep manpage?\n>>\n>> --colour[=WHEN], --color[=WHEN]\n>> Surround the matching string with the marker find in GREP_COLOR\n>> environment variable. WHEN may be 'never', 'always', or 'auto'\n>>\n>> We probably would also want it configurable (via config variables),\n>> as `color.grep' I guess.\n\nAnd color.grep., where slot can be 'match', and probably later also\n'context' (I don't remember if git-grep implements -C option...).\n \n> Yes. But this patch was really to scratch my itch and I felt good\n> after having it colorized so did not bother with configuration system.\n> I will probably make it a config var when I get tired of typing\n> --color too often ;)\n\nThanks for sharing, although I guess Junio would prefer more complete\npatch...\n\n>> I think compatibility with grep (using GREP_COLOR) would be good idea,\n>> and easily implemented.\n> \n> I had no idea that variable existed.\n\nWell, I have though that git always use external grep, so this would\nbe for free. After reading the patch more carefully it seems like it\nisn't so.\n\nStill, perhaps that would be a good idea, but I guess for later patch.\n-- \nJakub Narebski\nPoland\n"},{"id":"77751","messageId":"483A7B1F.1050608@op5.se","threadId":"13649","inReplyTo":"200805241259.13132.jnareb@gmail.com","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-05-26T08:55:59Z","receivedAt":"2008-05-26T08:55:59Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Jakub Narebski wrote:\n> On Sat, 24 May 2008, Nguyen Thai Ngoc Duy wrote:\n>> On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski wrote:\n>>> Nguyen Thai Ngoc Duy writes:\n> \n>>> I think compatibility with grep (using GREP_COLOR) would be good idea,\n>>> and easily implemented.\n>> I had no idea that variable existed.\n> \n> Well, I have though that git always use external grep, so this would\n> be for free. After reading the patch more carefully it seems like it\n> isn't so.\n> \n> Still, perhaps that would be a good idea, but I guess for later patch.\n\nI can't help but think that colorization for external-grep systems only\nwould still be worthwhile, as the majority of git users are on such\nsystems. It'd still be a separate implementation from adding it to the\ninternal grep functionality anyways, so it's not as if work would be\nlost by going half the way here.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"77756","messageId":"alpine.DEB.1.00.0805261113050.30431@racer","threadId":"13649","inReplyTo":"483A7B1F.1050608@op5.se","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-26T10:16:05Z","receivedAt":"2008-05-26T10:16:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 May 2008, Andreas Ericsson wrote:\n\n> I can't help but think that colorization for external-grep systems only \n> would still be worthwhile, as the majority of git users are on such \n> systems. It'd still be a separate implementation from adding it to the \n> internal grep functionality anyways, so it's not as if work would be \n> lost by going half the way here.\n\nHuh? I thought that the external grep is only a matter of fallback for \nperformance reasons (if the files are already checked out), or if the user \nspecifies the external grep explicitely.\n\nAt least for the platform we bend over most, our beloved anachronistic M$ \nWindows, I do not recall forcing external grep.\n\nBesides, it would be a kludge at best to work _twice_ to find out the \nsearch terms, once in the external grep, and a second time in the coloring \ncode. So I think it should not be done.\n\nCiao,\nDscho\n"},{"id":"77760","messageId":"fcaeb9bf0805260357t2bce7753vab5331bb21d98e4b@mail.gmail.com","threadId":"13649","inReplyTo":"alpine.DEB.1.00.0805261113050.30431@racer","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-05-26T10:57:08Z","receivedAt":"2008-05-26T10:57:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin\n wrote:\n> Besides, it would be a kludge at best to work _twice_ to find out the\n> search terms, once in the external grep, and a second time in the coloring\n> code. So I think it should not be done.\n\nI think if it's GNU grep, just passing it --color, it will grep and\ncolorize search terms in one turn.\n-- \nDuy\n"},{"id":"77761","messageId":"alpine.DEB.1.00.0805261200330.30431@racer","threadId":"13649","inReplyTo":"fcaeb9bf0805260357t2bce7753vab5331bb21d98e4b@mail.gmail.com","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-26T11:00:47Z","receivedAt":"2008-05-26T11:00:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin\n> wrote:\n> > Besides, it would be a kludge at best to work _twice_ to find out the\n> > search terms, once in the external grep, and a second time in the coloring\n> > code. So I think it should not be done.\n> \n> I think if it's GNU grep, just passing it --color, it will grep and\n> colorize search terms in one turn.\n\nAnd what tells you that the called grep is GNU grep?\n\nCiao,\nDscho\n"},{"id":"77762","messageId":"fcaeb9bf0805260407r6a1bea23v3861d3c43df01939@mail.gmail.com","threadId":"13649","inReplyTo":"alpine.DEB.1.00.0805261200330.30431@racer","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-05-26T11:07:37Z","receivedAt":"2008-05-26T11:07:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, May 26, 2008 at 6:00 PM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:\n>\n>> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin\n>> wrote:\n>> > Besides, it would be a kludge at best to work _twice_ to find out the\n>> > search terms, once in the external grep, and a second time in the coloring\n>> > code. So I think it should not be done.\n>>\n>> I think if it's GNU grep, just passing it --color, it will grep and\n>> colorize search terms in one turn.\n>\n> And what tells you that the called grep is GNU grep?\n\nA newly added macro like HAS_GNU_GREP? Granted it won't work all the\ntime. The user who set the macro should know what he is doing. This\napproach is IMHO fine as long as we don't allow color customization.\n-- \nDuy\n"},{"id":"77765","messageId":"alpine.DEB.1.00.0805261227440.30431@racer","threadId":"13649","inReplyTo":"fcaeb9bf0805260407r6a1bea23v3861d3c43df01939@mail.gmail.com","subject":"Re: [PATCH] git-grep: add --color to highlight matches","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-26T11:29:38Z","receivedAt":"2008-05-26T11:29:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On Mon, May 26, 2008 at 6:00 PM, Johannes Schindelin\n> wrote:\n>\n> > On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:\n> >\n> >> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin\n> >> wrote:\n> >> > Besides, it would be a kludge at best to work _twice_ to find out \n> >> > the search terms, once in the external grep, and a second time in \n> >> > the coloring code. So I think it should not be done.\n> >>\n> >> I think if it's GNU grep, just passing it --color, it will grep and \n> >> colorize search terms in one turn.\n> >\n> > And what tells you that the called grep is GNU grep?\n> \n> A newly added macro like HAS_GNU_GREP? Granted it won't work all the\n> time. The user who set the macro should know what he is doing. This\n> approach is IMHO fine as long as we don't allow color customization.\n\nIMO this is just a kludge that is not even necessary, because the whole \nnotion of relying on an external grep's colorization is crap.\n\nThe OP has a patch that adds colorization to Git's internal grep code. \nThe user specifies \"--color\" with grep? Fine, then it's no external grep. \nFull stop.\n\nJust like the original patch implements it. Nice, simple, and easy. What \nis so wrong with that?\n\nCiao,\nDscho\n"}]}