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

Re: [PATCH v4 6/9] diff.c: add a blocks mode for moved code detection

From
BWBrandon Williams <bmwill@google.com>
Date
Jul 2, 2018, 17:18 UTC
Message-ID
<20180702171801.GB246956@google.com>
In-Reply-To
<20180629001958.85143-7-sbeller@google.com>
On 06/28, Stefan Beller wrote:
Show 132 quoted lines
> The new "blocks" mode provides a middle ground between plain and zebra.
> It is as intuitive (few colors) as plain, but still has the requirement
> for a minimum of lines/characters to count a block as moved.
> 
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>  (https://public-inbox.org/git/87o9j0uljo.fsf@evledraar.gmail.com/)
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/diff-options.txt |  8 ++++--
>  diff.c                         |  6 +++--
>  diff.h                         |  5 ++--
>  t/t4015-diff-whitespace.sh     | 49 ++++++++++++++++++++++++++++++++--
>  4 files changed, 60 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e3a44f03cdc..ba56169de31 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -276,10 +276,14 @@ 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.
> -zebra::
> +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 or
> +	painted using either the 'color.diff.{old,new}Moved' color.
> +	Adjacent blocks cannot be told apart.
> +zebra::
> +	Blocks of moved text are detected as in 'blocks' mode. The blocks
> +	are painted using either the 'color.diff.{old,new}Moved' color or
>  	'color.diff.{old,new}MovedAlternative'. The change between
>  	the two colors indicates that a new block was detected.
>  dimmed_zebra::
> diff --git a/diff.c b/diff.c
> index d1bae900cdc..95c51c0b7df 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"))
> @@ -278,7 +280,7 @@ static int parse_color_moved(const char *arg)
>  	else if (!strcmp(arg, "dimmed_zebra"))
>  		return COLOR_MOVED_ZEBRA_DIM;
>  	else
> -		return error(_("color moved setting must be one of 'no', 'default', 'zebra', 'dimmed_zebra', 'plain'"));
> +		return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
>  }
>  
>  int git_diff_ui_config(const char *var, const char *value, void *cb)
> @@ -903,7 +905,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 d29560f822c..7bd4f182c33 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -208,8 +208,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
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index ddbc3901385..e54529f026d 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1223,7 +1223,7 @@ test_expect_success 'plain moved code, inside file' '
>  	test_cmp expected actual
>  '
>  
> -test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
> +test_expect_success 'detect blocks of moved code' '
>  	git reset --hard &&
>  	cat <<-\EOF >lines.txt &&
>  		long line 1
> @@ -1271,6 +1271,50 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
>  	test_config color.diff.newMovedDimmed "normal cyan" &&
>  	test_config color.diff.oldMovedAlternativeDimmed "normal blue" &&
>  	test_config color.diff.newMovedAlternativeDimmed "normal yellow" &&
> +	git diff HEAD --no-renames --color-moved=blocks --color >actual.raw &&
> +	grep -v "index" actual.raw | test_decode_color >actual &&
> +	cat <<-\EOF >expected &&
> +	<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
> +	<BOLD>--- a/lines.txt<RESET>
> +	<BOLD>+++ b/lines.txt<RESET>
> +	<CYAN>@@ -1,16 +1,16 @@<RESET>
> +	<MAGENTA>-long line 1<RESET>
> +	<MAGENTA>-long line 2<RESET>
> +	<MAGENTA>-long line 3<RESET>
> +	 line 4<RESET>
> +	 line 5<RESET>
> +	 line 6<RESET>
> +	 line 7<RESET>
> +	 line 8<RESET>
> +	 line 9<RESET>
> +	<CYAN>+<RESET><CYAN>long line 1<RESET>
> +	<CYAN>+<RESET><CYAN>long line 2<RESET>
> +	<CYAN>+<RESET><CYAN>long line 3<RESET>
> +	<CYAN>+<RESET><CYAN>long line 14<RESET>
> +	<CYAN>+<RESET><CYAN>long line 15<RESET>
> +	<CYAN>+<RESET><CYAN>long line 16<RESET>
> +	 line 10<RESET>
> +	 line 11<RESET>
> +	 line 12<RESET>
> +	 line 13<RESET>
> +	<MAGENTA>-long line 14<RESET>
> +	<MAGENTA>-long line 15<RESET>
> +	<MAGENTA>-long line 16<RESET>
> +	EOF
> +	test_cmp expected actual
> +

Theres an empty line here. Not worth fixing if its the only issue though.

-- 
Brandon Williams
Previous: Stefan BellerNext: Stefan Beller
Message 18 of 25 in “Moved code detection: ignore space on uniform indentation”
  1. 0/8 Moved code detection: ignore space on uniform indentationStefan Beller, Jun 22, 2018
  2. 1/8 xdiff/xdiff.h: remove unused flagsStefan Beller, Jun 22, 2018
  3. 2/8 xdiff/xdiffi.c: remove unneeded function declarationsStefan Beller, Jun 22, 2018
  4. 4/8 diff.c: adjust hash function signature to match hashmap expectationStefan Beller, Jun 22, 2018
  5. 5/8 diff.c: add a blocks mode for moved code detectionStefan Beller, Jun 22, 2018
  6. 3/8 diff.c: do not pass diff options as keydata to hashmapStefan Beller, Jun 22, 2018
  7. 6/8 diff.c: decouple white space treatment from move detection algorithmStefan Beller, Jun 22, 2018
  8. 7/8 diff.c: factor advance_or_nullify out of mark_color_as_movedStefan Beller, Jun 22, 2018
  9. 8/8 diff.c: add white space mode to move detection that allows indent changesStefan Beller, Jun 22, 2018
  10. SZEDER GáborJun 23, 2018
  11. Junio C HamanoJun 22, 2018
  12. 0/9 Moved code detection: ignore space on uniform indentationStefan Beller, Jun 29, 2018
  13. 1/9 xdiff/xdiff.h: remove unused flagsStefan Beller, Jun 29, 2018
  14. 2/9 xdiff/xdiffi.c: remove unneeded function declarationsStefan Beller, Jun 29, 2018
  15. 4/9 diff.c: do not pass diff options as keydata to hashmapStefan Beller, Jun 29, 2018
  16. 5/9 diff.c: adjust hash function signature to match hashmap expectationStefan Beller, Jun 29, 2018
  17. 6/9 diff.c: add a blocks mode for moved code detectionStefan Beller, Jun 29, 2018
  18. Brandon WilliamsJul 2, 2018
  19. 7/9 diff.c: decouple white space treatment from move detection algorithmStefan Beller, Jun 29, 2018
  20. Brandon WilliamsJul 2, 2018
  21. 3/9 t4015: avoid git as a pipe inputStefan Beller, Jun 29, 2018
  22. 8/9 diff.c: factor advance_or_nullify out of mark_color_as_movedStefan Beller, Jun 29, 2018
  23. 9/9 diff.c: add white space mode to move detection that allows indent changesStefan Beller, Jun 29, 2018
  24. Brandon WilliamsJul 2, 2018
  25. Stefan BellerJul 2, 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.