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

[PATCH] diff: add a blocks mode for moved code detection

From
Stefan Beller <sbeller@google.com>
Date
Apr 3, 2018, 21:05 UTC
Message-ID
<20180403210536.33798-1-sbeller@google.com>
In-Reply-To
<87muykuij7.fsf@evledraar.gmail.com>
Show 32 quoted lines
> Currently we have plain, zebra & dimmed_zebra, and zebra is the
> default.
>
> I got an internal report from someone who had, because zebra looked
> crappy in his terminal, moved to "plain", and was reporting getting
> worse moved diffs as a result.
>
> I found that there's essentially a missing setting between "plain" and
> "zebra", in git command terms:
>
>     # The "plain" setting
>     git -c diff.colorMoved=true \
>         -c diff.colorMoved=plain \
>         show <commit>
>
>     # We don't have this, it's "plain" but with "zebra" heuristics,
>     # plain_zebra?
>     git -c diff.colorMoved=true \
>         -c color.diff.oldMovedAlternative="bold magenta" \
>         -c color.diff.newMovedAlternative="bold yellow" \
>         -c diff.colorMoved=zebra \
>         show <commit>
>
>     # The "zebra" setting.
>     git -c diff.colorMoved=true \
>         -c diff.colorMoved=zebra \
>         show <commit>
>
> Which is what I mean by the current config conflating two (to me)
> unrelated things. One is how we, via any method, detect what's moved or
> not, and the other is what color/format we use to present this to the
> user.
Oh I see.

Reading the docs again, maybe we want to have a "blocks" mode, that is zebra with the same color for any block?

> You can feed that plain_zebra invocation input where it'll color-wise
> produce something that looks *almost* like "plain", but will differ (and
> usually be better) in what lines it decides to show as moved, which of
> course is due to *MovedAlternative.

I would think this is close to what you want (module implementation errors, I did not run/test this code).

One could also argue that this is *too* weak, as when there are multiple blocks of say 15 chars adjacent, they might be one large block.

---8<----
From dde04f6afa35a313fac3575100fe83b554ec2b59 Mon Sep 17 00:00:00 2001
From: Stefan Beller <sbeller@google.com>
Date: Tue, 3 Apr 2018 14:03:01 -0700
Subject: [PATCH] diff: add a blocks mode for moved code detection
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/diff-options.txt | 5 +++++
 diff.c                         | 4 +++-
 diff.h                         | 5 +++--
 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c330c01ff0..abce5142d2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -268,6 +268,11 @@ plain::
 	that are added somewhere else in the diff. This mode picks up any
 	moved line, but it is not very useful in a review to determine
 	if a block of code was moved without permutation.
+blocks:
+	Blocks of moved text of at least 20 alphanumeric characters
+	are detected greedily. The detected blocks are
+	painted using either the 'color.diff.{old,new}Moved' color.
+	Adjacent blocks cannot be told apart.
 zebra::
 	Blocks of moved text of at least 20 alphanumeric characters
 	are detected greedily. The detected blocks are
diff --git a/diff.c b/diff.c
index 21c3838b25..80dd8cbd9a 100644
--- a/diff.c
+++ b/diff.c
@@ -271,6 +271,8 @@ static int parse_color_moved(const char *arg)
 		return COLOR_MOVED_NO;
 	else if (!strcmp(arg, "plain"))
 		return COLOR_MOVED_PLAIN;
+	else if (!strcmp(arg, "blocks"))
+		return COLOR_MOVED_BLOCKS;
 	else if (!strcmp(arg, "zebra"))
 		return COLOR_MOVED_ZEBRA;
 	else if (!strcmp(arg, "default"))
@@ -899,7 +901,7 @@ static void mark_color_as_moved(struct diff_options *o,
 
 		block_length++;
 
-		if (flipped_block)
+		if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
 			l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
 	}
 	adjust_last_block(o, n, block_length);
diff --git a/diff.h b/diff.h
index 6bd278aac1..3a228861d9 100644
--- a/diff.h
+++ b/diff.h
@@ -207,8 +207,9 @@ struct diff_options {
 	enum {
 		COLOR_MOVED_NO = 0,
 		COLOR_MOVED_PLAIN = 1,
-		COLOR_MOVED_ZEBRA = 2,
-		COLOR_MOVED_ZEBRA_DIM = 3,
+		COLOR_MOVED_BLOCKS = 2,
+		COLOR_MOVED_ZEBRA = 3,
+		COLOR_MOVED_ZEBRA_DIM = 4,
 	} color_moved;
 	#define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
 	#define COLOR_MOVED_MIN_ALNUM_COUNT 20
-- 
2.17.0.484.g0c8726318c-goog
Previous: Ævar Arnfjörð BjarmasonNext: Stefan Beller
Message 12 of 26 in “Moved code detection: ignore space on uniform indentation”
  1. 0/7 Moved code detection: ignore space on uniform indentationStefan Beller, Apr 2, 2018
  2. 2/7 xdiff/xdiffi.c: remove unneeded function declarationsStefan Beller, Apr 2, 2018
  3. 1/7 xdiff/xdiff.h: remove unused flagsStefan Beller, Apr 2, 2018
  4. 5/7 diff.c: refactor internal representation for coloring moved codeStefan Beller, Apr 2, 2018
  5. Jonathan TanApr 2, 2018
  6. Stefan BellerApr 3, 2018
  7. Stefan BellerApr 6, 2018
  8. Jonathan TanApr 6, 2018
  9. Ævar Arnfjörð BjarmasonApr 3, 2018
  10. Stefan BellerApr 3, 2018
  11. Ævar Arnfjörð BjarmasonApr 3, 2018
  12. diff: add a blocks mode for moved code detectionStefan Beller, Apr 3, 2018
  13. 6/7 diff.c: decouple white space treatment for move detection from generic optionStefan Beller, Apr 2, 2018
  14. Jonathan TanApr 3, 2018
  15. 7/7 diff.c: add --color-moved-ignore-space-delta optionStefan Beller, Apr 2, 2018
  16. Jonathan TanApr 3, 2018
  17. Stefan BellerApr 3, 2018
  18. Jonathan TanApr 3, 2018
  19. 3/7 diff.c: do not pass diff options as keydata to hashmapStefan Beller, Apr 2, 2018
  20. Jeff KingApr 6, 2018
  21. Stefan BellerApr 6, 2018
  22. 4/7 diff.c: adjust hash function signature to match hashmap expectationStefan Beller, Apr 2, 2018
  23. Jonathan TanApr 2, 2018
  24. Jacob KellerApr 3, 2018
  25. Stefan BellerApr 3, 2018
  26. Jacob KellerApr 3, 2018

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.