git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: avarab@gmail.com, peff@peff.net, gitster@pobox.com
Subject: [PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)'
Date: Fri, 22 Jun 2018 10:49:26 -0500	[thread overview]
Message-ID: <cover.1529682173.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1529365072.git.me@ttaylorr.com>

Hi,

Attached is my third--anticipate the final--re-roll of my series to
teach 'git grep --column'.

Since the last time, only a couple of things have changed at Peff's
suggestions in [1]. The changes are summarized here, and an inter-diff
is available below:

  - Change "%zu" to PRIuMAX (and an appropriate cast into uintmax_t). I
    plan to send a follow-up patch to convert this back to "%zu" to see
    how people feel about it, but I wanted to keep that out of the
    present series in order to not hold things up.

  - Don't short-circuit AND when given --column, since an earlier NOT
    higher in the tree may cause an AND to be converted into an OR via
    de Morgan's Law, in which case the problem is reduced to the OR case
    (and should not have been short-circuited in the first place).

  - Add a test in t7810 to cover this behavior (i.e., '--not \( -e x
    --and -e y \)').

Thanks,
Taylor

[1]: https://public-inbox.org/git/20180621115302.GB15293@sigill.intra.peff.net/

Taylor Blau (7):
  Documentation/config.txt: camel-case lineNumber for consistency
  grep.c: expose {,inverted} match column in match_line()
  grep.[ch]: extend grep_opt to allow showing matched column
  grep.c: display column number of first match
  builtin/grep.c: add '--column' option to 'git-grep(1)'
  grep.c: add configuration variables to show matched option
  contrib/git-jump/git-jump: jump to exact location

 Documentation/config.txt   |   7 +-
 Documentation/git-grep.txt |   9 ++-
 builtin/grep.c             |   1 +
 contrib/git-jump/README    |  12 +++-
 contrib/git-jump/git-jump  |   2 +-
 grep.c                     | 134 +++++++++++++++++++++++++++++--------
 grep.h                     |   2 +
 t/t7810-grep.sh            |  95 ++++++++++++++++++++++++++
 8 files changed, 228 insertions(+), 34 deletions(-)

Inter-diff (since: v2)

diff --git a/grep.c b/grep.c
index 08d3df2855..992673fe7e 100644
--- a/grep.c
+++ b/grep.c
@@ -1286,11 +1286,17 @@ static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x, char *bol,
 				     0);
 		break;
 	case GREP_NODE_AND:
