{"thread":{"id":"48734","subject":"[PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","startedAt":"2018-06-18T23:43:08Z","lastAt":"2018-06-26T16:46:04Z","messageCount":56,"participants":["Taylor Blau","Jeff King","Junio C Hamano","René Scharfe","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"350437","messageId":"cover.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":null,"subject":"[PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:01Z","receivedAt":"2018-06-18T23:43:08Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is a ``fresh start'' of my series to teach 'git grep --column'.\nSince the last time I sent this, much has changed, notably the semantics\nfor deciding which column is the first when given (1) extended\nexpressions and (2) --invert.\n\nBoth (1) and (2) are described in-depth in patch 2/7, but I am happy to\nanswer more questions should they arise here. Peff and I worked on this\ntogether off-list, and we are both happy with the semantics, and believe\nthat it covers most reasonable cases.\n\nThe notable case that it does _not_ cover is matching the following\nline:\n\n  a ... b\n\nwith the following expression\n\n  git grep --column -e b --or -e a\n\nThis will produce the column for 'b' rather than the column for 'a',\nsince we short-circuit an --or when the left child finds a match, in\nthis case 'b'. So, we break the semantics for this case, at the benefit\nof not having to do twice the work.\n\nIn the future, I'd like to revisit this, since any performance gains\nthat we _do_ make in this area are moot when we rescan all lines in\nshow_line() with --color. A path forward, I imagine, would look like a\nlist of regmatch_t's, or a set of locations in the expression tree, such\nthat we could either enumerate the list or walk the tree in order to\ncolorize the line. But, I think for now that is #leftoverbits.\n\nThanks especially to the last round of reviewers for their detailed\nfeedback, and I hope that starting in a new series will be OK. I figure\nthat enough has changed that I'd rather not clutter an already busy\nthread.\n\n\nThanks,\nTaylor\n\nTaylor Blau (7):\n  Documentation/config.txt: camel-case lineNumber for consistency\n  grep.c: expose {,inverted} match column in match_line()\n  grep.[ch]: extend grep_opt to allow showing matched column\n  grep.c: display column number of first match\n  builtin/grep.c: add '--column' option to 'git-grep(1)'\n  grep.c: add configuration variables to show matched option\n  contrib/git-jump/git-jump: jump to exact location\n\n Documentation/config.txt   |  7 ++-\n Documentation/git-grep.txt |  9 +++-\n builtin/grep.c             |  1 +\n contrib/git-jump/README    | 12 ++++-\n contrib/git-jump/git-jump  |  2 +-\n grep.c                     | 95 +++++++++++++++++++++++++++++---------\n grep.h                     |  2 +\n t/t7810-grep.sh            | 63 +++++++++++++++++++++++++\n 8 files changed, 163 insertions(+), 28 deletions(-)\n\n--\n2.17.0.582.gccdcbd54c\n"},{"id":"350438","messageId":"0e2a7e38aa7f23a784e3dc81296f73803b9ec329.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 1/7] Documentation/config.txt: camel-case lineNumber for consistency","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:05Z","receivedAt":"2018-06-18T23:43:14Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"lineNumber has casing that is inconsistent with surrounding options,\nlike color.grep.matchContext, and color.grep.matchSelected. Re-case this\ndocumentation in order to be consistent with the text around it, and to\nensure that new entries are consistent, too.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ab641bf5a9..58fde4daea 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1181,7 +1181,7 @@ color.grep.<slot>::\n \tfilename prefix (when not using `-h`)\n `function`;;\n \tfunction name lines (when using `-p`)\n-`linenumber`;;\n+`lineNumber`;;\n \tline number prefix (when using `-n`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350439","messageId":"d0a6b3ca2a8e67c6a5ed4d2c56c5121d4bc3554b.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 2/7] grep.c: expose {,inverted} match column in match_line()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:08Z","receivedAt":"2018-06-18T23:43:16Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When calling match_line(), callers presently cannot determine the\nrelative offset of the match because match_line() discards the\n'regmatch_t' that contains this information.\n\nInstead, teach match_line() to take in two 'ssize_t's. Fill the first\nwith the offset of the match produced by the given expression. If\nextended, fill the later with the offset of the match produced as if\n--invert were given.\n\nFor instance, matching \"--not -e x\" on this line produces a columnar\noffset of 0, (i.e., the whole line does not contain an x), but \"--invert\n--not -e -x\" will fill the later ssize_t of the column containing an\n\"x\", because this expression is semantically equivalent to \"-e x\".\n\nTo determine the column for the inverted and non-inverted case, do the\nfollowing:\n\n  - If matching an atom, the non-inverted column is as given from\n    match_one_pattern(), and the inverted column is unset.\n\n  - If matching a --not, the inverted column and non-inverted column swap.\n\n  - If matching an --and, or --or, the non-inverted column is the\n    minimum of the two children, with the exception that --or is\n    short-circuiting. For instance, if we match \"-e a --or -e b\" on a\n    line that contains both \"a\" and \"b\" (and \"b\" comes first), the match\n    column will hold \"a\", since we inspected the left child first, and\n    short-circuited over the right child.\n\nThis patch will become useful when we later pick between the two new\nresults in order to display the column number of the first match on a\nline with --column.\n\nCo-authored-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 56 ++++++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 38 insertions(+), 18 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 45ec7e636c..19c782aa9d 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1249,10 +1249,10 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n }\n \n static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t   enum grep_context ctx, int collect_hits)\n+\t\t\t   enum grep_context ctx, ssize_t *col,\n+\t\t\t   ssize_t *icol, int collect_hits)\n {\n \tint h = 0;\n-\tregmatch_t match;\n \n \tif (!x)\n \t\tdie(\"Not a valid grep expression\");\n@@ -1261,25 +1261,39 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\th = 1;\n \t\tbreak;\n \tcase GREP_NODE_ATOM:\n-\t\th = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);\n+\t\t{\n+\t\t\tregmatch_t tmp;\n+\t\t\th = match_one_pattern(x->u.atom, bol, eol, ctx,\n+\t\t\t\t\t      &tmp, 0);\n+\t\t\tif (h && (*col < 0 || tmp.rm_so < *col))\n+\t\t\t\t*col = tmp.rm_so;\n+\t\t}\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n+\t\t/*\n+\t\t * Upon visiting a GREP_NODE_NOT, imatch and match become\n+\t\t * swapped.\n+\t\t */\n+\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))\n+\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t     icol, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);\n+\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n \t\tif (!collect_hits)\n-\t\t\treturn (match_expr_eval(x->u.binary.left,\n-\t\t\t\t\t\tbol, eol, ctx, 0) ||\n-\t\t\t\tmatch_expr_eval(x->u.binary.right,\n-\t\t\t\t\t\tbol, eol, ctx, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);\n+\t\t\treturn (match_expr_eval(x->u.binary.left, bol, eol, ctx,\n+\t\t\t\t\t\tcol, icol, 0) ||\n+\t\t\t\tmatch_expr_eval(x->u.binary.right, bol, eol,\n+\t\t\t\t\t\tctx, col, icol, 0));\n+\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);\n+\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t     icol, 1);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1290,25 +1304,30 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n }\n \n static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n-\t\t      enum grep_context ctx, int collect_hits)\n+\t\t      enum grep_context ctx, ssize_t *col,\n+\t\t      ssize_t *icol, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, collect_hits);\n+\treturn match_expr_eval(x, bol, eol, ctx, col, icol, collect_hits);\n }\n \n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n+\t\t      ssize_t *col, ssize_t *icol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n-\tregmatch_t match;\n \n \tif (opt->extended)\n-\t\treturn match_expr(opt, bol, eol, ctx, collect_hits);\n+\t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n+\t\t\t\t  collect_hits);\n \n \t/* we do not call with collect_hits without being extended */\n \tfor (p = opt->pattern_list; p; p = p->next) {\n-\t\tif (match_one_pattern(p, bol, eol, ctx, &match, 0))\n+\t\tregmatch_t tmp;\n+\t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n+\t\t\t*col = tmp.rm_so;\n \t\t\treturn 1;\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -1763,6 +1782,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n \t\t * look_ahead() skips quickly to the line that possibly\n@@ -1786,7 +1806,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\tif ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\n \t\t\tctx = GREP_CONTEXT_BODY;\n \n-\t\thit = match_line(opt, bol, eol, ctx, collect_hits);\n+\t\thit = match_line(opt, bol, eol, &col, &icol, ctx, collect_hits);\n \t\t*eol = ch;\n \n \t\tif (collect_hits)\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350440","messageId":"4b22c427f14c4e0a17f04b41788f38ff366f1ad4.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 3/7] grep.[ch]: extend grep_opt to allow showing matched column","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:11Z","receivedAt":"2018-06-18T23:43:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support showing the matched column when calling 'git-grep(1)', teach\n'grep_opt' the normal set of options to configure the default behavior\nand colorization of this feature.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 3 +++\n grep.h | 2 ++\n 2 files changed, 5 insertions(+)\n\ndiff --git a/grep.c b/grep.c\nindex 19c782aa9d..33f8572e6d 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -46,6 +46,7 @@ void init_grep_defaults(void)\n \tcolor_set(opt->color_filename, \"\");\n \tcolor_set(opt->color_function, \"\");\n \tcolor_set(opt->color_lineno, \"\");\n+\tcolor_set(opt->color_columnno, \"\");\n \tcolor_set(opt->color_match_context, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_selected, \"\");\n@@ -155,6 +156,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \topt->extended_regexp_option = def->extended_regexp_option;\n \topt->pattern_type_option = def->pattern_type_option;\n \topt->linenum = def->linenum;\n+\topt->columnnum = def->columnnum;\n \topt->max_depth = def->max_depth;\n \topt->pathname = def->pathname;\n \topt->relative = def->relative;\n@@ -164,6 +166,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \tcolor_set(opt->color_filename, def->color_filename);\n \tcolor_set(opt->color_function, def->color_function);\n \tcolor_set(opt->color_lineno, def->color_lineno);\n+\tcolor_set(opt->color_columnno, def->color_columnno);\n \tcolor_set(opt->color_match_context, def->color_match_context);\n \tcolor_set(opt->color_match_selected, def->color_match_selected);\n \tcolor_set(opt->color_selected, def->color_selected);\ndiff --git a/grep.h b/grep.h\nindex 399381c908..08a0b391c5 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -127,6 +127,7 @@ struct grep_opt {\n \tint prefix_length;\n \tregex_t regexp;\n \tint linenum;\n+\tint columnnum;\n \tint invert;\n \tint ignore_case;\n \tint status_only;\n@@ -159,6 +160,7 @@ struct grep_opt {\n \tchar color_filename[COLOR_MAXLEN];\n \tchar color_function[COLOR_MAXLEN];\n \tchar color_lineno[COLOR_MAXLEN];\n+\tchar color_columnno[COLOR_MAXLEN];\n \tchar color_match_context[COLOR_MAXLEN];\n \tchar color_match_selected[COLOR_MAXLEN];\n \tchar color_selected[COLOR_MAXLEN];\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350441","messageId":"df2d08efc1fb717a564157fb760ff71becb658dd.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 4/7] grep.c: display column number of first match","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:14Z","receivedAt":"2018-06-18T23:43:21Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To prepare for 'git grep' learning '--column', teach grep.c's\nshow_line() how to show the column of the first match on non-context\nlines.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 30 +++++++++++++++++++++++++-----\n 1 file changed, 25 insertions(+), 5 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 33f8572e6d..9f5b00a471 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1381,7 +1381,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\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+\t\t      const char *name, unsigned lno, unsigned cno, char sign)\n {\n \tint rest = eol - bol;\n \tconst char *match_color, *line_color = NULL;\n@@ -1416,6 +1416,17 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\toutput_color(opt, buf, strlen(buf), opt->color_lineno);\n \t\toutput_sep(opt, sign);\n \t}\n+\t/*\n+\t * Treat 'cno' as the 1-indexed offset from the start of a non-context\n+\t * line to its first match. Otherwise, 'cno' is 0 indicating that we are\n+\t * being called with a context line.\n+\t */\n+\tif (opt->columnnum && cno) {\n+\t\tchar buf[32];\n+\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n+\t\toutput_color(opt, buf, strlen(buf), opt->color_columnno);\n+\t\toutput_sep(opt, sign);\n+\t}\n \tif (opt->color) {\n \t\tregmatch_t match;\n \t\tenum grep_context ctx = GREP_CONTEXT_BODY;\n@@ -1521,7 +1532,7 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,\n \t\t\tbreak;\n \n \t\tif (match_funcname(opt, gs, bol, eol)) {\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '=');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, 0, '=');\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1586,7 +1597,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \n \t\twhile (*eol != '\\n')\n \t\t\teol++;\n-\t\tshow_line(opt, bol, eol, gs->name, cur, sign);\n+\t\tshow_line(opt, bol, eol, gs->name, cur, 0, sign);\n \t\tbol = eol + 1;\n \t\tcur++;\n \t}\n@@ -1785,6 +1796,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t cno;\n \t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n@@ -1850,7 +1862,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t\tshow_pre_context(opt, gs, bol, eol, lno);\n \t\t\telse if (opt->funcname)\n \t\t\t\tshow_funcname_line(opt, gs, bol, lno);\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, ':');\n+\t\t\tcno = opt->invert ? icol : col;\n+\t\t\tif (cno < 0) {\n+\t\t\t\t/*\n+\t\t\t\t * A negative cno means that there was no match.\n+\t\t\t\t * Clamp to the beginning of the line.\n+\t\t\t\t */\n+\t\t\t\tcno = 0;\n+\t\t\t}\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, cno + 1, ':');\n \t\t\tlast_hit = lno;\n \t\t\tif (opt->funcbody)\n \t\t\t\tshow_function = 1;\n@@ -1879,7 +1899,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t/* If the last hit is within the post context,\n \t\t\t * we need to show this line.\n \t\t\t */\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '-');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, col + 1, '-');\n \t\t}\n \n \tnext_line:\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350442","messageId":"0374418de5082186468553a21d070907c0e51058.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:17Z","receivedAt":"2018-06-18T23:43:25Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Teach 'git-grep(1)' a new option, '--column', to show the column\nnumber of the first match on a non-context line. This makes it possible\nto teach 'contrib/git-jump/git-jump' how to seek to the first matching\nposition of a grep match in your editor, and allows similar additional\nscripting capabilities.\n\nFor example:\n\n  $ git grep -n --column foo | head -n3\n  .clang-format:51:14:# myFunction(foo, bar, baz);\n  .clang-format:64:7:# int foo();\n  .clang-format:75:8:# void foo()\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-grep.txt |  6 +++-\n builtin/grep.c             |  1 +\n t/t7810-grep.sh            | 63 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 69 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 312409a607..31dc0392a6 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t   [-v | --invert-match] [-h|-H] [--full-name]\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-P | --perl-regexp]\n-\t   [-F | --fixed-strings] [-n | --line-number]\n+\t   [-F | --fixed-strings] [-n | --line-number] [--column]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n \t   [(-O | --open-files-in-pager) [<pager>]]\n \t   [-z | --null]\n@@ -169,6 +169,10 @@ providing this option will cause it to die.\n --line-number::\n \tPrefix the line number to matching lines.\n \n+--column::\n+\tPrefix the 1-indexed byte-offset of the first match from the start of the\n+\tmatching line.\n+\n -l::\n --files-with-matches::\n --name-only::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex ee753a403e..61bcaf6e58 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -828,6 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t    GREP_PATTERN_TYPE_PCRE),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('n', \"line-number\", &opt.linenum, N_(\"show line numbers\")),\n+\t\tOPT_BOOL(0, \"column\", &opt.columnnum, N_(\"show column number of first match\")),\n \t\tOPT_NEGBIT('h', NULL, &opt.pathname, N_(\"don't show filenames\"), 1),\n \t\tOPT_BIT('H', NULL, &opt.pathname, N_(\"show filenames\"), 1),\n \t\tOPT_NEGBIT(0, \"full-name\", &opt.relative,\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 1797f632a3..daaf7b4c44 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -99,6 +99,69 @@ do\n \t\ttest_cmp expected actual\n \t'\n \n+\ttest_expect_success \"grep -w $L (with --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, extended)\" '\n+\t\t{\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:19:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap$ --or -e baz $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, --invert)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap\n+\t\t\techo ${HC}file:1:foo mmap bar_mmap\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -w -e baz $H -- file >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, --invert, extended)\" '\n+\t\t{\n+\t\t\techo ${HC}hello_world:6:HeLLo_world\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -e ll --or --not -e _ $H -- hello_world \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, -C)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file-foo_mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -C1 -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --line-number, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:5:foo mmap bar\n+\t\t\techo ${HC}file:3:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:4:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:5:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep -n --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep -w $L\" '\n \t\t{\n \t\t\techo ${HC}file:1:foo mmap bar\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350443","messageId":"4a36cd93c616b2a546c21c923d64f953a780e55d.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 6/7] grep.c: add configuration variables to show matched option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:20Z","receivedAt":"2018-06-18T23:43:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support git-grep(1)'s new option, '--column', document and teach\ngrep.c how to interpret relevant configuration options, similar to those\nassociated with '--line-number'.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt   | 5 +++++\n Documentation/git-grep.txt | 3 +++\n grep.c                     | 6 ++++++\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 58fde4daea..e4cbed3078 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1183,6 +1183,8 @@ color.grep.<slot>::\n \tfunction name lines (when using `-p`)\n `lineNumber`;;\n \tline number prefix (when using `-n`)\n+`column`;;\n+\tcolumn number prefix (when using `--column`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n `matchContext`;;\n@@ -1797,6 +1799,9 @@ gitweb.snapshot::\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 31dc0392a6..0de3493b80 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -44,6 +44,9 @@ CONFIGURATION\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/grep.c b/grep.c\nindex 9f5b00a471..8ffa94c791 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -96,6 +96,10 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\topt->linenum = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"grep.column\")) {\n+\t\topt->columnnum = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"grep.fullname\")) {\n \t\topt->relative = !git_config_bool(var, value);\n@@ -112,6 +116,8 @@ int grep_config(const char *var, const char *value, void *cb)\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.column\"))\n+\t\tcolor = opt->color_columnno;\n \telse if (!strcmp(var, \"color.grep.matchcontext\"))\n \t\tcolor = opt->color_match_context;\n \telse if (!strcmp(var, \"color.grep.matchselected\"))\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350444","messageId":"8819fcfbb1c443b37cbc719f1e4a0d61d6ed808d.1529365072.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH 7/7] contrib/git-jump/git-jump: jump to exact location","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-18T23:43:23Z","receivedAt":"2018-06-18T23:43:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Take advantage of 'git-grep(1)''s new option, '--column' in order to\nteach Peff's 'git-jump' script how to jump to the correct column for any\ngiven match.\n\n'git-grep(1)''s output is in the correct format for Vim's jump list, so\nno additional cleanup is necessary.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n contrib/git-jump/README   | 12 ++++++++++--\n contrib/git-jump/git-jump |  2 +-\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/git-jump/README b/contrib/git-jump/README\nindex 4484bda410..2f618a7f97 100644\n--- a/contrib/git-jump/README\n+++ b/contrib/git-jump/README\n@@ -25,6 +25,13 @@ git-jump will feed this to the editor:\n foo.c:2: printf(\"hello word!\\n\");\n -----------------------------------\n \n+Or, when running 'git jump grep', column numbers will also be emitted,\n+e.g. `git jump grep \"hello\"` would return:\n+\n+-----------------------------------\n+foo.c:2:9: printf(\"hello word!\\n\");\n+-----------------------------------\n+\n Obviously this trivial case isn't that interesting; you could just open\n `foo.c` yourself. But when you have many changes scattered across a\n project, you can use the editor's support to \"jump\" from point to point.\n@@ -35,7 +42,8 @@ Git-jump can generate four types of interesting lists:\n \n   2. The beginning of any merge conflict markers.\n \n-  3. Any grep matches.\n+  3. Any grep matches, including the column of the first match on a\n+     line.\n \n   4. Any whitespace errors detected by `git diff --check`.\n \n@@ -82,7 +90,7 @@ which does something similar to `git jump grep`. However, it is limited\n to positioning the cursor to the correct line in only the first file,\n leaving you to locate subsequent hits in that file or other files using\n the editor or pager. By contrast, git-jump provides the editor with a\n-complete list of files and line numbers for each match.\n+complete list of files, lines, and a column number for each match.\n \n \n Limitations\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 80ab0590bc..931b0fe3a9 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -52,7 +52,7 @@ mode_merge() {\n # editor shows them to us in the status bar.\n mode_grep() {\n \tcmd=$(git config jump.grepCmd)\n-\ttest -n \"$cmd\" || cmd=\"git grep -n\"\n+\ttest -n \"$cmd\" || cmd=\"git grep -n --column\"\n \t$cmd \"$@\" |\n \tperl -pe '\n \ts/[ \\t]+/ /g;\n-- \n2.17.0.582.gccdcbd54c\n"},{"id":"350492","messageId":"20180619162825.GA22034@sigill.intra.peff.net","threadId":"48734","inReplyTo":"df2d08efc1fb717a564157fb760ff71becb658dd.1529365072.git.me@ttaylorr.com","subject":"Re: [PATCH 4/7] grep.c: display column number of first match","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-19T16:28:26Z","receivedAt":"2018-06-19T16:28:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 18, 2018 at 06:43:14PM -0500, Taylor Blau wrote:\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> +\t\t      const char *name, unsigned lno, unsigned cno, char sign)\n\nHere \"cno\" is unsigned. But later...\n\n> +\tif (opt->columnnum && cno) {\n> +\t\tchar buf[32];\n> +\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n\n...we print it with \"%d\". Should this be \"%u\"?\n\nBut ultimately, the column number comes from this code:\n\n> @@ -1785,6 +1796,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n>  \twhile (left) {\n>  \t\tchar *eol, ch;\n>  \t\tint hit;\n> +\t\tssize_t cno;\n>  \t\tssize_t col = -1, icol = -1;\n>  \n>  \t\t/*\n> @@ -1850,7 +1862,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n>  \t\t\t\tshow_pre_context(opt, gs, bol, eol, lno);\n>  \t\t\telse if (opt->funcname)\n>  \t\t\t\tshow_funcname_line(opt, gs, bol, lno);\n> -\t\t\tshow_line(opt, bol, eol, gs->name, lno, ':');\n> +\t\t\tcno = opt->invert ? icol : col;\n> +\t\t\tif (cno < 0) {\n> +\t\t\t\t/*\n> +\t\t\t\t * A negative cno means that there was no match.\n> +\t\t\t\t * Clamp to the beginning of the line.\n> +\t\t\t\t */\n> +\t\t\t\tcno = 0;\n> +\t\t\t}\n\n...which is a ssize_t. Should we just be using ssize_t consistently?\n\nWe do at least clamp the negative values here, but on 64-bit systems\nssize_t is much larger than \"unsigned\".  I admit that it's probably\nridiculous for any single line to overflow 32 bits, but it seems like we\nshould consistently use size_t/ssize_t for buffer offsets, and then we\ndon't have to think about it.\n\n-Peff\n"},{"id":"350494","messageId":"20180619163414.GA27246@syl.attlocal.net","threadId":"48734","inReplyTo":"20180619162825.GA22034@sigill.intra.peff.net","subject":"Re: [PATCH 4/7] grep.c: display column number of first match","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T16:34:14Z","receivedAt":"2018-06-19T16:34:21Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 12:28:26PM -0400, Jeff King wrote:\n> On Mon, Jun 18, 2018 at 06:43:14PM -0500, Taylor Blau wrote:\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> > +\t\t      const char *name, unsigned lno, unsigned cno, char sign)\n>\n> Here \"cno\" is unsigned. But later...\n>\n> > +\tif (opt->columnnum && cno) {\n> > +\t\tchar buf[32];\n> > +\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n>\n> ...we print it with \"%d\". Should this be \"%u\"?\n\nThanks, that's certainly a mistake. I think (per the hunk of this\nresponse below) that it should be \"%zu\" in the case that we change this\npatch to take an ssize_t.\n\n> But ultimately, the column number comes from this code:\n>\n> > @@ -1785,6 +1796,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n> >  \twhile (left) {\n> >  \t\tchar *eol, ch;\n> >  \t\tint hit;\n> > +\t\tssize_t cno;\n> >  \t\tssize_t col = -1, icol = -1;\n> >\n> >  \t\t/*\n> > @@ -1850,7 +1862,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n> >  \t\t\t\tshow_pre_context(opt, gs, bol, eol, lno);\n> >  \t\t\telse if (opt->funcname)\n> >  \t\t\t\tshow_funcname_line(opt, gs, bol, lno);\n> > -\t\t\tshow_line(opt, bol, eol, gs->name, lno, ':');\n> > +\t\t\tcno = opt->invert ? icol : col;\n> > +\t\t\tif (cno < 0) {\n> > +\t\t\t\t/*\n> > +\t\t\t\t * A negative cno means that there was no match.\n> > +\t\t\t\t * Clamp to the beginning of the line.\n> > +\t\t\t\t */\n> > +\t\t\t\tcno = 0;\n> > +\t\t\t}\n>\n> ...which is a ssize_t. Should we just be using ssize_t consistently?\n>\n> We do at least clamp the negative values here, but on 64-bit systems\n> ssize_t is much larger than \"unsigned\".  I admit that it's probably\n> ridiculous for any single line to overflow 32 bits, but it seems like we\n> should consistently use size_t/ssize_t for buffer offsets, and then we\n> don't have to think about it.\n\nI agree that it's unlikely that a single line will overflow 32 bits, and\ncertainly at that point we might have other problems to worry about :-).\n\nThis was an unsigned in my original patch, and I left it this way in the\nrevised series for consistency with the other arguments to show_line().\nBut, I agree with your reasoning and think that this should be an\nssize_t, instead.\n\n\nThanks,\nTaylor\n"},{"id":"350495","messageId":"20180619163506.GB22034@sigill.intra.peff.net","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-19T16:35:06Z","receivedAt":"2018-06-19T16:35:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote:\n\n> Attached is a ``fresh start'' of my series to teach 'git grep --column'.\n> Since the last time I sent this, much has changed, notably the semantics\n> for deciding which column is the first when given (1) extended\n> expressions and (2) --invert.\n> \n> Both (1) and (2) are described in-depth in patch 2/7, but I am happy to\n> answer more questions should they arise here. Peff and I worked on this\n> together off-list, and we are both happy with the semantics, and believe\n> that it covers most reasonable cases.\n\nSo with the exception of some minor type-quibbling in patch 4, this all\nlooks good to me. Since we discussed this quite a bit off-list, you can\ntake that review either with a giant grain of salt (reviewing something\nI helped to generate in the first place) or a ringing endorsement (I\nthought about it a lot more than I would have for a normal review ;) ).\n\n> The notable case that it does _not_ cover is matching the following\n> line:\n> \n>   a ... b\n> \n> with the following expression\n> \n>   git grep --column -e b --or -e a\n> \n> This will produce the column for 'b' rather than the column for 'a',\n> since we short-circuit an --or when the left child finds a match, in\n> this case 'b'. So, we break the semantics for this case, at the benefit\n> of not having to do twice the work.\n> \n> In the future, I'd like to revisit this, since any performance gains\n> that we _do_ make in this area are moot when we rescan all lines in\n> show_line() with --color. A path forward, I imagine, would look like a\n> list of regmatch_t's, or a set of locations in the expression tree, such\n> that we could either enumerate the list or walk the tree in order to\n> colorize the line. But, I think for now that is #leftoverbits.\n\nThe key thing about this iteration is that it doesn't regress\nperformance, because we always short-circuit where we used to. The other\nobvious route is to stop short-circuiting only when \"--column\" is in\neffect, which would have the same property (at the expense of a little\nextra code in match_expr_eval()).\n\n-Peff\n"},{"id":"350497","messageId":"xmqq1sd2wwpz.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-19T16:46:16Z","receivedAt":"2018-06-19T16:46:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> Attached is a ``fresh start'' of my series to teach 'git grep --column'.\n> Since the last time I sent this, much has changed, notably the semantics\n> for deciding which column is the first when given (1) extended\n> expressions and (2) --invert.\n> ...\n> In the future, I'd like to revisit this, since any performance gains\n> that we _do_ make in this area are moot when we rescan all lines in\n> show_line() with --color.\n\nSounds like a plan.  From a quick scan, it seems that this is\nsufficiently different from the last round so I won't bother\nrebuilding your \"--only\" series on top, and instead just drop those\ntwo series from the older round and queue this as the new and\nimproved tb/grep-column topic.\n\nThanks.\n"},{"id":"350498","messageId":"xmqqwouuvi0e.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"d0a6b3ca2a8e67c6a5ed4d2c56c5121d4bc3554b.1529365072.git.me@ttaylorr.com","subject":"Re: [PATCH 2/7] grep.c: expose {,inverted} match column in match_line()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-19T16:49:21Z","receivedAt":"2018-06-19T16:49:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n>  \tcase GREP_NODE_NOT:\n> -\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n> +\t\t/*\n> +\t\t * Upon visiting a GREP_NODE_NOT, imatch and match become\n> +\t\t * swapped.\n> +\t\t */\n> +\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);\n\nA minor nit, but the comment talks about something that are\ndifferent from the variable names; perhaps you called col/icol with\ndifferent names in an earlier incarnation of this patch?\n\n"},{"id":"350500","messageId":"20180619170219.GA36718@syl.attlocal.net","threadId":"48734","inReplyTo":"xmqq1sd2wwpz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T17:02:19Z","receivedAt":"2018-06-19T17:02:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 09:46:16AM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > Attached is a ``fresh start'' of my series to teach 'git grep --column'.\n> > Since the last time I sent this, much has changed, notably the semantics\n> > for deciding which column is the first when given (1) extended\n> > expressions and (2) --invert.\n> > ...\n> > In the future, I'd like to revisit this, since any performance gains\n> > that we _do_ make in this area are moot when we rescan all lines in\n> > show_line() with --color.\n>\n> Sounds like a plan.  From a quick scan, it seems that this is\n> sufficiently different from the last round so I won't bother\n> rebuilding your \"--only\" series on top, and instead just drop those\n> two series from the older round and queue this as the new and\n> improved tb/grep-column topic.\n\nThank you. I recommend dropping the second series\n(tb/grep-only-matching) entirely for now. I will rebase that on top of\nthis so that you can apply it later in the future with ease.\n\nI don't expect the parts of this series that affect that one to change\nmuch, but I'll hold off on rebasing it in general until this series is\nstable, hopefully soon.\n\n> Thanks.\n\nThanks,\nTaylor\n"},{"id":"350501","messageId":"20180619170248.GA37277@syl.attlocal.net","threadId":"48734","inReplyTo":"xmqqwouuvi0e.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/7] grep.c: expose {,inverted} match column in match_line()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T17:02:48Z","receivedAt":"2018-06-19T17:02:55Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 09:49:21AM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> >  \tcase GREP_NODE_NOT:\n> > -\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n> > +\t\t/*\n> > +\t\t * Upon visiting a GREP_NODE_NOT, imatch and match become\n> > +\t\t * swapped.\n> > +\t\t */\n> > +\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);\n>\n> A minor nit, but the comment talks about something that are\n> different from the variable names; perhaps you called col/icol with\n> different names in an earlier incarnation of this patch?\n\nGood catch, thanks. I've amended my local copy.\n\nThanks,\nTaylor\n"},{"id":"350505","messageId":"5282e3bb-bf7a-ab3a-98dc-d29ff1c37468@web.de","threadId":"48734","inReplyTo":"20180619163506.GB22034@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-06-19T17:33:39Z","receivedAt":"2018-06-19T17:33:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.06.2018 um 18:35 schrieb Jeff King:\n> On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote:\n>> The notable case that it does _not_ cover is matching the following\n>> line:\n>>\n>>    a ... b\n>>\n>> with the following expression\n>>\n>>    git grep --column -e b --or -e a\n>>\n>> This will produce the column for 'b' rather than the column for 'a',\n>> since we short-circuit an --or when the left child finds a match, in\n>> this case 'b'. So, we break the semantics for this case, at the benefit\n>> of not having to do twice the work.\n>>\n>> In the future, I'd like to revisit this, since any performance gains\n>> that we _do_ make in this area are moot when we rescan all lines in\n>> show_line() with --color. A path forward, I imagine, would look like a\n>> list of regmatch_t's, or a set of locations in the expression tree, such\n>> that we could either enumerate the list or walk the tree in order to\n>> colorize the line. But, I think for now that is #leftoverbits.\n> \n> The key thing about this iteration is that it doesn't regress\n> performance, because we always short-circuit where we used to. The other\n> obvious route is to stop short-circuiting only when \"--column\" is in\n> effect, which would have the same property (at the expense of a little\n> extra code in match_expr_eval()).\n\nThe performance impact of the exhaustive search for --color scales with\nthe number of shown lines, while it would scale with the total number of\nlines for --column.  Coloring the results of highly selective patterns\nis relatively cheap, short-circuiting them still helps significantly.\n\nDisabling that optimization for --column wouldn't be a regression since\nit's a new option..  Picking a random result (based on the order of\nevaluation) seems sloppy and is probably going to surprise users.\n\nWe could add an optimizer pass to reduce the number of regular\nexpressions in certain cases if that is really too slow.  E.g. this:\n\n\t$ git grep -e b -e a\n\n... is equivalent to:\n\n\t$ git grep -e '\\(b\\)\\|\\(a\\)'\n\nIn that example the optimizer should use a single kwset instead of a\nregex, but you get the idea, namely to leave the short-circuiting to the\nregex engine or kwset, which probably do a good job of it.\n\nRené\n"},{"id":"350509","messageId":"20180619174452.GA47272@syl.attlocal.net","threadId":"48734","inReplyTo":"5282e3bb-bf7a-ab3a-98dc-d29ff1c37468@web.de","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T17:44:52Z","receivedAt":"2018-06-19T17:45:03Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:\n> Am 19.06.2018 um 18:35 schrieb Jeff King:\n> > On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote:\n> >> The notable case that it does _not_ cover is matching the following\n> >> line:\n> >>\n> >>    a ... b\n> >>\n> >> with the following expression\n> >>\n> >>    git grep --column -e b --or -e a\n> >>\n> >> This will produce the column for 'b' rather than the column for 'a',\n> >> since we short-circuit an --or when the left child finds a match, in\n> >> this case 'b'. So, we break the semantics for this case, at the benefit\n> >> of not having to do twice the work.\n> >>\n> >> In the future, I'd like to revisit this, since any performance gains\n> >> that we _do_ make in this area are moot when we rescan all lines in\n> >> show_line() with --color. A path forward, I imagine, would look like a\n> >> list of regmatch_t's, or a set of locations in the expression tree, such\n> >> that we could either enumerate the list or walk the tree in order to\n> >> colorize the line. But, I think for now that is #leftoverbits.\n> >\n> > The key thing about this iteration is that it doesn't regress\n> > performance, because we always short-circuit where we used to. The other\n> > obvious route is to stop short-circuiting only when \"--column\" is in\n> > effect, which would have the same property (at the expense of a little\n> > extra code in match_expr_eval()).\n>\n> The performance impact of the exhaustive search for --color scales with\n> the number of shown lines, while it would scale with the total number of\n> lines for --column.  Coloring the results of highly selective patterns\n> is relatively cheap, short-circuiting them still helps significantly.\n\nSure. I was pointing out that we are repeating work in cases where it is\nunnecessary to do so. It seems natural to me to take one of the two\nabove approaches, and optimize where we can.\n\n> Disabling that optimization for --column wouldn't be a regression since\n> it's a new option..  Picking a random result (based on the order of\n> evaluation) seems sloppy and is probably going to surprise users.\n\nThat's fair, and something I'm happy to do if others feel OK about it.\nHere is a patch to that effect:\n\ndiff --git a/grep.c b/grep.c\nindex f3329d82ed..a09935d8c5 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1257,8 +1257,8 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \treturn hit;\n }\n\n-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t   enum grep_context ctx, ssize_t *col,\n+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n+\t\t\t   char *eol, enum grep_context ctx, ssize_t *col,\n \t\t\t   ssize_t *icol, int collect_hits)\n {\n \tint h = 0;\n@@ -1282,26 +1282,27 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\t/*\n \t\t * Upon visiting a GREP_NODE_NOT, col and icol become swapped.\n \t\t */\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);\n+\t\th = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col, 0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t     icol, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n+\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n-\t\tif (!collect_hits)\n-\t\t\treturn (match_expr_eval(x->u.binary.left, bol, eol, ctx,\n+\t\tif (!(collect_hits || opt->columnnum))\n+\t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol, ctx,\n \t\t\t\t\t\tcol, icol, 0) ||\n-\t\t\t\tmatch_expr_eval(x->u.binary.right, bol, eol,\n+\t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol, eol,\n \t\t\t\t\t\tctx, col, icol, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n-\t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n-\t\t\t\t     icol, 1);\n+\t\tif (collect_hits)\n+\t\t\tx->u.binary.left->hit |= h;\n+\t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t     icol, collect_hits);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1316,7 +1317,7 @@ static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n \t\t      ssize_t *icol, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, col, icol, collect_hits);\n+\treturn match_expr_eval(opt, x, bol, eol, ctx, col, icol, collect_hits);\n }\n\n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n\n> We could add an optimizer pass to reduce the number of regular\n> expressions in certain cases if that is really too slow.  E.g. this:\n>\n> \t$ git grep -e b -e a\n>\n> ... is equivalent to:\n>\n> \t$ git grep -e '\\(b\\)\\|\\(a\\)'\n>\n> In that example the optimizer should use a single kwset instead of a\n> regex, but you get the idea, namely to leave the short-circuiting to the\n> regex engine or kwset, which probably do a good job of it.\n\nI think that--while this pushes that decision to the appropriate level\nof indirection--that it is out of scope for this series, and that the\nabove patch should do a sufficient job at not surprising users.\n\n> René\n\nThanks,\nTaylor\n"},{"id":"350510","messageId":"20180619174846.GA27820@sigill.intra.peff.net","threadId":"48734","inReplyTo":"5282e3bb-bf7a-ab3a-98dc-d29ff1c37468@web.de","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-19T17:48:47Z","receivedAt":"2018-06-19T17:48:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:\n\n> > The key thing about this iteration is that it doesn't regress\n> > performance, because we always short-circuit where we used to. The other\n> > obvious route is to stop short-circuiting only when \"--column\" is in\n> > effect, which would have the same property (at the expense of a little\n> > extra code in match_expr_eval()).\n> \n> The performance impact of the exhaustive search for --color scales with\n> the number of shown lines, while it would scale with the total number of\n> lines for --column.  Coloring the results of highly selective patterns\n> is relatively cheap, short-circuiting them still helps significantly.\n\nI thought that at first, too, but I think we'd still scale with the\nnumber of shown lines. We're talking about short-circuiting OR, so by\ndefinition we stop the short-circuit because we matched the first half\nof the OR.\n\nIf you stop short-circuiting AND, then yes, you incur a penalty for\nevery line. But I don't think --column would need to do that.\n\nAlthough there are interesting cases around inversion. For example:\n\n  git grep --not \\( --not -e a --and --not -e b \\)\n\nis equivalent to:\n\n  git grep -e a --or -e b\n\nDo people care if we actually hunt down the exact column where we\n_didn't_ match \"b\" in the first case?  The two are equivalent, but I\nhave to wonder if somebody writing the first one really cares.\n\n> Disabling that optimization for --column wouldn't be a regression since\n> it's a new option..  Picking a random result (based on the order of\n> evaluation) seems sloppy and is probably going to surprise users.\n\nI don't see it as a random result; short-circuiting logic is well\nunderstood and we follow the user's ordering.\n\nI think the place where it's _most_ ugly is \"--column --color\", where we\nmay color the short-circuited value in the second pass.\n\n> We could add an optimizer pass to reduce the number of regular\n> expressions in certain cases if that is really too slow.  E.g. this:\n\nYes, we actually discussed this kind of transformation. I think it's way\nout of scope for this patch series, though. If we do anything more, I\nthink it should be to disable short-circuiting when --column is in use.\n\n-Peff\n"},{"id":"350511","messageId":"20180619175422.GA48866@syl.attlocal.net","threadId":"48734","inReplyTo":"20180619174846.GA27820@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T17:54:22Z","receivedAt":"2018-06-19T17:54:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 01:48:47PM -0400, Jeff King wrote:\n> On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:\n> > Disabling that optimization for --column wouldn't be a regression since\n> > it's a new option..  Picking a random result (based on the order of\n> > evaluation) seems sloppy and is probably going to surprise users.\n>\n> I don't see it as a random result; short-circuiting logic is well\n> understood and we follow the user's ordering.\n\nAgreed. I would prefer _not_ to apply the patch that I sent to René,\nsince I think it adds more complexity than its worth (precisely because\nthe short-circuiting logic is known, though certainly the problem gets\nworse as the expression tree grows).\n\n> I think the place where it's _most_ ugly is \"--column --color\", where we\n> may color the short-circuited value in the second pass.\n\nAgreed again, but it's worth noting that --color is the default. To play\ndevil's advocate, the use-case that I imagine will be most common is\nwith \"git jump,\" so perhaps that doesn't matter as much.\n\n> > We could add an optimizer pass to reduce the number of regular\n> > expressions in certain cases if that is really too slow.  E.g. this:\n>\n> Yes, we actually discussed this kind of transformation. I think it's way\n> out of scope for this patch series, though. If we do anything more, I\n> think it should be to disable short-circuiting when --column is in use.\n\nPiggy-backing on what I said to René, agreed again.\n\n> -Peff\n\nThanks,\nTaylor\n"},{"id":"350512","messageId":"838caafd-9172-3320-7d4c-b8e53eb3ea01@web.de","threadId":"48734","inReplyTo":"20180619174452.GA47272@syl.attlocal.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-06-19T17:50:52Z","receivedAt":"2018-06-19T17:56:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.06.2018 um 19:44 schrieb Taylor Blau:\n> On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:\n>> Am 19.06.2018 um 18:35 schrieb Jeff King:\n>>> On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote:\n>> We could add an optimizer pass to reduce the number of regular\n>> expressions in certain cases if that is really too slow.  E.g. this:\n>>\n>> \t$ git grep -e b -e a\n>>\n>> ... is equivalent to:\n>>\n>> \t$ git grep -e '\\(b\\)\\|\\(a\\)'\n>>\n>> In that example the optimizer should use a single kwset instead of a\n>> regex, but you get the idea, namely to leave the short-circuiting to the\n>> regex engine or kwset, which probably do a good job of it.\n> \n> I think that--while this pushes that decision to the appropriate level\n> of indirection--that it is out of scope for this series, and that the\n> above patch should do a sufficient job at not surprising users.\n\nDefinitely.  I'm not even convinced that performance problem is real --\notherwise someone would have added such an optimization already, right?\n:)\n\nRené\n"},{"id":"350513","messageId":"xmqqo9g6vet5.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"20180619174846.GA27820@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-19T17:58:30Z","receivedAt":"2018-06-19T17:58:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Although there are interesting cases around inversion. For example:\n>\n>   git grep --not \\( --not -e a --and --not -e b \\)\n>\n> is equivalent to:\n>\n>   git grep -e a --or -e b\n>\n> Do people care if we actually hunt down the exact column where we\n> _didn't_ match \"b\" in the first case?  The two are equivalent, but I\n> have to wonder if somebody writing the first one really cares.\n\nI may be misunderstanding the question, but I personally would feel\nthat \"git grep --not <ANYTHING>\" is OK to say \"the entire line is at\nfault that it did not satisify the criteria to match <ANYTHING>\".\nI.e., I'd be happy if --column marked the first column as the\nbeginning of the match, or --color painted the entire line in the\noutput of the former.\n\n\n"},{"id":"350514","messageId":"20180619180225.GA50589@syl.attlocal.net","threadId":"48734","inReplyTo":"xmqqo9g6vet5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T18:02:25Z","receivedAt":"2018-06-19T18:02:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Jun 19, 2018 at 10:58:30AM -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n>\n> > Although there are interesting cases around inversion. For example:\n> >\n> >   git grep --not \\( --not -e a --and --not -e b \\)\n> >\n> > is equivalent to:\n> >\n> >   git grep -e a --or -e b\n> >\n> > Do people care if we actually hunt down the exact column where we\n> > _didn't_ match \"b\" in the first case?  The two are equivalent, but I\n> > have to wonder if somebody writing the first one really cares.\n>\n> I may be misunderstanding the question, but I personally would feel\n> that \"git grep --not <ANYTHING>\" is OK to say \"the entire line is at\n> fault that it did not satisify the criteria to match <ANYTHING>\".\n> I.e., I'd be happy if --column marked the first column as the\n> beginning of the match, or --color painted the entire line in the\n> output of the former.\n\nI think that Peff is pointing out that a short-circuiting OR will cause\na line like\n\n  b foo a\n\nto print the column for \"a\", not \"b\" (when given \"-e a --or -e b\").\n\nThanks,\nTaylor\n"},{"id":"350515","messageId":"20180619180525.GA28203@sigill.intra.peff.net","threadId":"48734","inReplyTo":"xmqqo9g6vet5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-19T18:05:25Z","receivedAt":"2018-06-19T18:05:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2018 at 10:58:30AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Although there are interesting cases around inversion. For example:\n> >\n> >   git grep --not \\( --not -e a --and --not -e b \\)\n> >\n> > is equivalent to:\n> >\n> >   git grep -e a --or -e b\n> >\n> > Do people care if we actually hunt down the exact column where we\n> > _didn't_ match \"b\" in the first case?  The two are equivalent, but I\n> > have to wonder if somebody writing the first one really cares.\n> \n> I may be misunderstanding the question, but I personally would feel\n> that \"git grep --not <ANYTHING>\" is OK to say \"the entire line is at\n> fault that it did not satisify the criteria to match <ANYTHING>\".\n> I.e., I'd be happy if --column marked the first column as the\n> beginning of the match, or --color painted the entire line in the\n> output of the former.\n\nEven if it's a double-inversion? The reason we carry both `col` and\n`icol` is that it allows:\n\n  git grep --not --not --not --not -e a\n\nto still say \"we found 'a' here\". That's a dumb thing to ask for, but it\nis true in the end that we show lines with \"a\" (and will colorize them\nas such).\n\n-Peff\n"},{"id":"350517","messageId":"xmqqfu1iveaw.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"20180619180525.GA28203@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-19T18:09:27Z","receivedAt":"2018-06-19T18:09:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Even if it's a double-inversion? The reason we carry both `col` and\n> `icol` is that it allows:\n>\n>   git grep --not --not --not --not -e a\n>\n> to still say \"we found 'a' here\". That's a dumb thing to ask for, but it\n> is true in the end that we show lines with \"a\" (and will colorize them\n> as such).\n\nYes.  I think the code is doing too much to cater to a dumb request\n;-)\n\n"},{"id":"350523","messageId":"6ab94453-84ef-e269-c320-e102b02d6f3b@web.de","threadId":"48734","inReplyTo":"20180619174846.GA27820@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-06-19T18:50:16Z","receivedAt":"2018-06-19T18:50:44Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.06.2018 um 19:48 schrieb Jeff King:\n> On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:\n> \n>>> The key thing about this iteration is that it doesn't regress\n>>> performance, because we always short-circuit where we used to. The other\n>>> obvious route is to stop short-circuiting only when \"--column\" is in\n>>> effect, which would have the same property (at the expense of a little\n>>> extra code in match_expr_eval()).\n>>\n>> The performance impact of the exhaustive search for --color scales with\n>> the number of shown lines, while it would scale with the total number of\n>> lines for --column.  Coloring the results of highly selective patterns\n>> is relatively cheap, short-circuiting them still helps significantly.\n> \n> I thought that at first, too, but I think we'd still scale with the\n> number of shown lines. We're talking about short-circuiting OR, so by\n> definition we stop the short-circuit because we matched the first half\n> of the OR.\n> \n> If you stop short-circuiting AND, then yes, you incur a penalty for\n> every line. But I don't think --column would need to do that.\n\nGood point.  So disabling that optimization for --column (or in even\nin general) shouldn't be a dramatic loss.\n\n> Although there are interesting cases around inversion. For example:\n> \n>    git grep --not \\( --not -e a --and --not -e b \\)\n> \n> is equivalent to:\n> \n>    git grep -e a --or -e b\n> \n> Do people care if we actually hunt down the exact column where we\n> _didn't_ match \"b\" in the first case?  The two are equivalent, but I\n> have to wonder if somebody writing the first one really cares.\n\nI'd rather not guess the intentions of someone using such convoluted\nexpressions. ;-)\n\nNegation causes the whole non-matching line to match, with --column\nreporting 1 or nothing in such a case, right?  Or I think doing the\nsame when the operator is applied a second time is explainable.\n\n>> Disabling that optimization for --column wouldn't be a regression since\n>> it's a new option..  Picking a random result (based on the order of\n>> evaluation) seems sloppy and is probably going to surprise users.\n> \n> I don't see it as a random result; short-circuiting logic is well\n> understood and we follow the user's ordering.\n\nWhen ORing multiple expressions I don't pay attention to their order\nas that operator is commutative.  Having results depend on that\norder would at least surprise me.\n\n> I think the place where it's _most_ ugly is \"--column --color\", where we\n> may color the short-circuited value in the second pass.\n\nThe double negative example you gave above has that discrepancy as\nwell, but I think in that case it just highlights the different\nintentions (--color: highlight search terms, --column: show leftmost\nmatch).\n\n>> We could add an optimizer pass to reduce the number of regular\n>> expressions in certain cases if that is really too slow.  E.g. this:\n> \n> Yes, we actually discussed this kind of transformation. I think it's way\n> out of scope for this patch series, though. If we do anything more, I\n> think it should be to disable short-circuiting when --column is in use.\n\nYep.\n\nRené\n"},{"id":"350526","messageId":"20180619191156.GA21641@sigill.intra.peff.net","threadId":"48734","inReplyTo":"6ab94453-84ef-e269-c320-e102b02d6f3b@web.de","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-19T19:11:56Z","receivedAt":"2018-06-19T19:12:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 19, 2018 at 08:50:16PM +0200, René Scharfe wrote:\n\n> Negation causes the whole non-matching line to match, with --column\n> reporting 1 or nothing in such a case, right?  Or I think doing the\n> same when the operator is applied a second time is explainable.\n\nYes to your first question.\n\nRegarding the final sentence, yes, I agree it's explainable. But I\nthought that handling negation like this was one of the main complaints\nof earlier iterations?\n\n> When ORing multiple expressions I don't pay attention to their order\n> as that operator is commutative.  Having results depend on that\n> order would at least surprise me.\n\nOK. Let's just disable the short-circuit for --column then (i.e., what\nTaylor posted earlier). That's explainable, and I doubt the performance\nimplications are going to be all that noticeable.\n\n-Peff\n"},{"id":"350530","messageId":"80b9a0b1-3849-7097-fe1a-dd80835d62ae@web.de","threadId":"48734","inReplyTo":"20180619174452.GA47272@syl.attlocal.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-06-19T20:26:47Z","receivedAt":"2018-06-19T20:27:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.06.2018 um 19:44 schrieb Taylor Blau:\n> diff --git a/grep.c b/grep.c\n> index f3329d82ed..a09935d8c5 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -1257,8 +1257,8 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>   \treturn hit;\n>   }\n> \n> -static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n> -\t\t\t   enum grep_context ctx, ssize_t *col,\n> +static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n> +\t\t\t   char *eol, enum grep_context ctx, ssize_t *col,\n>   \t\t\t   ssize_t *icol, int collect_hits)\n\nPassing opt in is one way.  Piggy-backing on collect_hits and making it\na flags parameter for two bits might be easier.  At least it wouldn't\nincrease the number of function arguments any further.  Not sure.\n\nAnyway, something like this would be needed as well; or we could\nforce opt->columnnum to switch opt->extended on:\n\ndiff --git a/grep.c b/grep.c\nindex 8ffa94c791..a724ca3010 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1325,6 +1321,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n+\tint hit = 0;\n \n \tif (opt->extended)\n \t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n@@ -1334,11 +1331,14 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \tfor (p = opt->pattern_list; p; p = p->next) {\n \t\tregmatch_t tmp;\n \t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n-\t\t\t*col = tmp.rm_so;\n-\t\t\treturn 1;\n+\t\t\thit |= 1;\n+\t\t\tif (!opt->columnnum)\n+\t\t\t\tbreak;\n+\t\t\tif (*col < 0 || tmp.rm_so < *col)\n+\t\t\t\t*col = tmp.rm_so;\n \t\t}\n \t}\n-\treturn 0;\n+\treturn hit;\n }\n \n static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n"},{"id":"350531","messageId":"385b429e-5d57-91c0-054c-0f04c59538e3@web.de","threadId":"48734","inReplyTo":"20180619191156.GA21641@sigill.intra.peff.net","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-06-19T20:34:44Z","receivedAt":"2018-06-19T20:34:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.06.2018 um 21:11 schrieb Jeff King:\n> On Tue, Jun 19, 2018 at 08:50:16PM +0200, René Scharfe wrote:\n> \n>> Negation causes the whole non-matching line to match, with --column\n>> reporting 1 or nothing in such a case, right?  Or I think doing the\n>> same when the operator is applied a second time is explainable.\n\n(Not sure where that extra \"Or\" came from.)\n\n> Yes to your first question.\n> \n> Regarding the final sentence, yes, I agree it's explainable. But I\n> thought that handling negation like this was one of the main complaints\n> of earlier iterations?\n\nThat's possible -- I didn't read the full thread, and I didn't argue\nfor or against any specific behavior in this regard before AFAIR.\n\nSo let's see what your example does:\n\n   $ git grep --column --not \\( --not -e foo --or --not -e bar \\) trace.h\n   trace.h:13: *  #define foo(format, ...) bar(format, __VA_ARGS__)\n   $ git grep --column --not \\( --not -e bar --or --not -e foo \\) trace.h\n   trace.h:13: *  #define foo(format, ...) bar(format, __VA_ARGS__)\n\nImpressive.  That expression is confusing at first sight, but showing\nthe first matching column anyway requires no further explanation.  I\nlike it.\n\nRené\n"},{"id":"350534","messageId":"xmqq7emuv6s4.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"385b429e-5d57-91c0-054c-0f04c59538e3@web.de","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-19T20:51:55Z","receivedAt":"2018-06-19T20:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> So let's see what your example does:\n>\n>    $ git grep --column --not \\( --not -e foo --or --not -e bar \\) trace.h\n>    trace.h:13: *  #define foo(format, ...) bar(format, __VA_ARGS__)\n>    $ git grep --column --not \\( --not -e bar --or --not -e foo \\) trace.h\n>    trace.h:13: *  #define foo(format, ...) bar(format, __VA_ARGS__)\n>\n> Impressive.  That expression is confusing at first sight, but showing\n> the first matching column anyway requires no further explanation.  I\n> like it.\n\nOK, obviously many people are a lot more math/logic minded than I\nam.  I still think that any code that attempts to show the same\nresult as '-e bar --and -e foo' from the above is over-engineered,\nbut as long as it is done corretly and efficiently I won't have any\nobjection.\n\nThanks.\n\n\n\n"},{"id":"350544","messageId":"20180619225115.GA58486@syl.attlocal.net","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-19T22:51:15Z","receivedAt":"2018-06-19T22:51:24Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote:\n> Hi,\n>\n> Attached is a ``fresh start'' of my series to teach 'git grep --column'.\n> Since the last time I sent this, much has changed, notably the semantics\n> for deciding which column is the first when given (1) extended\n> expressions and (2) --invert.\n\nHi (again),\n\nI have posted an updated version of this series on my fork available at\n[1], in case anyone is interested in the changes before I publish them\nhere tomorrow.\n\nI am going to let this thread sit overnight to collect any more\nfeedback.\n\n\nThanks,\nTaylor\n\n[1]: https://github.com/ttaylorr/git/compare/tb/grep-colno\n"},{"id":"350580","messageId":"cover.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:30Z","receivedAt":"2018-06-20T20:05:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nHere is a re-roll of my series to add --column to 'git-grep(1)'. Since\nlast time, not much has changed other than the following:\n\n  - Fix a typo where 'col', 'icol' were spelled as 'match', 'imatch'\n    [1].\n\n  - Disable short-circuiting OR when --column is given [2].\n\n  - Disable early-return in match_line() when multiple patterns are\n    given and --column is, too [3].\n\n  - Add some more tests in t7810.\n\nThanks again for your kind and through review; hopefully this re-roll\nshould be OK to queue as-is.\n\n\nThanks,\nTaylor\n\n[1]: https://public-inbox.org/git/xmqqwouuvi0e.fsf@gitster-ct.c.googlers.com/\n[2]: https://public-inbox.org/git/20180619174452.GA47272@syl.attlocal.net/\n[3]: https://public-inbox.org/git/80b9a0b1-3849-7097-fe1a-dd80835d62ae@web.de/\n\nTaylor Blau (7):\n  Documentation/config.txt: camel-case lineNumber for consistency\n  grep.c: expose {,inverted} match column in match_line()\n  grep.[ch]: extend grep_opt to allow showing matched column\n  grep.c: display column number of first match\n  builtin/grep.c: add '--column' option to 'git-grep(1)'\n  grep.c: add configuration variables to show matched option\n  contrib/git-jump/git-jump: jump to exact location\n\n Documentation/config.txt   |   7 ++-\n Documentation/git-grep.txt |   9 ++-\n builtin/grep.c             |   1 +\n contrib/git-jump/README    |  12 +++-\n contrib/git-jump/git-jump  |   2 +-\n grep.c                     | 126 ++++++++++++++++++++++++++++---------\n grep.h                     |   2 +\n t/t7810-grep.sh            |  84 +++++++++++++++++++++++++\n 8 files changed, 210 insertions(+), 33 deletions(-)\n\nInter-diff (since v1):\n\ndiff --git a/grep.c b/grep.c\nindex 8ffa94c791..08d3df2855 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1257,8 +1257,8 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \treturn hit;\n }\n\n-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t   enum grep_context ctx, ssize_t *col,\n+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n+\t\t\t   char *eol, enum grep_context ctx, ssize_t *col,\n \t\t\t   ssize_t *icol, int collect_hits)\n {\n \tint h = 0;\n@@ -1280,29 +1280,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n \t\t/*\n-\t\t * Upon visiting a GREP_NODE_NOT, imatch and match become\n-\t\t * swapped.\n+\t\t * Upon visiting a GREP_NODE_NOT, col and icol become swapped.\n \t\t */\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, icol, col, 0);\n+\t\th = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col,\n+\t\t\t\t     0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t     icol, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n+\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n-\t\tif (!collect_hits)\n-\t\t\treturn (match_expr_eval(x->u.binary.left, bol, eol, ctx,\n-\t\t\t\t\t\tcol, icol, 0) ||\n-\t\t\t\tmatch_expr_eval(x->u.binary.right, bol, eol,\n-\t\t\t\t\t\tctx, col, icol, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, col,\n+\t\tif (!(collect_hits || opt->columnnum)) {\n+\t\t\t/*\n+\t\t\t * Don't short-circuit OR when given --column (or\n+\t\t\t * collecting hits) to ensure we don't skip a later\n+\t\t\t * child that would produce an earlier match.\n+\t\t\t */\n+\t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol,\n+\t\t\t\t\t\tctx, col, icol, 0) ||\n+\t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol,\n+\t\t\t\t\t\teol, ctx, col, icol, 0));\n+\t\t}\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n-\t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, col,\n-\t\t\t\t     icol, 1);\n+\t\tif (collect_hits)\n+\t\t\tx->u.binary.left->hit |= h;\n+\t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t     icol, collect_hits);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1317,7 +1324,7 @@ static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n \t\t      ssize_t *icol, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, col, icol, collect_hits);\n+\treturn match_expr_eval(opt, x, bol, eol, ctx, col, icol, collect_hits);\n }\n\n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n@@ -1325,6 +1332,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n+\tint hit = 0;\n\n \tif (opt->extended)\n \t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n@@ -1334,11 +1342,21 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \tfor (p = opt->pattern_list; p; p = p->next) {\n \t\tregmatch_t tmp;\n \t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n-\t\t\t*col = tmp.rm_so;\n-\t\t\treturn 1;\n+\t\t\thit |= 1;\n+\t\t\tif (!opt->columnnum) {\n+\t\t\t\t/*\n+\t\t\t\t * Without --column, any single match on a line\n+\t\t\t\t * is enough to know that it needs to be\n+\t\t\t\t * printed. With --column, scan _all_ patterns\n+\t\t\t\t * to find the earliest.\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (*col < 0 || tmp.rm_so < *col)\n+\t\t\t\t*col = tmp.rm_so;\n \t\t}\n \t}\n-\treturn 0;\n+\treturn hit;\n }\n\n static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n@@ -1387,7 +1405,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\n }\n\n static void show_line(struct grep_opt *opt, char *bol, char *eol,\n-\t\t      const char *name, unsigned lno, unsigned cno, char sign)\n+\t\t      const char *name, unsigned lno, ssize_t cno, char sign)\n {\n \tint rest = eol - bol;\n \tconst char *match_color, *line_color = NULL;\n@@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t */\n \tif (opt->columnnum && cno) {\n \t\tchar buf[32];\n-\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n+\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n \t\toutput_color(opt, buf, strlen(buf), opt->color_columnno);\n \t\toutput_sep(opt, sign);\n \t}\n@@ -1871,8 +1889,11 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\tcno = opt->invert ? icol : col;\n \t\t\tif (cno < 0) {\n \t\t\t\t/*\n-\t\t\t\t * A negative cno means that there was no match.\n-\t\t\t\t * Clamp to the beginning of the line.\n+\t\t\t\t * A negative cno indicates that there was no\n+\t\t\t\t * match on the line. We are thus inverted and\n+\t\t\t\t * being asked to show all lines that _don't_\n+\t\t\t\t * match a given expression. Therefore, set cno\n+\t\t\t\t * to 0 to suggest the whole line matches.\n \t\t\t\t */\n \t\t\t\tcno = 0;\n \t\t\t}\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex daaf7b4c44..bf0b572dab 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -139,6 +139,15 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n+\ttest_expect_success \"grep $L (with --column, double-negation)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column --not \\( --not -e foo --or --not -e baz \\) $H -- file \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep -w $L (with --column, -C)\" '\n \t\t{\n \t\t\techo ${HC}file:5:foo mmap bar\n@@ -162,6 +171,18 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n+\ttest_expect_success \"grep -w $L (with non-extended patterns, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e bar -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep -w $L\" '\n \t\t{\n \t\t\techo ${HC}file:1:foo mmap bar\n--\n2.17.0.582.gccdcbd54c\n"},{"id":"350581","messageId":"2da767ef5f90a06df30d85c99562cbe0d16a2e39.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 1/7] Documentation/config.txt: camel-case lineNumber for consistency","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:34Z","receivedAt":"2018-06-20T20:05:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"lineNumber has casing that is inconsistent with surrounding options,\nlike color.grep.matchContext, and color.grep.matchSelected. Re-case this\ndocumentation in order to be consistent with the text around it, and to\nensure that new entries are consistent, too.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ab641bf5a9..58fde4daea 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1181,7 +1181,7 @@ color.grep.<slot>::\n \tfilename prefix (when not using `-h`)\n `function`;;\n \tfunction name lines (when using `-p`)\n-`linenumber`;;\n+`lineNumber`;;\n \tline number prefix (when using `-n`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350582","messageId":"047550bcfd35941476dd9c8cb86351ef1ab37482.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 2/7] grep.c: expose {,inverted} match column in match_line()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:36Z","receivedAt":"2018-06-20T20:05:45Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When calling match_line(), callers presently cannot determine the\nrelative offset of the match because match_line() discards the\n'regmatch_t' that contains this information.\n\nInstead, teach match_line() to take in two 'ssize_t's. Fill the first\nwith the offset of the match produced by the given expression. If\nextended, fill the later with the offset of the match produced as if\n--invert were given.\n\nFor instance, matching \"--not -e x\" on this line produces a columnar\noffset of 0, (i.e., the whole line does not contain an x), but \"--invert\n--not -e -x\" will fill the later ssize_t of the column containing an\n\"x\", because this expression is semantically equivalent to \"-e x\".\n\nTo determine the column for the inverted and non-inverted case, do the\nfollowing:\n\n  - If matching an atom, the non-inverted column is as given from\n    match_one_pattern(), and the inverted column is unset.\n\n  - If matching a --not, the inverted column and non-inverted column\n    swap.\n\n  - If matching an --and, or --or, the non-inverted column is the\n    minimum of the two children, with the exception that --or is\n    short-circuiting. For instance, if we match \"-e a --or -e b\" on a\n    line that contains both \"a\" and \"b\" (and \"b\" comes first), the match\n    column will hold \"a\", since we inspected the left child first, and\n    short-circuited over the right child.\n\nThis patch will become useful when we later pick between the two new\nresults in order to display the column number of the first match on a\nline with --column.\n\nCo-authored-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 58 +++++++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 39 insertions(+), 19 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 45ec7e636c..dedfe17f93 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1248,11 +1248,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \treturn hit;\n }\n \n-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t   enum grep_context ctx, int collect_hits)\n+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n+\t\t\t   char *eol, enum grep_context ctx, ssize_t *col,\n+\t\t\t   ssize_t *icol, int collect_hits)\n {\n \tint h = 0;\n-\tregmatch_t match;\n \n \tif (!x)\n \t\tdie(\"Not a valid grep expression\");\n@@ -1261,25 +1261,39 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\th = 1;\n \t\tbreak;\n \tcase GREP_NODE_ATOM:\n-\t\th = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);\n+\t\t{\n+\t\t\tregmatch_t tmp;\n+\t\t\th = match_one_pattern(x->u.atom, bol, eol, ctx,\n+\t\t\t\t\t      &tmp, 0);\n+\t\t\tif (h && (*col < 0 || tmp.rm_so < *col))\n+\t\t\t\t*col = tmp.rm_so;\n+\t\t}\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n+\t\t/*\n+\t\t * Upon visiting a GREP_NODE_NOT, col and icol become swapped.\n+\t\t */\n+\t\th = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col,\n+\t\t\t\t     0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))\n+\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t     icol, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);\n+\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n \t\tif (!collect_hits)\n-\t\t\treturn (match_expr_eval(x->u.binary.left,\n-\t\t\t\t\t\tbol, eol, ctx, 0) ||\n-\t\t\t\tmatch_expr_eval(x->u.binary.right,\n-\t\t\t\t\t\tbol, eol, ctx, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);\n+\t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol,\n+\t\t\t\t\t\tctx, col, icol, 0) ||\n+\t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol,\n+\t\t\t\t\t\teol, ctx, col, icol, 0));\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);\n+\t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t     icol, 1);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1290,25 +1304,30 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n }\n \n static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n-\t\t      enum grep_context ctx, int collect_hits)\n+\t\t      enum grep_context ctx, ssize_t *col,\n+\t\t      ssize_t *icol, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, collect_hits);\n+\treturn match_expr_eval(opt, x, bol, eol, ctx, col, icol, collect_hits);\n }\n \n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n+\t\t      ssize_t *col, ssize_t *icol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n-\tregmatch_t match;\n \n \tif (opt->extended)\n-\t\treturn match_expr(opt, bol, eol, ctx, collect_hits);\n+\t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n+\t\t\t\t  collect_hits);\n \n \t/* we do not call with collect_hits without being extended */\n \tfor (p = opt->pattern_list; p; p = p->next) {\n-\t\tif (match_one_pattern(p, bol, eol, ctx, &match, 0))\n+\t\tregmatch_t tmp;\n+\t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n+\t\t\t*col = tmp.rm_so;\n \t\t\treturn 1;\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -1763,6 +1782,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n \t\t * look_ahead() skips quickly to the line that possibly\n@@ -1786,7 +1806,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\tif ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\n \t\t\tctx = GREP_CONTEXT_BODY;\n \n-\t\thit = match_line(opt, bol, eol, ctx, collect_hits);\n+\t\thit = match_line(opt, bol, eol, &col, &icol, ctx, collect_hits);\n \t\t*eol = ch;\n \n \t\tif (collect_hits)\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350583","messageId":"d66124e7930b0e70628b3359378aeff26f9732ac.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 3/7] grep.[ch]: extend grep_opt to allow showing matched column","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:39Z","receivedAt":"2018-06-20T20:05:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support showing the matched column when calling 'git-grep(1)', teach\n'grep_opt' the normal set of options to configure the default behavior\nand colorization of this feature.\n\nNow that we have opt->columnnum, use it to disable short-circuiting over\nORs so that col and icol are always filled with the earliest matches on\neach line. In addition, don't return the first match from match_line(),\nfor the same reason.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 33 +++++++++++++++++++++++++++------\n grep.h |  2 ++\n 2 files changed, 29 insertions(+), 6 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex dedfe17f93..d56d4a3a37 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -46,6 +46,7 @@ void init_grep_defaults(void)\n \tcolor_set(opt->color_filename, \"\");\n \tcolor_set(opt->color_function, \"\");\n \tcolor_set(opt->color_lineno, \"\");\n+\tcolor_set(opt->color_columnno, \"\");\n \tcolor_set(opt->color_match_context, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_selected, \"\");\n@@ -155,6 +156,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \topt->extended_regexp_option = def->extended_regexp_option;\n \topt->pattern_type_option = def->pattern_type_option;\n \topt->linenum = def->linenum;\n+\topt->columnnum = def->columnnum;\n \topt->max_depth = def->max_depth;\n \topt->pathname = def->pathname;\n \topt->relative = def->relative;\n@@ -164,6 +166,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \tcolor_set(opt->color_filename, def->color_filename);\n \tcolor_set(opt->color_function, def->color_function);\n \tcolor_set(opt->color_lineno, def->color_lineno);\n+\tcolor_set(opt->color_columnno, def->color_columnno);\n \tcolor_set(opt->color_match_context, def->color_match_context);\n \tcolor_set(opt->color_match_selected, def->color_match_selected);\n \tcolor_set(opt->color_selected, def->color_selected);\n@@ -1284,16 +1287,23 @@ static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n \t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n-\t\tif (!collect_hits)\n+\t\tif (!(collect_hits || opt->columnnum)) {\n+\t\t\t/*\n+\t\t\t * Don't short-circuit OR when given --column (or\n+\t\t\t * collecting hits) to ensure we don't skip a later\n+\t\t\t * child that would produce an earlier match.\n+\t\t\t */\n \t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol,\n \t\t\t\t\t\tctx, col, icol, 0) ||\n \t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol,\n \t\t\t\t\t\teol, ctx, col, icol, 0));\n+\t\t}\n \t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n-\t\tx->u.binary.left->hit |= h;\n+\t\tif (collect_hits)\n+\t\t\tx->u.binary.left->hit |= h;\n \t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n-\t\t\t\t     icol, 1);\n+\t\t\t\t     icol, collect_hits);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1316,6 +1326,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n+\tint hit = 0;\n \n \tif (opt->extended)\n \t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n@@ -1325,11 +1336,21 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \tfor (p = opt->pattern_list; p; p = p->next) {\n \t\tregmatch_t tmp;\n \t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n-\t\t\t*col = tmp.rm_so;\n-\t\t\treturn 1;\n+\t\t\thit |= 1;\n+\t\t\tif (!opt->columnnum) {\n+\t\t\t\t/*\n+\t\t\t\t * Without --column, any single match on a line\n+\t\t\t\t * is enough to know that it needs to be\n+\t\t\t\t * printed. With --column, scan _all_ patterns\n+\t\t\t\t * to find the earliest.\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (*col < 0 || tmp.rm_so < *col)\n+\t\t\t\t*col = tmp.rm_so;\n \t\t}\n \t}\n-\treturn 0;\n+\treturn hit;\n }\n \n static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\ndiff --git a/grep.h b/grep.h\nindex 399381c908..08a0b391c5 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -127,6 +127,7 @@ struct grep_opt {\n \tint prefix_length;\n \tregex_t regexp;\n \tint linenum;\n+\tint columnnum;\n \tint invert;\n \tint ignore_case;\n \tint status_only;\n@@ -159,6 +160,7 @@ struct grep_opt {\n \tchar color_filename[COLOR_MAXLEN];\n \tchar color_function[COLOR_MAXLEN];\n \tchar color_lineno[COLOR_MAXLEN];\n+\tchar color_columnno[COLOR_MAXLEN];\n \tchar color_match_context[COLOR_MAXLEN];\n \tchar color_match_selected[COLOR_MAXLEN];\n \tchar color_selected[COLOR_MAXLEN];\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350584","messageId":"91014cf1993a9b43983c9e7d2e19fbe442f0d89d.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 4/7] grep.c: display column number of first match","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:42Z","receivedAt":"2018-06-20T20:05:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To prepare for 'git grep' learning '--column', teach grep.c's\nshow_line() how to show the column of the first match on non-context\nlines.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 33 ++++++++++++++++++++++++++++-----\n 1 file changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex d56d4a3a37..d353d5d976 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1399,7 +1399,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\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+\t\t      const char *name, unsigned lno, ssize_t cno, char sign)\n {\n \tint rest = eol - bol;\n \tconst char *match_color, *line_color = NULL;\n@@ -1434,6 +1434,17 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\toutput_color(opt, buf, strlen(buf), opt->color_lineno);\n \t\toutput_sep(opt, sign);\n \t}\n+\t/*\n+\t * Treat 'cno' as the 1-indexed offset from the start of a non-context\n+\t * line to its first match. Otherwise, 'cno' is 0 indicating that we are\n+\t * being called with a context line.\n+\t */\n+\tif (opt->columnnum && cno) {\n+\t\tchar buf[32];\n+\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n+\t\toutput_color(opt, buf, strlen(buf), opt->color_columnno);\n+\t\toutput_sep(opt, sign);\n+\t}\n \tif (opt->color) {\n \t\tregmatch_t match;\n \t\tenum grep_context ctx = GREP_CONTEXT_BODY;\n@@ -1539,7 +1550,7 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,\n \t\t\tbreak;\n \n \t\tif (match_funcname(opt, gs, bol, eol)) {\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '=');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, 0, '=');\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1604,7 +1615,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \n \t\twhile (*eol != '\\n')\n \t\t\teol++;\n-\t\tshow_line(opt, bol, eol, gs->name, cur, sign);\n+\t\tshow_line(opt, bol, eol, gs->name, cur, 0, sign);\n \t\tbol = eol + 1;\n \t\tcur++;\n \t}\n@@ -1803,6 +1814,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t cno;\n \t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n@@ -1868,7 +1880,18 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t\tshow_pre_context(opt, gs, bol, eol, lno);\n \t\t\telse if (opt->funcname)\n \t\t\t\tshow_funcname_line(opt, gs, bol, lno);\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, ':');\n+\t\t\tcno = opt->invert ? icol : col;\n+\t\t\tif (cno < 0) {\n+\t\t\t\t/*\n+\t\t\t\t * A negative cno indicates that there was no\n+\t\t\t\t * match on the line. We are thus inverted and\n+\t\t\t\t * being asked to show all lines that _don't_\n+\t\t\t\t * match a given expression. Therefore, set cno\n+\t\t\t\t * to 0 to suggest the whole line matches.\n+\t\t\t\t */\n+\t\t\t\tcno = 0;\n+\t\t\t}\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, cno + 1, ':');\n \t\t\tlast_hit = lno;\n \t\t\tif (opt->funcbody)\n \t\t\t\tshow_function = 1;\n@@ -1897,7 +1920,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t/* If the last hit is within the post context,\n \t\t\t * we need to show this line.\n \t\t\t */\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '-');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, col + 1, '-');\n \t\t}\n \n \tnext_line:\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350585","messageId":"13e39136f0e12aac72f300a3a088c0663b83ce8e.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 6/7] grep.c: add configuration variables to show matched option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:47Z","receivedAt":"2018-06-20T20:05:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support git-grep(1)'s new option, '--column', document and teach\ngrep.c how to interpret relevant configuration options, similar to those\nassociated with '--line-number'.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt   | 5 +++++\n Documentation/git-grep.txt | 3 +++\n grep.c                     | 6 ++++++\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 58fde4daea..e4cbed3078 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1183,6 +1183,8 @@ color.grep.<slot>::\n \tfunction name lines (when using `-p`)\n `lineNumber`;;\n \tline number prefix (when using `-n`)\n+`column`;;\n+\tcolumn number prefix (when using `--column`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n `matchContext`;;\n@@ -1797,6 +1799,9 @@ gitweb.snapshot::\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 31dc0392a6..0de3493b80 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -44,6 +44,9 @@ CONFIGURATION\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/grep.c b/grep.c\nindex d353d5d976..08d3df2855 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -96,6 +96,10 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\topt->linenum = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"grep.column\")) {\n+\t\topt->columnnum = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"grep.fullname\")) {\n \t\topt->relative = !git_config_bool(var, value);\n@@ -112,6 +116,8 @@ int grep_config(const char *var, const char *value, void *cb)\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.column\"))\n+\t\tcolor = opt->color_columnno;\n \telse if (!strcmp(var, \"color.grep.matchcontext\"))\n \t\tcolor = opt->color_match_context;\n \telse if (!strcmp(var, \"color.grep.matchselected\"))\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350586","messageId":"7e2cbeeccacd8462a4128830927dae15895c4e1b.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 7/7] contrib/git-jump/git-jump: jump to exact location","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:49Z","receivedAt":"2018-06-20T20:05:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Take advantage of 'git-grep(1)''s new option, '--column' in order to\nteach Peff's 'git-jump' script how to jump to the correct column for any\ngiven match.\n\n'git-grep(1)''s output is in the correct format for Vim's jump list, so\nno additional cleanup is necessary.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n contrib/git-jump/README   | 12 ++++++++++--\n contrib/git-jump/git-jump |  2 +-\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/git-jump/README b/contrib/git-jump/README\nindex 4484bda410..2f618a7f97 100644\n--- a/contrib/git-jump/README\n+++ b/contrib/git-jump/README\n@@ -25,6 +25,13 @@ git-jump will feed this to the editor:\n foo.c:2: printf(\"hello word!\\n\");\n -----------------------------------\n \n+Or, when running 'git jump grep', column numbers will also be emitted,\n+e.g. `git jump grep \"hello\"` would return:\n+\n+-----------------------------------\n+foo.c:2:9: printf(\"hello word!\\n\");\n+-----------------------------------\n+\n Obviously this trivial case isn't that interesting; you could just open\n `foo.c` yourself. But when you have many changes scattered across a\n project, you can use the editor's support to \"jump\" from point to point.\n@@ -35,7 +42,8 @@ Git-jump can generate four types of interesting lists:\n \n   2. The beginning of any merge conflict markers.\n \n-  3. Any grep matches.\n+  3. Any grep matches, including the column of the first match on a\n+     line.\n \n   4. Any whitespace errors detected by `git diff --check`.\n \n@@ -82,7 +90,7 @@ which does something similar to `git jump grep`. However, it is limited\n to positioning the cursor to the correct line in only the first file,\n leaving you to locate subsequent hits in that file or other files using\n the editor or pager. By contrast, git-jump provides the editor with a\n-complete list of files and line numbers for each match.\n+complete list of files, lines, and a column number for each match.\n \n \n Limitations\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 80ab0590bc..931b0fe3a9 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -52,7 +52,7 @@ mode_merge() {\n # editor shows them to us in the status bar.\n mode_grep() {\n \tcmd=$(git config jump.grepCmd)\n-\ttest -n \"$cmd\" || cmd=\"git grep -n\"\n+\ttest -n \"$cmd\" || cmd=\"git grep -n --column\"\n \t$cmd \"$@\" |\n \tperl -pe '\n \ts/[ \\t]+/ /g;\n-- \n2.17.0.582.gccdcbd54c\n"},{"id":"350587","messageId":"ed1c197cc2cdbcfe7fce14747b8601d1488f89a9.1529524852.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"[PATCH v2 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-20T20:05:44Z","receivedAt":"2018-06-20T20:05:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Teach 'git-grep(1)' a new option, '--column', to show the column\nnumber of the first match on a non-context line. This makes it possible\nto teach 'contrib/git-jump/git-jump' how to seek to the first matching\nposition of a grep match in your editor, and allows similar additional\nscripting capabilities.\n\nFor example:\n\n  $ git grep -n --column foo | head -n3\n  .clang-format:51:14:# myFunction(foo, bar, baz);\n  .clang-format:64:7:# int foo();\n  .clang-format:75:8:# void foo()\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-grep.txt |  6 ++-\n builtin/grep.c             |  1 +\n t/t7810-grep.sh            | 84 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 90 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 312409a607..31dc0392a6 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t   [-v | --invert-match] [-h|-H] [--full-name]\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-P | --perl-regexp]\n-\t   [-F | --fixed-strings] [-n | --line-number]\n+\t   [-F | --fixed-strings] [-n | --line-number] [--column]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n \t   [(-O | --open-files-in-pager) [<pager>]]\n \t   [-z | --null]\n@@ -169,6 +169,10 @@ providing this option will cause it to die.\n --line-number::\n \tPrefix the line number to matching lines.\n \n+--column::\n+\tPrefix the 1-indexed byte-offset of the first match from the start of the\n+\tmatching line.\n+\n -l::\n --files-with-matches::\n --name-only::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex ee753a403e..61bcaf6e58 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -828,6 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t    GREP_PATTERN_TYPE_PCRE),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('n', \"line-number\", &opt.linenum, N_(\"show line numbers\")),\n+\t\tOPT_BOOL(0, \"column\", &opt.columnnum, N_(\"show column number of first match\")),\n \t\tOPT_NEGBIT('h', NULL, &opt.pathname, N_(\"don't show filenames\"), 1),\n \t\tOPT_BIT('H', NULL, &opt.pathname, N_(\"show filenames\"), 1),\n \t\tOPT_NEGBIT(0, \"full-name\", &opt.relative,\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 1797f632a3..bf0b572dab 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -99,6 +99,90 @@ do\n \t\ttest_cmp expected actual\n \t'\n \n+\ttest_expect_success \"grep -w $L (with --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, extended)\" '\n+\t\t{\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:19:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap$ --or -e baz $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, --invert)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap\n+\t\t\techo ${HC}file:1:foo mmap bar_mmap\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -w -e baz $H -- file >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, --invert, extended)\" '\n+\t\t{\n+\t\t\techo ${HC}hello_world:6:HeLLo_world\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -e ll --or --not -e _ $H -- hello_world \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, double-negation)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column --not \\( --not -e foo --or --not -e baz \\) $H -- file \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, -C)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file-foo_mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -C1 -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --line-number, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:5:foo mmap bar\n+\t\t\techo ${HC}file:3:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:4:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:5:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep -n --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with non-extended patterns, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e bar -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep -w $L\" '\n \t\t{\n \t\t\techo ${HC}file:1:foo mmap bar\n-- \n2.17.0.582.gccdcbd54c\n\n"},{"id":"350626","messageId":"20180621115302.GB15293@sigill.intra.peff.net","threadId":"48734","inReplyTo":"cover.1529524852.git.me@ttaylorr.com","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-21T11:53:02Z","receivedAt":"2018-06-21T11:53:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 20, 2018 at 03:05:30PM -0500, Taylor Blau wrote:\n\n> Hi,\n> \n> Here is a re-roll of my series to add --column to 'git-grep(1)'. Since\n> last time, not much has changed other than the following:\n> \n>   - Fix a typo where 'col', 'icol' were spelled as 'match', 'imatch'\n>     [1].\n> \n>   - Disable short-circuiting OR when --column is given [2].\n\nIf we're going to do this, should we be short-circuiting AND, too?\n\nHandling just OR makes this work:\n\n  $ ./git grep --column -e scalable --or -e fast -- README.md\n  README.md:7:Git - fast, scalable, distributed revision control system\n  README.md:10:Git is a fast, scalable, distributed revision control system with an\n\nbut not this:\n\n  $ ./git grep --column -v --not -e scalable --and --not -e fast -- README.md\n  README.md:13:Git - fast, scalable, distributed revision control system\n  README.md:16:Git is a fast, scalable, distributed revision control system with an\n\nI wasn't sure where we landed in the discussion on \"how much crazy stuff\nto support\". But AFAIK, the code in this iteration handles every crazy\ncase already except this one. If we're going to care about OR, maybe we\nshould just cover all cases.\n\n> @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n>  \t */\n>  \tif (opt->columnnum && cno) {\n>  \t\tchar buf[32];\n> -\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n> +\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n\nUnfortunately %z isn't portable. You have to use:\n\n  xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n\n-Peff\n"},{"id":"350628","messageId":"20180621120159.GC15293@sigill.intra.peff.net","threadId":"48734","inReplyTo":"20180621115302.GB15293@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-21T12:01:59Z","receivedAt":"2018-06-21T12:02:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 21, 2018 at 07:53:02AM -0400, Jeff King wrote:\n\n> > @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n> >  \t */\n> >  \tif (opt->columnnum && cno) {\n> >  \t\tchar buf[32];\n> > -\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n> > +\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n> \n> Unfortunately %z isn't portable. You have to use:\n> \n>   xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n\nTo clarify: yes, it is in C99. But traditionally we have not required\nthat.\n\nThis might be a candidate for another \"weather balloon\" patch to see if\nanybody complains, though. The last time time we dealt with this in a\nmajor way was over 7 years ago in 28bd70d811 (unbreak and eliminate\nNO_C99_FORMAT, 2011-03-16).\n\nI know Johannes switched out some \"%lu\" for PRIuMAX as recently as last\nAugust[1], but I think that is more about the Windows size_t not matching\n\"unsigned long\", and the decision to use PRIuMAX was to match the\nexisting codebase. AFAIK %zu is available on Windows.\n\n-Peff\n\n[1] https://public-inbox.org/git/alpine.DEB.2.21.1.1708100011391.11175@virtualbox/\n"},{"id":"350676","messageId":"xmqq7emrsw09.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"20180621115302.GB15293@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-21T20:52:06Z","receivedAt":"2018-06-21T20:52:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I wasn't sure where we landed in the discussion on \"how much crazy stuff\n> to support\". But AFAIK, the code in this iteration handles every crazy\n> case already except this one. If we're going to care about OR, maybe we\n> should just cover all cases.\n\nI think I was the only one who didn't like the tool knowing more\nboolean logic then the mere mortal user.  As long as the resulting\nbehaviour appears logically consistent to more math minded people, I\nam actually OK if the outcome to overly complex input feels \"too\nmagical\" ;-)\n\nAnd it appears that not short-circuiting AND will make the result\nbetter in that respect, so I am all for the suggestion above.\n\n"},{"id":"350691","messageId":"20180621214549.GA47197@syl.local","threadId":"48734","inReplyTo":"20180621115302.GB15293@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-21T21:45:49Z","receivedAt":"2018-06-21T21:45:56Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jun 21, 2018 at 07:53:02AM -0400, Jeff King wrote:\n> On Wed, Jun 20, 2018 at 03:05:30PM -0500, Taylor Blau wrote:\n>\n> > Hi,\n> >\n> > Here is a re-roll of my series to add --column to 'git-grep(1)'. Since\n> > last time, not much has changed other than the following:\n> >\n> >   - Fix a typo where 'col', 'icol' were spelled as 'match', 'imatch'\n> >     [1].\n> >\n> >   - Disable short-circuiting OR when --column is given [2].\n>\n> If we're going to do this, should we be short-circuiting AND, too?\n>\n> Handling just OR makes this work:\n>\n>   $ ./git grep --column -e scalable --or -e fast -- README.md\n>   README.md:7:Git - fast, scalable, distributed revision control system\n>   README.md:10:Git is a fast, scalable, distributed revision control system with an\n>\n> but not this:\n>\n>   $ ./git grep --column -v --not -e scalable --and --not -e fast -- README.md\n>   README.md:13:Git - fast, scalable, distributed revision control system\n>   README.md:16:Git is a fast, scalable, distributed revision control system with an\n>\n> I wasn't sure where we landed in the discussion on \"how much crazy stuff\n> to support\". But AFAIK, the code in this iteration handles every crazy\n> case already except this one. If we're going to care about OR, maybe we\n> should just cover all cases.\n\nGood catch. As I understand it, we need to short-circuit AND because an\n--invert or a --not above it in the expression tree would cause it to be\nturned into an OR by de Morgan's Law, and therefore be susceptible to\nthe same reasoning that caused me to remove short-circuiting on OR.\n\n> > @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n> >  \t */\n> >  \tif (opt->columnnum && cno) {\n> >  \t\tchar buf[32];\n> > -\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n> > +\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n>\n> Unfortunately %z isn't portable. You have to use:\n>\n>   xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n\nI see. Per your next note, I've changed this to \"%\"PRIuMAX (and the\nappropriate cast into uintmax_t), and will send another patch out in the\nfuture changing it _back_ to \"%zu\" and figure out how folks feel about\nit.\n\nI published these changes on my fork at [1], and will let this thread\nsit overnight again to see if anyone has any more feedback. Thank you!\n\n> -Peff\nThanks,\nTaylor\n\n[1]: https://github.com/ttaylorr/git/compare/tb/grep-colno\n"},{"id":"350712","messageId":"20180622072206.GB21200@sigill.intra.peff.net","threadId":"48734","inReplyTo":"20180621214549.GA47197@syl.local","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-22T07:22:06Z","receivedAt":"2018-06-22T07:22:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 21, 2018 at 04:45:49PM -0500, Taylor Blau wrote:\n\n> > but not this:\n> >\n> >   $ ./git grep --column -v --not -e scalable --and --not -e fast -- README.md\n> >   README.md:13:Git - fast, scalable, distributed revision control system\n> >   README.md:16:Git is a fast, scalable, distributed revision control system with an\n> >\n> > I wasn't sure where we landed in the discussion on \"how much crazy stuff\n> > to support\". But AFAIK, the code in this iteration handles every crazy\n> > case already except this one. If we're going to care about OR, maybe we\n> > should just cover all cases.\n> \n> Good catch. As I understand it, we need to short-circuit AND because an\n> --invert or a --not above it in the expression tree would cause it to be\n> turned into an OR by de Morgan's Law, and therefore be susceptible to\n> the same reasoning that caused me to remove short-circuiting on OR.\n\nRight, exactly.\n\n> > Unfortunately %z isn't portable. You have to use:\n> >\n> >   xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n> \n> I see. Per your next note, I've changed this to \"%\"PRIuMAX (and the\n> appropriate cast into uintmax_t), and will send another patch out in the\n> future changing it _back_ to \"%zu\" and figure out how folks feel about\n> it.\n\nThat sounds perfect. I think the time may be good for a \"%zu\"\nweather-balloon, but it's best not to tangle it up with your other\nchanges.\n\n-Peff\n"},{"id":"350725","messageId":"cover.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529365072.git.me@ttaylorr.com","subject":"[PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:26Z","receivedAt":"2018-06-22T15:49:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is my third--anticipate the final--re-roll of my series to\nteach 'git grep --column'.\n\nSince the last time, only a couple of things have changed at Peff's\nsuggestions in [1]. The changes are summarized here, and an inter-diff\nis available below:\n\n  - Change \"%zu\" to PRIuMAX (and an appropriate cast into uintmax_t). I\n    plan to send a follow-up patch to convert this back to \"%zu\" to see\n    how people feel about it, but I wanted to keep that out of the\n    present series in order to not hold things up.\n\n  - Don't short-circuit AND when given --column, since an earlier NOT\n    higher in the tree may cause an AND to be converted into an OR via\n    de Morgan's Law, in which case the problem is reduced to the OR case\n    (and should not have been short-circuited in the first place).\n\n  - Add a test in t7810 to cover this behavior (i.e., '--not \\( -e x\n    --and -e y \\)').\n\nThanks,\nTaylor\n\n[1]: https://public-inbox.org/git/20180621115302.GB15293@sigill.intra.peff.net/\n\nTaylor Blau (7):\n  Documentation/config.txt: camel-case lineNumber for consistency\n  grep.c: expose {,inverted} match column in match_line()\n  grep.[ch]: extend grep_opt to allow showing matched column\n  grep.c: display column number of first match\n  builtin/grep.c: add '--column' option to 'git-grep(1)'\n  grep.c: add configuration variables to show matched option\n  contrib/git-jump/git-jump: jump to exact location\n\n Documentation/config.txt   |   7 +-\n Documentation/git-grep.txt |   9 ++-\n builtin/grep.c             |   1 +\n contrib/git-jump/README    |  12 +++-\n contrib/git-jump/git-jump  |   2 +-\n grep.c                     | 134 +++++++++++++++++++++++++++++--------\n grep.h                     |   2 +\n t/t7810-grep.sh            |  95 ++++++++++++++++++++++++++\n 8 files changed, 228 insertions(+), 34 deletions(-)\n\nInter-diff (since: v2)\n\ndiff --git a/grep.c b/grep.c\nindex 08d3df2855..992673fe7e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1286,11 +1286,17 @@ static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n \t\t\t\t     0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n-\t\t\t\t     icol, 0))\n-\t\t\treturn 0;\n-\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n+\t\tif (h || opt->columnnum) {\n+\t\t\t/*\n+\t\t\t * Don't short-circuit AND when given --column, since a\n+\t\t\t * NOT earlier in the tree may turn this into an OR. In\n+\t\t\t * this case, see the below comment.\n+\t\t\t */\n+\t\t\th &= match_expr_eval(opt, x->u.binary.right, bol, eol,\n+\t\t\t\t\t     ctx, col, icol, 0);\n+\t\t}\n \t\tbreak;\n \tcase GREP_NODE_OR:\n \t\tif (!(collect_hits || opt->columnnum)) {\n@@ -1447,7 +1453,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t */\n \tif (opt->columnnum && cno) {\n \t\tchar buf[32];\n-\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n+\t\txsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n \t\toutput_color(opt, buf, strlen(buf), opt->color_columnno);\n \t\toutput_sep(opt, sign);\n \t}\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex bf0b572dab..9312c8daf5 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -110,7 +110,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n-\ttest_expect_success \"grep -w $L (with --column, extended)\" '\n+\ttest_expect_success \"grep -w $L (with --column, extended OR)\" '\n \t\t{\n \t\t\techo ${HC}file:14:foo_mmap bar mmap\n \t\t\techo ${HC}file:19:foo_mmap bar mmap baz\n@@ -130,7 +130,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n-\ttest_expect_success \"grep $L (with --column, --invert, extended)\" '\n+\ttest_expect_success \"grep $L (with --column, --invert, extended OR)\" '\n \t\t{\n \t\t\techo ${HC}hello_world:6:HeLLo_world\n \t\t} >expected &&\n@@ -139,6 +139,17 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n+\ttest_expect_success \"grep $L (with --column, --invert, extended AND)\" '\n+\t\t{\n+\t\t\techo ${HC}hello_world:3:Hello world\n+\t\t\techo ${HC}hello_world:3:Hello_world\n+\t\t\techo ${HC}hello_world:6:HeLLo_world\n+\t\t} >expected &&\n+\t\tgit grep --column --invert --not -e _ --and --not -e ll $H -- hello_world \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep $L (with --column, double-negation)\" '\n \t\t{\n \t\t\techo ${HC}file:1:foo_mmap bar mmap baz\n\n--\n2.18.0\n"},{"id":"350726","messageId":"fce254289e5408d2789e76c8a0147517ee1e5bab.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 1/7] Documentation/config.txt: camel-case lineNumber for consistency","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:31Z","receivedAt":"2018-06-22T15:49:38Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"lineNumber has casing that is inconsistent with surrounding options,\nlike color.grep.matchContext, and color.grep.matchSelected. Re-case this\ndocumentation in order to be consistent with the text around it, and to\nensure that new entries are consistent, too.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ab641bf5a9..58fde4daea 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1181,7 +1181,7 @@ color.grep.<slot>::\n \tfilename prefix (when not using `-h`)\n `function`;;\n \tfunction name lines (when using `-p`)\n-`linenumber`;;\n+`lineNumber`;;\n \tline number prefix (when using `-n`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n-- \n2.18.0\n\n"},{"id":"350727","messageId":"f8188471d165afd4a122d3de44973cb79d61e1de.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 2/7] grep.c: expose {,inverted} match column in match_line()","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:35Z","receivedAt":"2018-06-22T15:49:42Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When calling match_line(), callers presently cannot determine the\nrelative offset of the match because match_line() discards the\n'regmatch_t' that contains this information.\n\nInstead, teach match_line() to take in two 'ssize_t's. Fill the first\nwith the offset of the match produced by the given expression. If\nextended, fill the later with the offset of the match produced as if\n--invert were given.\n\nFor instance, matching \"--not -e x\" on this line produces a columnar\noffset of 0, (i.e., the whole line does not contain an x), but \"--invert\n--not -e -x\" will fill the later ssize_t of the column containing an\n\"x\", because this expression is semantically equivalent to \"-e x\".\n\nTo determine the column for the inverted and non-inverted case, do the\nfollowing:\n\n  - If matching an atom, the non-inverted column is as given from\n    match_one_pattern(), and the inverted column is unset.\n\n  - If matching a --not, the inverted column and non-inverted column\n    swap.\n\n  - If matching an --and, or --or, the non-inverted column is the\n    minimum of the two children.\n\nPresently, the existing short-circuiting logic for AND and OR applies as\nbefore. This will change in the following commit when we add options to\nconfigure the --column flag. Taken together, this and the forthcoming\nchange will always yield the earlier column on a given line.\n\nThis patch will become useful when we later pick between the two new\nresults in order to display the column number of the first match on a\nline with --column.\n\nCo-authored-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 58 +++++++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 39 insertions(+), 19 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 45ec7e636c..dedfe17f93 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1248,11 +1248,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \treturn hit;\n }\n \n-static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t   enum grep_context ctx, int collect_hits)\n+static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n+\t\t\t   char *eol, enum grep_context ctx, ssize_t *col,\n+\t\t\t   ssize_t *icol, int collect_hits)\n {\n \tint h = 0;\n-\tregmatch_t match;\n \n \tif (!x)\n \t\tdie(\"Not a valid grep expression\");\n@@ -1261,25 +1261,39 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\th = 1;\n \t\tbreak;\n \tcase GREP_NODE_ATOM:\n-\t\th = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);\n+\t\t{\n+\t\t\tregmatch_t tmp;\n+\t\t\th = match_one_pattern(x->u.atom, bol, eol, ctx,\n+\t\t\t\t\t      &tmp, 0);\n+\t\t\tif (h && (*col < 0 || tmp.rm_so < *col))\n+\t\t\t\t*col = tmp.rm_so;\n+\t\t}\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n+\t\t/*\n+\t\t * Upon visiting a GREP_NODE_NOT, col and icol become swapped.\n+\t\t */\n+\t\th = !match_expr_eval(opt, x->u.unary, bol, eol, ctx, icol, col,\n+\t\t\t\t     0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))\n+\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t     icol, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);\n+\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n \t\tif (!collect_hits)\n-\t\t\treturn (match_expr_eval(x->u.binary.left,\n-\t\t\t\t\t\tbol, eol, ctx, 0) ||\n-\t\t\t\tmatch_expr_eval(x->u.binary.right,\n-\t\t\t\t\t\tbol, eol, ctx, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);\n+\t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol,\n+\t\t\t\t\t\tctx, col, icol, 0) ||\n+\t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol,\n+\t\t\t\t\t\teol, ctx, col, icol, 0));\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n+\t\t\t\t    icol, 0);\n \t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);\n+\t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\t\t\t     icol, 1);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1290,25 +1304,30 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n }\n \n static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n-\t\t      enum grep_context ctx, int collect_hits)\n+\t\t      enum grep_context ctx, ssize_t *col,\n+\t\t      ssize_t *icol, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, collect_hits);\n+\treturn match_expr_eval(opt, x, bol, eol, ctx, col, icol, collect_hits);\n }\n \n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n+\t\t      ssize_t *col, ssize_t *icol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n-\tregmatch_t match;\n \n \tif (opt->extended)\n-\t\treturn match_expr(opt, bol, eol, ctx, collect_hits);\n+\t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n+\t\t\t\t  collect_hits);\n \n \t/* we do not call with collect_hits without being extended */\n \tfor (p = opt->pattern_list; p; p = p->next) {\n-\t\tif (match_one_pattern(p, bol, eol, ctx, &match, 0))\n+\t\tregmatch_t tmp;\n+\t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n+\t\t\t*col = tmp.rm_so;\n \t\t\treturn 1;\n+\t\t}\n \t}\n \treturn 0;\n }\n@@ -1763,6 +1782,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n \t\t * look_ahead() skips quickly to the line that possibly\n@@ -1786,7 +1806,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\tif ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\n \t\t\tctx = GREP_CONTEXT_BODY;\n \n-\t\thit = match_line(opt, bol, eol, ctx, collect_hits);\n+\t\thit = match_line(opt, bol, eol, &col, &icol, ctx, collect_hits);\n \t\t*eol = ch;\n \n \t\tif (collect_hits)\n-- \n2.18.0\n\n"},{"id":"350728","messageId":"a3971a1a44bfbac599e74a9b177b21ccd2bb9a95.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 3/7] grep.[ch]: extend grep_opt to allow showing matched column","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:39Z","receivedAt":"2018-06-22T15:49:46Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support showing the matched column when calling 'git-grep(1)', teach\n'grep_opt' the normal set of options to configure the default behavior\nand colorization of this feature.\n\nNow that we have opt->columnnum, use it to disable short-circuiting over\nORs and ANDs so that col and icol are always filled with the earliest\nmatches on each line. In addition, don't return the first match from\nmatch_line(), for the same reason.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 47 +++++++++++++++++++++++++++++++++++++----------\n grep.h |  2 ++\n 2 files changed, 39 insertions(+), 10 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex dedfe17f93..c885101017 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -46,6 +46,7 @@ void init_grep_defaults(void)\n \tcolor_set(opt->color_filename, \"\");\n \tcolor_set(opt->color_function, \"\");\n \tcolor_set(opt->color_lineno, \"\");\n+\tcolor_set(opt->color_columnno, \"\");\n \tcolor_set(opt->color_match_context, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_match_selected, GIT_COLOR_BOLD_RED);\n \tcolor_set(opt->color_selected, \"\");\n@@ -155,6 +156,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \topt->extended_regexp_option = def->extended_regexp_option;\n \topt->pattern_type_option = def->pattern_type_option;\n \topt->linenum = def->linenum;\n+\topt->columnnum = def->columnnum;\n \topt->max_depth = def->max_depth;\n \topt->pathname = def->pathname;\n \topt->relative = def->relative;\n@@ -164,6 +166,7 @@ void grep_init(struct grep_opt *opt, const char *prefix)\n \tcolor_set(opt->color_filename, def->color_filename);\n \tcolor_set(opt->color_function, def->color_function);\n \tcolor_set(opt->color_lineno, def->color_lineno);\n+\tcolor_set(opt->color_columnno, def->color_columnno);\n \tcolor_set(opt->color_match_context, def->color_match_context);\n \tcolor_set(opt->color_match_selected, def->color_match_selected);\n \tcolor_set(opt->color_selected, def->color_selected);\n@@ -1277,23 +1280,36 @@ static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,\n \t\t\t\t     0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n-\t\t\t\t     icol, 0))\n-\t\t\treturn 0;\n-\t\th = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n+\t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n+\t\tif (h || opt->columnnum) {\n+\t\t\t/*\n+\t\t\t * Don't short-circuit AND when given --column, since a\n+\t\t\t * NOT earlier in the tree may turn this into an OR. In\n+\t\t\t * this case, see the below comment.\n+\t\t\t */\n+\t\t\th &= match_expr_eval(opt, x->u.binary.right, bol, eol,\n+\t\t\t\t\t     ctx, col, icol, 0);\n+\t\t}\n \t\tbreak;\n \tcase GREP_NODE_OR:\n-\t\tif (!collect_hits)\n+\t\tif (!(collect_hits || opt->columnnum)) {\n+\t\t\t/*\n+\t\t\t * Don't short-circuit OR when given --column (or\n+\t\t\t * collecting hits) to ensure we don't skip a later\n+\t\t\t * child that would produce an earlier match.\n+\t\t\t */\n \t\t\treturn (match_expr_eval(opt, x->u.binary.left, bol, eol,\n \t\t\t\t\t\tctx, col, icol, 0) ||\n \t\t\t\tmatch_expr_eval(opt, x->u.binary.right, bol,\n \t\t\t\t\t\teol, ctx, col, icol, 0));\n+\t\t}\n \t\th = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,\n \t\t\t\t    icol, 0);\n-\t\tx->u.binary.left->hit |= h;\n+\t\tif (collect_hits)\n+\t\t\tx->u.binary.left->hit |= h;\n \t\th |= match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,\n-\t\t\t\t     icol, 1);\n+\t\t\t\t     icol, collect_hits);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -1316,6 +1332,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t      enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n+\tint hit = 0;\n \n \tif (opt->extended)\n \t\treturn match_expr(opt, bol, eol, ctx, col, icol,\n@@ -1325,11 +1342,21 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,\n \tfor (p = opt->pattern_list; p; p = p->next) {\n \t\tregmatch_t tmp;\n \t\tif (match_one_pattern(p, bol, eol, ctx, &tmp, 0)) {\n-\t\t\t*col = tmp.rm_so;\n-\t\t\treturn 1;\n+\t\t\thit |= 1;\n+\t\t\tif (!opt->columnnum) {\n+\t\t\t\t/*\n+\t\t\t\t * Without --column, any single match on a line\n+\t\t\t\t * is enough to know that it needs to be\n+\t\t\t\t * printed. With --column, scan _all_ patterns\n+\t\t\t\t * to find the earliest.\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (*col < 0 || tmp.rm_so < *col)\n+\t\t\t\t*col = tmp.rm_so;\n \t\t}\n \t}\n-\treturn 0;\n+\treturn hit;\n }\n \n static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\ndiff --git a/grep.h b/grep.h\nindex 399381c908..08a0b391c5 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -127,6 +127,7 @@ struct grep_opt {\n \tint prefix_length;\n \tregex_t regexp;\n \tint linenum;\n+\tint columnnum;\n \tint invert;\n \tint ignore_case;\n \tint status_only;\n@@ -159,6 +160,7 @@ struct grep_opt {\n \tchar color_filename[COLOR_MAXLEN];\n \tchar color_function[COLOR_MAXLEN];\n \tchar color_lineno[COLOR_MAXLEN];\n+\tchar color_columnno[COLOR_MAXLEN];\n \tchar color_match_context[COLOR_MAXLEN];\n \tchar color_match_selected[COLOR_MAXLEN];\n \tchar color_selected[COLOR_MAXLEN];\n-- \n2.18.0\n\n"},{"id":"350729","messageId":"4256d38228a9122f6962d40372c88dfc199507b9.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 4/7] grep.c: display column number of first match","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:42Z","receivedAt":"2018-06-22T15:49:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To prepare for 'git grep' learning '--column', teach grep.c's\nshow_line() how to show the column of the first match on non-context\nlines.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n grep.c | 33 ++++++++++++++++++++++++++++-----\n 1 file changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex c885101017..83fe32a6a0 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1405,7 +1405,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\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+\t\t      const char *name, unsigned lno, ssize_t cno, char sign)\n {\n \tint rest = eol - bol;\n \tconst char *match_color, *line_color = NULL;\n@@ -1440,6 +1440,17 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\toutput_color(opt, buf, strlen(buf), opt->color_lineno);\n \t\toutput_sep(opt, sign);\n \t}\n+\t/*\n+\t * Treat 'cno' as the 1-indexed offset from the start of a non-context\n+\t * line to its first match. Otherwise, 'cno' is 0 indicating that we are\n+\t * being called with a context line.\n+\t */\n+\tif (opt->columnnum && cno) {\n+\t\tchar buf[32];\n+\t\txsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n+\t\toutput_color(opt, buf, strlen(buf), opt->color_columnno);\n+\t\toutput_sep(opt, sign);\n+\t}\n \tif (opt->color) {\n \t\tregmatch_t match;\n \t\tenum grep_context ctx = GREP_CONTEXT_BODY;\n@@ -1545,7 +1556,7 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,\n \t\t\tbreak;\n \n \t\tif (match_funcname(opt, gs, bol, eol)) {\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '=');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, 0, '=');\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1610,7 +1621,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \n \t\twhile (*eol != '\\n')\n \t\t\teol++;\n-\t\tshow_line(opt, bol, eol, gs->name, cur, sign);\n+\t\tshow_line(opt, bol, eol, gs->name, cur, 0, sign);\n \t\tbol = eol + 1;\n \t\tcur++;\n \t}\n@@ -1809,6 +1820,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \twhile (left) {\n \t\tchar *eol, ch;\n \t\tint hit;\n+\t\tssize_t cno;\n \t\tssize_t col = -1, icol = -1;\n \n \t\t/*\n@@ -1874,7 +1886,18 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t\tshow_pre_context(opt, gs, bol, eol, lno);\n \t\t\telse if (opt->funcname)\n \t\t\t\tshow_funcname_line(opt, gs, bol, lno);\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, ':');\n+\t\t\tcno = opt->invert ? icol : col;\n+\t\t\tif (cno < 0) {\n+\t\t\t\t/*\n+\t\t\t\t * A negative cno indicates that there was no\n+\t\t\t\t * match on the line. We are thus inverted and\n+\t\t\t\t * being asked to show all lines that _don't_\n+\t\t\t\t * match a given expression. Therefore, set cno\n+\t\t\t\t * to 0 to suggest the whole line matches.\n+\t\t\t\t */\n+\t\t\t\tcno = 0;\n+\t\t\t}\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, cno + 1, ':');\n \t\t\tlast_hit = lno;\n \t\t\tif (opt->funcbody)\n \t\t\t\tshow_function = 1;\n@@ -1903,7 +1926,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\t\t/* If the last hit is within the post context,\n \t\t\t * we need to show this line.\n \t\t\t */\n-\t\t\tshow_line(opt, bol, eol, gs->name, lno, '-');\n+\t\t\tshow_line(opt, bol, eol, gs->name, lno, col + 1, '-');\n \t\t}\n \n \tnext_line:\n-- \n2.18.0\n\n"},{"id":"350730","messageId":"6d561ab546aff5d45d72039c928471c44a4672c8.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:45Z","receivedAt":"2018-06-22T15:49:52Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Teach 'git-grep(1)' a new option, '--column', to show the column\nnumber of the first match on a non-context line. This makes it possible\nto teach 'contrib/git-jump/git-jump' how to seek to the first matching\nposition of a grep match in your editor, and allows similar additional\nscripting capabilities.\n\nFor example:\n\n  $ git grep -n --column foo | head -n3\n  .clang-format:51:14:# myFunction(foo, bar, baz);\n  .clang-format:64:7:# int foo();\n  .clang-format:75:8:# void foo()\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-grep.txt |  6 ++-\n builtin/grep.c             |  1 +\n t/t7810-grep.sh            | 95 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 101 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 312409a607..31dc0392a6 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t   [-v | --invert-match] [-h|-H] [--full-name]\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-P | --perl-regexp]\n-\t   [-F | --fixed-strings] [-n | --line-number]\n+\t   [-F | --fixed-strings] [-n | --line-number] [--column]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n \t   [(-O | --open-files-in-pager) [<pager>]]\n \t   [-z | --null]\n@@ -169,6 +169,10 @@ providing this option will cause it to die.\n --line-number::\n \tPrefix the line number to matching lines.\n \n+--column::\n+\tPrefix the 1-indexed byte-offset of the first match from the start of the\n+\tmatching line.\n+\n -l::\n --files-with-matches::\n --name-only::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex ee753a403e..61bcaf6e58 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -828,6 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t    GREP_PATTERN_TYPE_PCRE),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('n', \"line-number\", &opt.linenum, N_(\"show line numbers\")),\n+\t\tOPT_BOOL(0, \"column\", &opt.columnnum, N_(\"show column number of first match\")),\n \t\tOPT_NEGBIT('h', NULL, &opt.pathname, N_(\"don't show filenames\"), 1),\n \t\tOPT_BIT('H', NULL, &opt.pathname, N_(\"show filenames\"), 1),\n \t\tOPT_NEGBIT(0, \"full-name\", &opt.relative,\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 1797f632a3..9312c8daf5 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -99,6 +99,101 @@ do\n \t\ttest_cmp expected actual\n \t'\n \n+\ttest_expect_success \"grep -w $L (with --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, extended OR)\" '\n+\t\t{\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:19:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e mmap$ --or -e baz $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, --invert)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap\n+\t\t\techo ${HC}file:1:foo mmap bar_mmap\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -w -e baz $H -- file >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, --invert, extended OR)\" '\n+\t\t{\n+\t\t\techo ${HC}hello_world:6:HeLLo_world\n+\t\t} >expected &&\n+\t\tgit grep --column --invert -e ll --or --not -e _ $H -- hello_world \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, --invert, extended AND)\" '\n+\t\t{\n+\t\t\techo ${HC}hello_world:3:Hello world\n+\t\t\techo ${HC}hello_world:3:Hello_world\n+\t\t\techo ${HC}hello_world:6:HeLLo_world\n+\t\t} >expected &&\n+\t\tgit grep --column --invert --not -e _ --and --not -e ll $H -- hello_world \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L (with --column, double-negation)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column --not \\( --not -e foo --or --not -e baz \\) $H -- file \\\n+\t\t\t>actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --column, -C)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file-foo_mmap bar\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -C1 -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with --line-number, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:1:5:foo mmap bar\n+\t\t\techo ${HC}file:3:14:foo_mmap bar mmap\n+\t\t\techo ${HC}file:4:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:5:14:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep -n --column -w -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -w $L (with non-extended patterns, --column)\" '\n+\t\t{\n+\t\t\techo ${HC}file:5:foo mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap\n+\t\t\techo ${HC}file:5:foo mmap bar_mmap\n+\t\t\techo ${HC}file:10:foo_mmap bar mmap baz\n+\t\t} >expected &&\n+\t\tgit grep --column -w -e bar -e mmap $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success \"grep -w $L\" '\n \t\t{\n \t\t\techo ${HC}file:1:foo mmap bar\n-- \n2.18.0\n\n"},{"id":"350731","messageId":"81c077c86d9e538a8ee094252a7b8e6beb05e224.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 7/7] contrib/git-jump/git-jump: jump to exact location","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:54Z","receivedAt":"2018-06-22T15:50:01Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Take advantage of 'git-grep(1)''s new option, '--column' in order to\nteach Peff's 'git-jump' script how to jump to the correct column for any\ngiven match.\n\n'git-grep(1)''s output is in the correct format for Vim's jump list, so\nno additional cleanup is necessary.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n contrib/git-jump/README   | 12 ++++++++++--\n contrib/git-jump/git-jump |  2 +-\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/git-jump/README b/contrib/git-jump/README\nindex 4484bda410..2f618a7f97 100644\n--- a/contrib/git-jump/README\n+++ b/contrib/git-jump/README\n@@ -25,6 +25,13 @@ git-jump will feed this to the editor:\n foo.c:2: printf(\"hello word!\\n\");\n -----------------------------------\n \n+Or, when running 'git jump grep', column numbers will also be emitted,\n+e.g. `git jump grep \"hello\"` would return:\n+\n+-----------------------------------\n+foo.c:2:9: printf(\"hello word!\\n\");\n+-----------------------------------\n+\n Obviously this trivial case isn't that interesting; you could just open\n `foo.c` yourself. But when you have many changes scattered across a\n project, you can use the editor's support to \"jump\" from point to point.\n@@ -35,7 +42,8 @@ Git-jump can generate four types of interesting lists:\n \n   2. The beginning of any merge conflict markers.\n \n-  3. Any grep matches.\n+  3. Any grep matches, including the column of the first match on a\n+     line.\n \n   4. Any whitespace errors detected by `git diff --check`.\n \n@@ -82,7 +90,7 @@ which does something similar to `git jump grep`. However, it is limited\n to positioning the cursor to the correct line in only the first file,\n leaving you to locate subsequent hits in that file or other files using\n the editor or pager. By contrast, git-jump provides the editor with a\n-complete list of files and line numbers for each match.\n+complete list of files, lines, and a column number for each match.\n \n \n Limitations\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 80ab0590bc..931b0fe3a9 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -52,7 +52,7 @@ mode_merge() {\n # editor shows them to us in the status bar.\n mode_grep() {\n \tcmd=$(git config jump.grepCmd)\n-\ttest -n \"$cmd\" || cmd=\"git grep -n\"\n+\ttest -n \"$cmd\" || cmd=\"git grep -n --column\"\n \t$cmd \"$@\" |\n \tperl -pe '\n \ts/[ \\t]+/ /g;\n-- \n2.18.0\n"},{"id":"350732","messageId":"63ad350585a265995b1f16d854c4f1d1df83dd24.1529682173.git.me@ttaylorr.com","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"[PATCH v3 6/7] grep.c: add configuration variables to show matched option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-22T15:49:49Z","receivedAt":"2018-06-22T15:50:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"To support git-grep(1)'s new option, '--column', document and teach\ngrep.c how to interpret relevant configuration options, similar to those\nassociated with '--line-number'.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/config.txt   | 5 +++++\n Documentation/git-grep.txt | 3 +++\n grep.c                     | 6 ++++++\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 58fde4daea..e4cbed3078 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1183,6 +1183,8 @@ color.grep.<slot>::\n \tfunction name lines (when using `-p`)\n `lineNumber`;;\n \tline number prefix (when using `-n`)\n+`column`;;\n+\tcolumn number prefix (when using `--column`)\n `match`;;\n \tmatching text (same as setting `matchContext` and `matchSelected`)\n `matchContext`;;\n@@ -1797,6 +1799,9 @@ gitweb.snapshot::\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 31dc0392a6..0de3493b80 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -44,6 +44,9 @@ CONFIGURATION\n grep.lineNumber::\n \tIf set to true, enable `-n` option by default.\n \n+grep.column::\n+\tIf set to true, enable the `--column` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/grep.c b/grep.c\nindex 83fe32a6a0..992673fe7e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -96,6 +96,10 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\topt->linenum = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"grep.column\")) {\n+\t\topt->columnnum = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"grep.fullname\")) {\n \t\topt->relative = !git_config_bool(var, value);\n@@ -112,6 +116,8 @@ int grep_config(const char *var, const char *value, void *cb)\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.column\"))\n+\t\tcolor = opt->color_columnno;\n \telse if (!strcmp(var, \"color.grep.matchcontext\"))\n \t\tcolor = opt->color_match_context;\n \telse if (!strcmp(var, \"color.grep.matchselected\"))\n-- \n2.18.0\n\n"},{"id":"350768","messageId":"nycvar.QRO.7.76.6.1806222344280.11870@tvgsbejvaqbjf.bet","threadId":"48734","inReplyTo":"20180621120159.GC15293@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-06-22T21:45:09Z","receivedAt":"2018-06-22T21:45:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 21 Jun 2018, Jeff King wrote:\n\n> On Thu, Jun 21, 2018 at 07:53:02AM -0400, Jeff King wrote:\n> \n> > > @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n> > >  \t */\n> > >  \tif (opt->columnnum && cno) {\n> > >  \t\tchar buf[32];\n> > > -\t\txsnprintf(buf, sizeof(buf), \"%d\", cno);\n> > > +\t\txsnprintf(buf, sizeof(buf), \"%zu\", cno);\n> > \n> > Unfortunately %z isn't portable. You have to use:\n> > \n> >   xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, (uintmax_t)cno);\n> \n> To clarify: yes, it is in C99. But traditionally we have not required\n> that.\n> \n> This might be a candidate for another \"weather balloon\" patch to see if\n> anybody complains, though. The last time time we dealt with this in a\n> major way was over 7 years ago in 28bd70d811 (unbreak and eliminate\n> NO_C99_FORMAT, 2011-03-16).\n> \n> I know Johannes switched out some \"%lu\" for PRIuMAX as recently as last\n> August[1], but I think that is more about the Windows size_t not matching\n> \"unsigned long\", and the decision to use PRIuMAX was to match the\n> existing codebase. AFAIK %zu is available on Windows.\n\nNope, it's not available:\n\ngit.c: In function 'cmd_main':\ngit.c:733:10: error: unknown conversion type character 'z' in format [-Werror=format=]\n die(\"x: %z\", (void *)(intptr_t)0x123456789a);\n\nCiao,\nDscho\n"},{"id":"350769","messageId":"20180622222613.GA4305@sigill.intra.peff.net","threadId":"48734","inReplyTo":"nycvar.QRO.7.76.6.1806222344280.11870@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-22T22:26:13Z","receivedAt":"2018-06-22T22:26:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 22, 2018 at 11:45:09PM +0200, Johannes Schindelin wrote:\n\n> > This might be a candidate for another \"weather balloon\" patch to see if\n> > anybody complains, though. The last time time we dealt with this in a\n> > major way was over 7 years ago in 28bd70d811 (unbreak and eliminate\n> > NO_C99_FORMAT, 2011-03-16).\n> > \n> > I know Johannes switched out some \"%lu\" for PRIuMAX as recently as last\n> > August[1], but I think that is more about the Windows size_t not matching\n> > \"unsigned long\", and the decision to use PRIuMAX was to match the\n> > existing codebase. AFAIK %zu is available on Windows.\n> \n> Nope, it's not available:\n> \n> git.c: In function 'cmd_main':\n> git.c:733:10: error: unknown conversion type character 'z' in format [-Werror=format=]\n>  die(\"x: %z\", (void *)(intptr_t)0x123456789a);\n\nWell, that resolve that, then. :)\n\nThanks for letting us know before we went down a dead-end.\n\n-Peff\n"},{"id":"350879","messageId":"20180625184350.GA26688@sigill.intra.peff.net","threadId":"48734","inReplyTo":"cover.1529682173.git.me@ttaylorr.com","subject":"Re: [PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-25T18:43:50Z","receivedAt":"2018-06-25T18:43:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 22, 2018 at 10:49:26AM -0500, Taylor Blau wrote:\n\n> Attached is my third--anticipate the final--re-roll of my series to\n> teach 'git grep --column'.\n\nYou know when you say that it jinxes it, right? :)\n\n> Since the last time, only a couple of things have changed at Peff's\n> suggestions in [1]. The changes are summarized here, and an inter-diff\n> is available below:\n> \n>   - Change \"%zu\" to PRIuMAX (and an appropriate cast into uintmax_t). I\n>     plan to send a follow-up patch to convert this back to \"%zu\" to see\n>     how people feel about it, but I wanted to keep that out of the\n>     present series in order to not hold things up.\n> \n>   - Don't short-circuit AND when given --column, since an earlier NOT\n>     higher in the tree may cause an AND to be converted into an OR via\n>     de Morgan's Law, in which case the problem is reduced to the OR case\n>     (and should not have been short-circuited in the first place).\n> \n>   - Add a test in t7810 to cover this behavior (i.e., '--not \\( -e x\n>     --and -e y \\)').\n\nJinxes aside, this interdiff looks good to me.\n\n-Peff\n"},{"id":"350880","messageId":"20180625184729.GA12140@syl.local","threadId":"48734","inReplyTo":"20180625184350.GA26688@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-06-25T18:47:29Z","receivedAt":"2018-06-25T18:47:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jun 25, 2018 at 02:43:50PM -0400, Jeff King wrote:\n> On Fri, Jun 22, 2018 at 10:49:26AM -0500, Taylor Blau wrote:\n> > Since the last time, only a couple of things have changed at Peff's\n> > suggestions in [1]. The changes are summarized here, and an inter-diff\n> > is available below:\n> >\n> >   - Change \"%zu\" to PRIuMAX (and an appropriate cast into uintmax_t). I\n> >     plan to send a follow-up patch to convert this back to \"%zu\" to see\n> >     how people feel about it, but I wanted to keep that out of the\n> >     present series in order to not hold things up.\n> >\n> >   - Don't short-circuit AND when given --column, since an earlier NOT\n> >     higher in the tree may cause an AND to be converted into an OR via\n> >     de Morgan's Law, in which case the problem is reduced to the OR case\n> >     (and should not have been short-circuited in the first place).\n> >\n> >   - Add a test in t7810 to cover this behavior (i.e., '--not \\( -e x\n> >     --and -e y \\)').\n>\n> Jinxes aside, this interdiff looks good to me.\n\nThanks; I hope that I haven't jinxed anything :-).\n\nI'm going to avoid sending the PRIuMAX -> \"%zu\" patch, since dscho\npoints out that it's not available on Windows [1].\n\nThanks,\nTaylor\n\n[1]: https://public-inbox.org/git/nycvar.QRO.7.76.6.1806222344280.11870@tvgsbejvaqbjf.bet/\n"},{"id":"350979","messageId":"xmqqa7rhmr7d.fsf@gitster-ct.c.googlers.com","threadId":"48734","inReplyTo":"20180625184729.GA12140@syl.local","subject":"Re: [PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-26T16:45:58Z","receivedAt":"2018-06-26T16:46:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Mon, Jun 25, 2018 at 02:43:50PM -0400, Jeff King wrote:\n>> On Fri, Jun 22, 2018 at 10:49:26AM -0500, Taylor Blau wrote:\n>> > Since the last time, only a couple of things have changed at Peff's\n>> > suggestions in [1]. The changes are summarized here, and an inter-diff\n>> > is available below:\n>> >\n>> >   - Change \"%zu\" to PRIuMAX (and an appropriate cast into uintmax_t). I\n>> >     plan to send a follow-up patch to convert this back to \"%zu\" to see\n>> >     how people feel about it, but I wanted to keep that out of the\n>> >     present series in order to not hold things up.\n>> ...\n>> Jinxes aside, this interdiff looks good to me.\n>\n> Thanks; I hope that I haven't jinxed anything :-).\n>\n> I'm going to avoid sending the PRIuMAX -> \"%zu\" patch, since dscho\n> points out that it's not available on Windows [1].\n\nOK, so what I queued on 'pu' seems to be ready to advance, which is\ngood.  Keeping topics in flight on 'pu', unable to convince myself\nthat they are ready to advance to 'next', makes me feel uneasy and\nunhappy, and having to worry about one less such topic is a good\nnews ;-)\n\n"}]}