git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] pickaxe: use textconv for -S counting

From
Jeff King <peff@peff.net>
Date
Oct 28, 2012, 12:47 UTC
Message-ID
<20121028124701.GB24548@sigill.intra.peff.net>
In-Reply-To
<20121028124540.GF11434@sigill.intra.peff.net>

We currently just look at raw blob data when using "-S" to pickaxe. This is mostly historical, as pickaxe predates the textconv feature. If the user has bothered to define a textconv filter, it is more likely that their search string will be on the textconv output, as that is what they will see in the diff (and we do not even provide a mechanism for them to search for binary needles that contain NUL characters).

This patch teaches "-S" to use textconv, just as we already do for "-G".

Signed-off-by: Jeff King <peff@peff.net>
---
 diffcore-pickaxe.c       | 56 +++++++++++++++++++++++++++++++++---------------
 t/t4030-diff-textconv.sh | 12 +++++++++++
 2 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 61f628c..b097fa7 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -157,17 +157,15 @@ static unsigned int contains(struct diff_filespec *one, struct diff_options *o,
 	return;
 }
 
-static unsigned int contains(struct diff_filespec *one, struct diff_options *o,
+static unsigned int contains(mmfile_t *mf, struct diff_options *o,
 			     regex_t *regexp, kwset_t kws)
 {
 	unsigned int cnt;
 	unsigned long sz;
 	const char *data;
-	if (diff_populate_filespec(one, 0))
-		return 0;
 
-	sz = one->size;
-	data = one->data;
+	sz = mf->size;
+	data = mf->ptr;
 	cnt = 0;
 
 	if (regexp) {
@@ -197,29 +195,53 @@ static int has_changes(struct diff_filepair *p, struct diff_options *o,
 			cnt++;
 		}
 	}
-	diff_free_filespec_data(one);
 	return cnt;
 }
 
 static int has_changes(struct diff_filepair *p, struct diff_options *o,
 		       regex_t *regexp, kwset_t kws)
 {
+	struct userdiff_driver *textconv_one = get_textconv(p->one);
+	struct userdiff_driver *textconv_two = get_textconv(p->two);
+	mmfile_t mf1, mf2;
+	int ret;
+
 	if (!o->pickaxe[0])
 		return 0;
 
-	if (!DIFF_FILE_VALID(p->one)) {
-		if (!DIFF_FILE_VALID(p->two))
-			return 0; /* ignore unmerged */
+	/*
+	 * If we have an unmodified pair, we know that the count will be the
+	 * same and don't even have to load the blobs. Unless textconv is in
+	 * play, _and_ we are using two different textconv filters (e.g.,
+	 * because a pair is an exact rename with different textconv attributes
+	 * for each side, which might generate different content).
+	 */
+	if (textconv_one == textconv_two && diff_unmodified_pair(p))
+		return 0;
+
+	fill_one(p->one, &mf1, &textconv_one);
+	fill_one(p->two, &mf2, &textconv_two);
+
+	if (!mf1.ptr) {
+		if (!mf2.ptr)
+			ret = 0; /* ignore unmerged */
 		/* created */
-		return contains(p->two, o, regexp, kws) != 0;
-	}
-	if (!DIFF_FILE_VALID(p->two))
-		return contains(p->one, o, regexp, kws) != 0;
-	if (!diff_unmodified_pair(p)) {
-		return contains(p->one, o, regexp, kws) !=
-		       contains(p->two, o, regexp, kws);
+		ret = contains(&mf2, o, regexp, kws) != 0;
 	}
-	return 0;
+	else if (!mf2.ptr) /* removed */
+		ret = contains(&mf1, o, regexp, kws) != 0;
+	else
+		ret = contains(&mf1, o, regexp, kws) !=
+		      contains(&mf2, o, regexp, kws);
+
+	if (textconv_one)
+		free(mf1.ptr);
+	if (textconv_two)
+		free(mf2.ptr);
+	diff_free_filespec_data(p->one);
+	diff_free_filespec_data(p->two);
+
+	return ret;
 }
 
 static void diffcore_pickaxe_count(struct diff_options *o)
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 461d27a..53ec330 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -96,6 +96,18 @@ test_expect_success 'grep-diff (-G) operates on textconv data (modification)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'pickaxe (-S) operates on textconv data (add)' '
+	echo one >expect &&
+	git log --root --format=%s -S0 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pickaxe (-S) operates on textconv data (modification)' '
+	echo two >expect &&
+	git log --root --format=%s -S1 >actual &&
+	test_cmp expect actual
+'
+
 cat >expect.stat <<'EOF'
  file | Bin 2 -> 4 bytes
  1 file changed, 0 insertions(+), 0 deletions(-)
-- 
1.8.0.3.g3456896
Previous: Jeff KingNext: Junio C Hamano
Message 5 of 24 in “crash on git diff-tree -Ganything <tree> for new files with textconv filter”
  1. Peter OberndorferOct 27, 2012
  2. Jeff KingOct 28, 2012
  3. 0/2 textconv support for "log -S"Jeff King, Oct 28, 2012
  4. 1/2 pickaxe: hoist empty needle checkJeff King, Oct 28, 2012
  5. 2/2 pickaxe: use textconv for -S countingJeff King, Oct 28, 2012
  6. Junio C HamanoNov 13, 2012
  7. Jeff KingNov 15, 2012
  8. Junio C HamanoNov 20, 2012
  9. Junio C HamanoNov 20, 2012
  10. Jeff KingNov 21, 2012
  11. Peter OberndorferOct 28, 2012
  12. Jeff KingOct 29, 2012
  13. Jeff KingOct 29, 2012
  14. Peter OberndorferOct 29, 2012
  15. Jeff KingOct 29, 2012
  16. Jeff KingOct 29, 2012
  17. Jeff KingOct 30, 2012
  18. Junio C HamanoOct 30, 2012
  19. Jeff KingOct 30, 2012
  20. Ramsay JonesNov 1, 2012
  21. Peter OberndorferNov 7, 2012
  22. Jeff KingNov 7, 2012
  23. Peter OberndorferJun 3, 2013
  24. Jeff KingJun 3, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.