{"thread":{"id":"56660","subject":"[PATCH v11 1/3] grep: refactor next_match() and match_one_pattern() for external use","startedAt":"2021-10-07T20:31:55Z","lastAt":"2021-10-09T15:52:09Z","messageCount":6,"participants":["Hamza Mahfooz","Junio C Hamano"],"isPatch":true,"patchVersion":11,"patchTotal":3},"messages":[{"id":"438222","messageId":"20211007203148.23888-1-someguy@effective-light.com","threadId":"56660","inReplyTo":null,"subject":"[PATCH v11 1/3] grep: refactor next_match() and match_one_pattern() for external use","fromName":"Hamza Mahfooz","fromEmail":"someguy@effective-light.com","sentAt":"2021-10-07T20:31:46Z","receivedAt":"2021-10-07T20:31:55Z","isPatch":true,"sender":{"key":"someguy@effective-light.com","avatar":"https://avatars.githubusercontent.com/u/3885502?v=4"},"body":"These changes are made in preparation of, the colorization support for the\n\"git log\" subcommands that, rely on regex functionality (i.e. \"--author\",\n\"--committer\" and \"--grep\"). These changes are necessary primarily because\nmatch_one_pattern() expects header lines to be prefixed, however, in\npretty, the prefixes are stripped from the lines because the name-email\npairs need to go through additional parsing, before they can be printed and\nbecause next_match() doesn't handle the case of\n\"ctx == GREP_CONTEXT_HEAD\" at all. So, teach next_match() how to handle the\nnew case and move match_one_pattern()'s core logic to\nheaderless_match_one_pattern() while preserving match_one_pattern()'s uses\nthat depend on the additional processing.\n\nSigned-off-by: Hamza Mahfooz <someguy@effective-light.com>\n---\nv5: separate grep changes from pretty changes.\n\nv6: rescope some variables.\n\nv7: export header_field[] and allow for subsequent matches on header lines\n    in match_one_pattern().\n\nv8: add headerless_match_one_pattern() and move header_field[] back.\n\nv9: get rid of the new check in headerless_match_one_pattern(), move the\n    header pattern filtering logic in grep_next_match() and document\n    grep_next_match() in grep.h.\n\nv10: add a \"magic\" comment in grep_next_match() to signify a fall through\n     in the switch statement.\n---\n grep.c | 79 ++++++++++++++++++++++++++++++++++++----------------------\n grep.h |  9 +++++++\n 2 files changed, 58 insertions(+), 30 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 14fe8a0fd2..fe847a0111 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -944,10 +944,10 @@ static struct {\n \t{ \"reflog \", 7 },\n };\n \n-static int match_one_pattern(struct grep_pat *p,\n-\t\t\t     const char *bol, const char *eol,\n-\t\t\t     enum grep_context ctx,\n-\t\t\t     regmatch_t *pmatch, int eflags)\n+static int headerless_match_one_pattern(struct grep_pat *p,\n+\t\t\t\t\tconst char *bol, const char *eol,\n+\t\t\t\t\tenum grep_context ctx,\n+\t\t\t\t\tregmatch_t *pmatch, int eflags)\n {\n \tint hit = 0;\n \tconst char *start = bol;\n@@ -956,25 +956,6 @@ static int match_one_pattern(struct grep_pat *p,\n \t    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))\n \t\treturn 0;\n \n-\tif (p->token == GREP_PATTERN_HEAD) {\n-\t\tconst char *field;\n-\t\tsize_t len;\n-\t\tassert(p->field < ARRAY_SIZE(header_field));\n-\t\tfield = header_field[p->field].field;\n-\t\tlen = header_field[p->field].len;\n-\t\tif (strncmp(bol, field, len))\n-\t\t\treturn 0;\n-\t\tbol += len;\n-\t\tswitch (p->field) {\n-\t\tcase GREP_HEADER_AUTHOR:\n-\t\tcase GREP_HEADER_COMMITTER:\n-\t\t\tstrip_timestamp(bol, &eol);\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n  again:\n \thit = patmatch(p, bol, eol, pmatch, eflags);\n \n@@ -1025,6 +1006,36 @@ static int match_one_pattern(struct grep_pat *p,\n \treturn hit;\n }\n \n+static int match_one_pattern(struct grep_pat *p,\n+\t\t\t     const char *bol, const char *eol,\n+\t\t\t     enum grep_context ctx, regmatch_t *pmatch,\n+\t\t\t     int eflags)\n+{\n+\tconst char *field;\n+\tsize_t len;\n+\n+\tif (p->token == GREP_PATTERN_HEAD) {\n+\t\tassert(p->field < ARRAY_SIZE(header_field));\n+\t\tfield = header_field[p->field].field;\n+\t\tlen = header_field[p->field].len;\n+\t\tif (strncmp(bol, field, len))\n+\t\t\treturn 0;\n+\t\tbol += len;\n+\n+\t\tswitch (p->field) {\n+\t\tcase GREP_HEADER_AUTHOR:\n+\t\tcase GREP_HEADER_COMMITTER:\n+\t\t\tstrip_timestamp(bol, &eol);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\treturn headerless_match_one_pattern(p, bol, eol, ctx, pmatch, eflags);\n+}\n+\n+\n static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x,\n \t\t\t   const char *bol, const char *eol,\n \t\t\t   enum grep_context ctx, ssize_t *col,\n@@ -1143,7 +1154,7 @@ static int match_next_pattern(struct grep_pat *p,\n {\n \tregmatch_t match;\n \n-\tif (!match_one_pattern(p, bol, eol, ctx, &match, eflags))\n+\tif (!headerless_match_one_pattern(p, bol, eol, ctx, &match, eflags))\n \t\treturn 0;\n \tif (match.rm_so < 0 || match.rm_eo < 0)\n \t\treturn 0;\n@@ -1158,19 +1169,26 @@ static int match_next_pattern(struct grep_pat *p,\n \treturn 1;\n }\n \n-static int next_match(struct grep_opt *opt,\n-\t\t      const char *bol, const char *eol,\n-\t\t      enum grep_context ctx, regmatch_t *pmatch, int eflags)\n+int grep_next_match(struct grep_opt *opt,\n+\t\t    const char *bol, const char *eol,\n+\t\t    enum grep_context ctx, regmatch_t *pmatch,\n+\t\t    enum grep_header_field field, int eflags)\n {\n \tstruct grep_pat *p;\n \tint hit = 0;\n \n \tpmatch->rm_so = pmatch->rm_eo = -1;\n \tif (bol < eol) {\n-\t\tfor (p = opt->pattern_list; p; p = p->next) {\n+\t\tfor (p = ((ctx == GREP_CONTEXT_HEAD)\n+\t\t\t   ? opt->header_list : opt->pattern_list);\n+\t\t\t  p; p = p->next) {\n \t\t\tswitch (p->token) {\n-\t\t\tcase GREP_PATTERN: /* atom */\n \t\t\tcase GREP_PATTERN_HEAD:\n+\t\t\t\tif ((field != GREP_HEADER_FIELD_MAX) &&\n+\t\t\t\t    (p->field != field))\n+\t\t\t\t\tcontinue;\n+\t\t\t\t/* fall thru */\n+\t\t\tcase GREP_PATTERN: /* atom */\n \t\t\tcase GREP_PATTERN_BODY:\n \t\t\t\thit |= match_next_pattern(p, bol, eol, ctx,\n \t\t\t\t\t\t\t  pmatch, eflags);\n@@ -1261,7 +1279,8 @@ static void show_line(struct grep_opt *opt,\n \t\t\telse if (sign == '=')\n \t\t\t\tline_color = opt->colors[GREP_COLOR_FUNCTION];\n \t\t}\n-\t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n+\t\twhile (grep_next_match(opt, bol, eol, ctx, &match,\n+\t\t\t\t       GREP_HEADER_FIELD_MAX, eflags)) {\n \t\t\tif (match.rm_so == match.rm_eo)\n \t\t\t\tbreak;\n \ndiff --git a/grep.h b/grep.h\nindex 3cb8a83ae8..808ad76f0c 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -191,6 +191,15 @@ void compile_grep_patterns(struct grep_opt *opt);\n void free_grep_patterns(struct grep_opt *opt);\n int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size);\n \n+/* The field parameter is only used to filter header patterns\n+ * (where appropriate). If filtering isn't desirable\n+ * GREP_HEADER_FIELD_MAX should be supplied.\n+ */\n+int grep_next_match(struct grep_opt *opt,\n+\t\t    const char *bol, const char *eol,\n+\t\t    enum grep_context ctx, regmatch_t *pmatch,\n+\t\t    enum grep_header_field field, int eflags);\n+\n struct grep_source {\n \tchar *name;\n \n-- \n2.33.0\n\n"},{"id":"438223","messageId":"20211007203148.23888-2-someguy@effective-light.com","threadId":"56660","inReplyTo":"20211007203148.23888-1-someguy@effective-light.com","subject":"[PATCH v11 2/3] pretty: colorize pattern matches in commit messages","fromName":"Hamza Mahfooz","fromEmail":"someguy@effective-light.com","sentAt":"2021-10-07T20:31:47Z","receivedAt":"2021-10-07T20:31:57Z","isPatch":true,"sender":{"key":"someguy@effective-light.com","avatar":"https://avatars.githubusercontent.com/u/3885502?v=4"},"body":"The \"git log\" command limits its output to the commits that contain strings\nmatched by a pattern when the \"--grep=<pattern>\" option is used, but unlike\noutput from \"git grep -e <pattern>\", the matches are not highlighted,\nmaking them harder to spot.\n\nTeach the pretty-printer code to highlight matches from the\n\"--grep=<pattern>\", \"--author=<pattern>\" and \"--committer=<pattern>\"\noptions (to view the last one, you may have to ask for --pretty=fuller).\n\nAlso, it must be noted that we are effectively greping the content twice\n(because it would be a hassle to rework the existing matching code to do\na /g match and then pass it all down to the coloring code), however it only\nslows down \"git log --author=^H\" on this repository by around 1-2%\n(compared to v2.33.0), so it should be a small enough slow down to justify\nthe addition of the feature.\n\nSigned-off-by: Hamza Mahfooz <someguy@effective-light.com>\n---\nv2: make the commit message whole (add the missing ingredients), rename\n    append_matched_line() to append_line_with_color(), use\n    colors[GREP_COLOR_MATCH_SELECTED] instead of\n    colors[GREP_COLOR_MATCH_CONTEXT], allow the background color to be\n    customized, don't copy strings to a buffer when not coloring in\n    append_line_with_color(), rename next_match() to grep_next_match(),\n    repurpose grep_next_match()/match_one_pattern() for use in\n    append_line_with_color() (allowing us to remove duplicated matching\n    code in append_line_with_color()), document how to customize the\n    feature and modify some of the tests to fit the feature better.\n\nv3: fix a formatting issue with the added documentation.\n\nv4: add strbuf_add_with_color(), use the correct color code scheme in the\n    unit tests and add more unit tests.\n\nv5: separate grep changes from pretty changes and add some performance\n    analysis in the commit message.\n\nv6: put the documentation in the correct place, cleanup pretty.c and\n    format the unit tests according to the current convention.\n\nv7: get rid of all manual strbuf management, constify where now appropriate\n    and fix the header line prefix issue properly.\n\nv8: remove code that relies on grep_header_fields[].\n\nv11: clarify why we are grepping twice.\n---\n Documentation/config/color.txt |   7 ++-\n pretty.c                       | 101 +++++++++++++++++++++++++++++----\n t/t4202-log.sh                 |  51 +++++++++++++++++\n 3 files changed, 145 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\nindex e05d520a86..91d9a9da32 100644\n--- a/Documentation/config/color.txt\n+++ b/Documentation/config/color.txt\n@@ -104,9 +104,12 @@ color.grep.<slot>::\n `matchContext`;;\n \tmatching text in context lines\n `matchSelected`;;\n-\tmatching text in selected lines\n+\tmatching text in selected lines. Also, used to customize the following\n+\tlinkgit:git-log[1] subcommands: `--grep`, `--author` and `--committer`.\n `selected`;;\n-\tnon-matching text in selected lines\n+\tnon-matching text in selected lines. Also, used to customize the\n+\tfollowing linkgit:git-log[1] subcommands: `--grep`, `--author` and\n+\t`--committer`.\n `separator`;;\n \tseparators between fields on a line (`:`, `-`, and `=`)\n \tand between hunks (`--`)\ndiff --git a/pretty.c b/pretty.c\nindex 73b5ead509..2dd94af886 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -431,6 +431,52 @@ const char *show_ident_date(const struct ident_split *ident,\n \treturn show_date(date, tz, mode);\n }\n \n+static inline void strbuf_add_with_color(struct strbuf *sb, const char *color,\n+\t\t\t\t\t const char *buf, size_t buflen)\n+{\n+\tstrbuf_addstr(sb, color);\n+\tstrbuf_add(sb, buf, buflen);\n+\tif (*color)\n+\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n+}\n+\n+static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,\n+\t\t\t\t   const char *line, size_t linelen,\n+\t\t\t\t   int color, enum grep_context ctx,\n+\t\t\t\t   enum grep_header_field field)\n+{\n+\tconst char *buf, *eol, *line_color, *match_color;\n+\tregmatch_t match;\n+\tint eflags = 0;\n+\n+\tbuf = line;\n+\teol = buf + linelen;\n+\n+\tif (!opt || !want_color(color) || opt->invert)\n+\t\tgoto end;\n+\n+\tline_color = opt->colors[GREP_COLOR_SELECTED];\n+\tmatch_color = opt->colors[GREP_COLOR_MATCH_SELECTED];\n+\n+\twhile (grep_next_match(opt, buf, eol, ctx, &match, field, eflags)) {\n+\t\tif (match.rm_so == match.rm_eo)\n+\t\t\tbreak;\n+\n+\t\tstrbuf_add_with_color(sb, line_color, buf, match.rm_so);\n+\t\tstrbuf_add_with_color(sb, match_color, buf + match.rm_so,\n+\t\t\t\t      match.rm_eo - match.rm_so);\n+\t\tbuf += match.rm_eo;\n+\t\teflags = REG_NOTBOL;\n+\t}\n+\n+\tif (eflags)\n+\t\tstrbuf_add_with_color(sb, line_color, buf, eol - buf);\n+\telse {\n+end:\n+\t\tstrbuf_add(sb, buf, eol - buf);\n+\t}\n+}\n+\n void pp_user_info(struct pretty_print_context *pp,\n \t\t  const char *what, struct strbuf *sb,\n \t\t  const char *line, const char *encoding)\n@@ -496,9 +542,26 @@ void pp_user_info(struct pretty_print_context *pp,\n \t\t\tstrbuf_addch(sb, '\\n');\n \t\tstrbuf_addf(sb, \" <%.*s>\\n\", (int)maillen, mailbuf);\n \t} else {\n-\t\tstrbuf_addf(sb, \"%s: %.*s%.*s <%.*s>\\n\", what,\n-\t\t\t    (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0, \"    \",\n-\t\t\t    (int)namelen, namebuf, (int)maillen, mailbuf);\n+\t\tstruct strbuf id = STRBUF_INIT;\n+\t\tenum grep_header_field field = GREP_HEADER_FIELD_MAX;\n+\t\tstruct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;\n+\n+\t\tif (!strcmp(what, \"Author\"))\n+\t\t\tfield = GREP_HEADER_AUTHOR;\n+\t\telse if (!strcmp(what, \"Commit\"))\n+\t\t\tfield = GREP_HEADER_COMMITTER;\n+\n+\t\tstrbuf_addf(sb, \"%s: \", what);\n+\t\tif (pp->fmt == CMIT_FMT_FULLER)\n+\t\t\tstrbuf_addchars(sb, ' ', 4);\n+\n+\t\tstrbuf_addf(&id, \"%.*s <%.*s>\", (int)namelen, namebuf,\n+\t\t\t    (int)maillen, mailbuf);\n+\n+\t\tappend_line_with_color(sb, opt, id.buf, id.len, pp->color,\n+\t\t\t\t       GREP_CONTEXT_HEAD, field);\n+\t\tstrbuf_addch(sb, '\\n');\n+\t\tstrbuf_release(&id);\n \t}\n \n \tswitch (pp->fmt) {\n@@ -1939,8 +2002,9 @@ static int pp_utf8_width(const char *start, const char *end)\n \treturn width;\n }\n \n-static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,\n-\t\t\t\t const char *line, int linelen)\n+static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,\n+\t\t\t\t int color, int tabwidth, const char *line,\n+\t\t\t\t int linelen)\n {\n \tconst char *tab;\n \n@@ -1957,7 +2021,9 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,\n \t\t\tbreak;\n \n \t\t/* Output the data .. */\n-\t\tstrbuf_add(sb, line, tab - line);\n+\t\tappend_line_with_color(sb, opt, line, tab - line, color,\n+\t\t\t\t       GREP_CONTEXT_BODY,\n+\t\t\t\t       GREP_HEADER_FIELD_MAX);\n \n \t\t/* .. and the de-tabified tab */\n \t\tstrbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));\n@@ -1972,7 +2038,8 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,\n \t * worrying about width - there's nothing more to\n \t * align.\n \t */\n-\tstrbuf_add(sb, line, linelen);\n+\tappend_line_with_color(sb, opt, line, linelen, color, GREP_CONTEXT_BODY,\n+\t\t\t       GREP_HEADER_FIELD_MAX);\n }\n \n /*\n@@ -1984,11 +2051,16 @@ static void pp_handle_indent(struct pretty_print_context *pp,\n \t\t\t     struct strbuf *sb, int indent,\n \t\t\t     const char *line, int linelen)\n {\n+\tstruct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;\n+\n \tstrbuf_addchars(sb, ' ', indent);\n \tif (pp->expand_tabs_in_log)\n-\t\tstrbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);\n+\t\tstrbuf_add_tabexpand(sb, opt, pp->color, pp->expand_tabs_in_log,\n+\t\t\t\t     line, linelen);\n \telse\n-\t\tstrbuf_add(sb, line, linelen);\n+\t\tappend_line_with_color(sb, opt, line, linelen, pp->color,\n+\t\t\t\t       GREP_CONTEXT_BODY,\n+\t\t\t\t       GREP_HEADER_FIELD_MAX);\n }\n \n static int is_mboxrd_from(const char *line, int len)\n@@ -2006,7 +2078,9 @@ void pp_remainder(struct pretty_print_context *pp,\n \t\t  struct strbuf *sb,\n \t\t  int indent)\n {\n+\tstruct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;\n \tint first = 1;\n+\n \tfor (;;) {\n \t\tconst char *line = *msg_p;\n \t\tint linelen = get_one_line(line);\n@@ -2027,14 +2101,17 @@ void pp_remainder(struct pretty_print_context *pp,\n \t\tif (indent)\n \t\t\tpp_handle_indent(pp, sb, indent, line, linelen);\n \t\telse if (pp->expand_tabs_in_log)\n-\t\t\tstrbuf_add_tabexpand(sb, pp->expand_tabs_in_log,\n-\t\t\t\t\t     line, linelen);\n+\t\t\tstrbuf_add_tabexpand(sb, opt, pp->color,\n+\t\t\t\t\t     pp->expand_tabs_in_log, line,\n+\t\t\t\t\t     linelen);\n \t\telse {\n \t\t\tif (pp->fmt == CMIT_FMT_MBOXRD &&\n \t\t\t\t\tis_mboxrd_from(line, linelen))\n \t\t\t\tstrbuf_addch(sb, '>');\n \n-\t\t\tstrbuf_add(sb, line, linelen);\n+\t\t\tappend_line_with_color(sb, opt, line, linelen,\n+\t\t\t\t\t       pp->color, GREP_CONTEXT_BODY,\n+\t\t\t\t\t       GREP_HEADER_FIELD_MAX);\n \t\t}\n \t\tstrbuf_addch(sb, '\\n');\n \t}\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 9dfead936b..3d240bba57 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -449,6 +449,57 @@ test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurati\n \t)\n '\n \n+test_expect_success 'log --author' '\n+\tcat >expect <<-\\EOF &&\n+\tAuthor: <BOLD;RED>A U<RESET> Thor <author@example.com>\n+\tEOF\n+\tgit log -1 --color=always --author=\"A U\" >log &&\n+\tgrep Author log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --committer' '\n+\tcat >expect <<-\\EOF &&\n+\tCommit:     C O Mitter <committer@<BOLD;RED>example<RESET>.com>\n+\tEOF\n+\tgit log -1 --color=always --pretty=fuller --committer=\"example\" >log &&\n+\tgrep \"Commit:\" log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log -i --grep with color' '\n+\tcat >expect <<-\\EOF &&\n+\t    <BOLD;RED>Sec<RESET>ond\n+\t    <BOLD;RED>sec<RESET>ond\n+\tEOF\n+\tgit log --color=always -i --grep=^sec >log &&\n+\tgrep -i sec log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '-c color.grep.selected log --grep' '\n+\tcat >expect <<-\\EOF &&\n+\t    <GREEN>th<RESET><BOLD;RED>ir<RESET><GREEN>d<RESET>\n+\tEOF\n+\tgit -c color.grep.selected=\"green\" log --color=always --grep=ir >log &&\n+\tgrep ir log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '-c color.grep.matchSelected log --grep' '\n+\tcat >expect <<-\\EOF &&\n+\t    <BLUE>i<RESET>n<BLUE>i<RESET>t<BLUE>i<RESET>al\n+\tEOF\n+\tgit -c color.grep.matchSelected=\"blue\" log --color=always --grep=i >log &&\n+\tgrep al log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat > expect <<EOF\n * Second\n * sixth\n-- \n2.33.0\n\n"},{"id":"438224","messageId":"20211007203148.23888-3-someguy@effective-light.com","threadId":"56660","inReplyTo":"20211007203148.23888-1-someguy@effective-light.com","subject":"[PATCH v11 3/3] grep: fix an edge case concerning ascii patterns and UTF-8 data","fromName":"Hamza Mahfooz","fromEmail":"someguy@effective-light.com","sentAt":"2021-10-07T20:31:48Z","receivedAt":"2021-10-07T20:31:58Z","isPatch":true,"sender":{"key":"someguy@effective-light.com","avatar":"https://avatars.githubusercontent.com/u/3885502?v=4"},"body":"If we attempt to grep non-ascii log message text with an ascii pattern, we\nrun into the following issue:\n\n    $ git log --color --author='.var.*Bjar' -1 origin/master | grep ^Author\n    grep: (standard input): binary file matches\n\nSo, to fix this teach the grep code to mark the pattern as UTF-8 (even if\nthe pattern is composed of only ascii characters), so long as the log\noutput is encoded using UTF-8.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Hamza Mahfooz <someguy@effective-light.com>\n---\n grep.c                          |  6 +++--\n grep.h                          |  1 +\n revision.c                      |  2 ++\n t/t7812-grep-icase-non-ascii.sh | 48 +++++++++++++++++++++++++++++++++\n 4 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex fe847a0111..978d30f053 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -382,8 +382,10 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\t}\n \t\toptions |= PCRE2_CASELESS;\n \t}\n-\tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n-\t    !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n+\tif ((opt->utf8_all_the_things && !has_non_ascii(p->pattern)) ||\n+\t    (!opt->ignore_locale && is_utf8_locale() &&\n+\t     has_non_ascii(p->pattern) && !(!opt->ignore_case &&\n+\t\t\t\t\t    (p->fixed || p->is_fixed))))\n \t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n \n #ifdef GIT_PCRE2_VERSION_10_36_OR_HIGHER\ndiff --git a/grep.h b/grep.h\nindex 808ad76f0c..c9ddd637d1 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -167,6 +167,7 @@ struct grep_opt {\n \tint extended_regexp_option;\n \tint pattern_type_option;\n \tint ignore_locale;\n+\tint utf8_all_the_things;\n \tchar colors[NR_GREP_COLORS][COLOR_MAXLEN];\n \tunsigned pre_context;\n \tunsigned post_context;\ndiff --git a/revision.c b/revision.c\nindex 0dabb5a0bc..0d751dceb7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2874,6 +2874,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\t\t &revs->grep_filter);\n \tif (!is_encoding_utf8(get_log_output_encoding()))\n \t\trevs->grep_filter.ignore_locale = 1;\n+\telse\n+\t\trevs->grep_filter.utf8_all_the_things = 1;\n \tcompile_grep_patterns(&revs->grep_filter);\n \n \tif (revs->reverse && revs->reflog_info)\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex e5d1e4ea68..42323b31c0 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -53,6 +53,54 @@ test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,PCRE 'log --author with an ascii pattern on UTF-8 data' '\n+\tcat >expected <<-\\EOF &&\n+\tAuthor: <BOLD;RED>À Ú Thor<RESET> <author@example.com>\n+\tEOF\n+\ttest_write_lines \"forth\" >file4 &&\n+\tgit add file4 &&\n+\tgit commit --author=\"À Ú Thor <author@example.com>\" -m sécond &&\n+\tgit log -1 --color=always --perl-regexp --author=\".*Thor\" >log &&\n+\tgrep Author log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,PCRE 'log --committer with an ascii pattern on ISO-8859-1 data' '\n+\tcat >expected <<-\\EOF &&\n+\tCommit:     Ç<BOLD;RED> O Mîtter <committer@example.com><RESET>\n+\tEOF\n+\ttest_write_lines \"fifth\" >file5 &&\n+\tgit add file5 &&\n+\texport GIT_COMMITTER_NAME=\"Ç O Mîtter\" &&\n+\texport GIT_COMMITTER_EMAIL=\"committer@example.com\" &&\n+\tgit -c i18n.commitEncoding=latin1 commit -m thïrd &&\n+\tgit -c i18n.logOutputEncoding=latin1 log -1 --pretty=fuller --color=always --perl-regexp --committer=\" O.*\" >log &&\n+\tgrep Commit: log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,PCRE 'log --grep with an ascii pattern on UTF-8 data' '\n+\tcat >expected <<-\\EOF &&\n+\t    sé<BOLD;RED>con<RESET>d\n+\tEOF\n+\tgit log -1 --color=always --perl-regexp --grep=\"con\" >log &&\n+\tgrep con log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,PCRE 'log --grep with an ascii pattern on ISO-8859-1 data' '\n+\tcat >expected <<-\\EOF &&\n+\t    <BOLD;RED>thïrd<RESET>\n+\tEOF\n+\tgit -c i18n.logOutputEncoding=latin1 log -1 --color=always --perl-regexp --grep=\"th.*rd\" >log &&\n+\tgrep \"th.*rd\" log >actual.raw &&\n+\ttest_decode_color <actual.raw >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data' '\n \tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n \techo \"ævar\" >expected &&\n-- \n2.33.0\n\n"},{"id":"438360","messageId":"xmqq1r4vjji3.fsf@gitster.g","threadId":"56660","inReplyTo":"20211007203148.23888-3-someguy@effective-light.com","subject":"Re: [PATCH v11 3/3] grep: fix an edge case concerning ascii patterns and UTF-8 data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-08T21:26:28Z","receivedAt":"2021-10-08T21:26:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hamza Mahfooz <someguy@effective-light.com> writes:\n\n> If we attempt to grep non-ascii log message text with an ascii pattern, we\n> run into the following issue:\n>\n>     $ git log --color --author='.var.*Bjar' -1 origin/master | grep ^Author\n>     grep: (standard input): binary file matches\n>\n> So, to fix this teach the grep code to mark the pattern as UTF-8 (even if\n> the pattern is composed of only ascii characters), so long as the log\n> output is encoded using UTF-8.\n\nWe'd need this only if we are using pcre2 backend, no?  If that is\nthe case, that fact needs to be recorded in the proposed log message\nto help later developers, when they wonder why this \"all-the-things\"\nknob exists.\n\nAnd if it is the case that this bit is needed only to work around a\nglitch while using pcre2 backend, I'd rather want to see a solution\nthat does not need to contaminate the more generic \"struct grep_opt\"\ndata and \"setup_revisions()\" codepath.\n\nIn other words, can't the function compile_pcre2_pattern() make the\n\"is log output encoding utf8?\" decision locally and act accordingly?\n\nThanks.\n"},{"id":"438393","messageId":"xmqqlf32g0jl.fsf@gitster.g","threadId":"56660","inReplyTo":"20211007203148.23888-3-someguy@effective-light.com","subject":"Re: [PATCH v11 3/3] grep: fix an edge case concerning ascii patterns and UTF-8 data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-09T06:44:14Z","receivedAt":"2021-10-09T06:44:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hamza Mahfooz <someguy@effective-light.com> writes:\n\n> diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\n> index e5d1e4ea68..42323b31c0 100755\n> --- a/t/t7812-grep-icase-non-ascii.sh\n> +++ b/t/t7812-grep-icase-non-ascii.sh\n> @@ -53,6 +53,54 @@ test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '\n> ...\n> +\tgit add file5 &&\n> +\texport GIT_COMMITTER_NAME=\"Ç O Mîtter\" &&\n> +\texport GIT_COMMITTER_EMAIL=\"committer@example.com\" &&\n\nThese lines are flagged as an error by test-lint; I've queued this\non top to make today's integration result to pass our tests.\n\nThanks.\n\n\n t/t7812-grep-icase-non-ascii.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 42323b31c0..313d4894ad 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -72,8 +72,8 @@ test_expect_success GETTEXT_LOCALE,PCRE 'log --committer with an ascii pattern o\n \tEOF\n \ttest_write_lines \"fifth\" >file5 &&\n \tgit add file5 &&\n-\texport GIT_COMMITTER_NAME=\"Ç O Mîtter\" &&\n-\texport GIT_COMMITTER_EMAIL=\"committer@example.com\" &&\n+\tGIT_COMMITTER_NAME=\"Ç O Mîtter\" GIT_COMMITTER_EMAIL=\"committer@example.com\" &&\n+\texport GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&\n \tgit -c i18n.commitEncoding=latin1 commit -m thïrd &&\n \tgit -c i18n.logOutputEncoding=latin1 log -1 --pretty=fuller --color=always --perl-regexp --committer=\" O.*\" >log &&\n \tgrep Commit: log >actual.raw &&\n-- \n2.33.0-960-g8d902839aa\n\n"},{"id":"438411","messageId":"O2WP0R.U0CLDS9323JP1@effective-light.com","threadId":"56660","inReplyTo":"xmqqlf32g0jl.fsf@gitster.g","subject":"Re: [PATCH v11 3/3] grep: fix an edge case concerning ascii patterns and UTF-8 data","fromName":"Hamza Mahfooz","fromEmail":"someguy@effective-light.com","sentAt":"2021-10-09T15:52:00Z","receivedAt":"2021-10-09T15:52:09Z","isPatch":true,"sender":{"key":"someguy@effective-light.com","avatar":"https://avatars.githubusercontent.com/u/3885502?v=4"},"body":"\nOn Fri, Oct 8 2021 at 11:44:14 PM -0700, Junio C Hamano \n<gitster@pobox.com> wrote:\n> These lines are flagged as an error by test-lint; I've queued this\n> on top to make today's integration result to pass our tests.\n\nI fixed this issue in v12 [1] btw.\n\n[1] \nhttps://lkml.kernel.org/r/20211008224918.603392-3-someguy@effective-light.com\n\n\n"}]}