-		if (!match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
-				     icol, 0))
-			return 0;
-		h = match_expr_eval(opt, x->u.binary.right, bol, eol, ctx, col,
+		h = match_expr_eval(opt, x->u.binary.left, bol, eol, ctx, col,
 				    icol, 0);
+		if (h || opt->columnnum) {
+			/*
+			 * Don't short-circuit AND when given --column, since a
+			 * NOT earlier in the tree may turn this into an OR. In
+			 * this case, see the below comment.
+			 */
+			h &= match_expr_eval(opt, x->u.binary.right, bol, eol,
+					     ctx, col, icol, 0);
+		}
 		break;
 	case GREP_NODE_OR:
 		if (!(collect_hits || opt->columnnum)) {
@@ -1447,7 +1453,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	 */
 	if (opt->columnnum && cno) {
 		char buf[32];
-		xsnprintf(buf, sizeof(buf), "%zu", cno);
+		xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno);
 		output_color(opt, buf, strlen(buf), opt->color_columnno);
 		output_sep(opt, sign);
 	}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index bf0b572dab..9312c8daf5 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -110,7 +110,7 @@ do
 		test_cmp expected actual
 	'

-	test_expect_success "grep -w $L (with --column, extended)" '
+	test_expect_success "grep -w $L (with --column, extended OR)" '
 		{
 			echo ${HC}file:14:foo_mmap bar mmap
 			echo ${HC}file:19:foo_mmap bar mmap baz
@@ -130,7 +130,7 @@ do
 		test_cmp expected actual
 	'

-	test_expect_success "grep $L (with --column, --invert, extended)" '
+	test_expect_success "grep $L (with --column, --invert, extended OR)" '
 		{
 			echo ${HC}hello_world:6:HeLLo_world
 		} >expected &&
@@ -139,6 +139,17 @@ do
 		test_cmp expected actual
 	'

+	test_expect_success "grep $L (with --column, --invert, extended AND)" '
+		{
+			echo ${HC}hello_world:3:Hello world
+			echo ${HC}hello_world:3:Hello_world
+			echo ${HC}hello_world:6:HeLLo_world
+		} >expected &&
+		git grep --column --invert --not -e _ --and --not -e ll $H -- hello_world \
+			>actual &&
+		test_cmp expected actual
+	'
+
 	test_expect_success "grep $L (with --column, double-negation)" '
 		{
 			echo ${HC}file:1:foo_mmap bar mmap baz

--
2.18.0

  parent reply	other threads:[~2018-06-22 15:49 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-18 23:43 [PATCH 0/7] grep.c: teach --column to 'git-grep(1)' Taylor Blau
2018-06-18 23:43 ` [PATCH 1/7] Documentation/config.txt: camel-case lineNumber for consistency Taylor Blau
2018-06-18 23:43 ` [PATCH 2/7] grep.c: expose {,inverted} match column in match_line() Taylor Blau
2018-06-19 16:49   ` Junio C Hamano
2018-06-19 17:02     ` Taylor Blau
2018-06-18 23:43 ` [PATCH 3/7] grep.[ch]: extend grep_opt to allow showing matched column Taylor Blau
2018-06-18 23:43 ` [PATCH 4/7] grep.c: display column number of first match Taylor Blau
2018-06-19 16:28   ` Jeff King
2018-06-19 16:34     ` Taylor Blau
2018-06-18 23:43 ` [PATCH 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)' Taylor Blau
2018-06-18 23:43 ` [PATCH 6/7] grep.c: add configuration variables to show matched option Taylor Blau
2018-06-18 23:43 ` [PATCH 7/7] contrib/git-jump/git-jump: jump to exact location Taylor Blau
2018-06-19 16:35 ` [PATCH 0/7] grep.c: teach --column to 'git-grep(1)' Jeff King
2018-06-19 17:33   ` René Scharfe
2018-06-19 17:44     ` Taylor Blau
2018-06-19 17:50       ` René Scharfe
2018-06-19 20:26       ` René Scharfe
2018-06-19 17:48     ` Jeff King
2018-06-19 17:54       ` Taylor Blau
2018-06-19 17:58       ` Junio C Hamano
2018-06-19 18:02         ` Taylor Blau
2018-06-19 18:05         ` Jeff King
2018-06-19 18:09           ` Junio C Hamano
2018-06-19 18:50       ` René Scharfe
2018-06-19 19:11         ` Jeff King
2018-06-19 20:34           ` René Scharfe
2018-06-19 20:51             ` Junio C Hamano
2018-06-19 16:46 ` Junio C Hamano
2018-06-19 17:02   ` Taylor Blau
2018-06-19 22:51 ` Taylor Blau
2018-06-20 20:05 ` [PATCH v2 " Taylor Blau
2018-06-20 20:05   ` [PATCH v2 1/7] Documentation/config.txt: camel-case lineNumber for consistency Taylor Blau
2018-06-20 20:05   ` [PATCH v2 2/7] grep.c: expose {,inverted} match column in match_line() Taylor Blau
2018-06-20 20:05   ` [PATCH v2 3/7] grep.[ch]: extend grep_opt to allow showing matched column Taylor Blau
2018-06-20 20:05   ` [PATCH v2 4/7] grep.c: display column number of first match Taylor Blau
2018-06-20 20:05   ` [PATCH v2 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)' Taylor Blau
2018-06-20 20:05   ` [PATCH v2 6/7] grep.c: add configuration variables to show matched option Taylor Blau
2018-06-20 20:05   ` [PATCH v2 7/7] contrib/git-jump/git-jump: jump to exact location Taylor Blau
2018-06-21 11:53   ` [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)' Jeff King
2018-06-21 12:01     ` Jeff King
2018-06-22 21:45       ` Johannes Schindelin
2018-06-22 22:26         ` Jeff King
2018-06-21 20:52     ` Junio C Hamano
2018-06-21 21:45     ` Taylor Blau
2018-06-22  7:22       ` Jeff King
2018-06-22 15:49 ` Taylor Blau [this message]
2018-06-22 15:49   ` [PATCH v3 1/7] Documentation/config.txt: camel-case lineNumber for consistency Taylor Blau
2018-06-22 15:49   ` [PATCH v3 2/7] grep.c: expose {,inverted} match column in match_line() Taylor Blau
2018-06-22 15:49   ` [PATCH v3 3/7] grep.[ch]: extend grep_opt to allow showing matched column Taylor Blau
2018-06-22 15:49   ` [PATCH v3 4/7] grep.c: display column number of first match Taylor Blau
2018-06-22 15:49   ` [PATCH v3 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)' Taylor Blau
2018-06-22 15:49   ` [PATCH v3 6/7] grep.c: add configuration variables to show matched option Taylor Blau
2018-06-22 15:49   ` [PATCH v3 7/7] contrib/git-jump/git-jump: jump to exact location Taylor Blau
2018-06-25 18:43   ` [PATCH v3 0/7] grep.c: teach --column to 'git-grep(1)' Jeff King
2018-06-25 18:47     ` Taylor Blau
2018-06-26 16:45       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1529682173.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).