From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sat, 23 Jun 2018 16:52:33 GMT
Subject: Re: [PATCH v3 8/8] diff.c: add white space mode to move detection that allows indent changes
Message-ID: <20180623165233.23653-1-szeder.dev@gmail.com>
In-Reply-To: <20180622015725.219575-9-sbeller@google.com>

> diff --git a/diff.c b/diff.c
> index 040b46545e5..9e357111864 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -302,12 +302,18 @@ static int parse_color_moved_ws(const char *arg)
>  			ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
>  		else if (!strcmp(sb.buf, "ignore-all-space"))
>  			ret |= XDF_IGNORE_WHITESPACE;
> +		else if (!strcmp(sb.buf, "allow-indentation-change"))
> +			ret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;
>  		else
>  			error(_("ignoring unknown color-moved-ws mode '%s'"), sb.buf);
>  
>  		strbuf_release(&sb);
>  	}
>  
> +	if ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&
> +	    (ret & XDF_WHITESPACE_FLAGS))
> +		die(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));

Note that this is a translated error message.

> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index aad0870c8a1..13b20be591e 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1827,4 +1827,94 @@ test_expect_success 'only move detection ignores white spaces' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'compare whitespace delta across moved blocks' '
> +
> +	git reset --hard &&
> +	q_to_tab <<-\EOF >text.txt &&
> +	QIndented
> +	QText across
> +	Qsome lines
> +	QBut! <- this stands out
> +	QAdjusting with
> +	QQdifferent starting
> +	Qwhite spaces
> +	QAnother outlier
> +	QQQIndented
> +	QQQText across
> +	QQQfive lines
> +	QQQthat has similar lines
> +	QQQto previous blocks, but with different indent
> +	QQQYetQAnotherQoutlierQ
> +	EOF
> +
> +	git add text.txt &&
> +	git commit -m "add text.txt" &&
> +
> +	q_to_tab <<-\EOF >text.txt &&
> +	QQIndented
> +	QQText across
> +	QQsome lines
> +	QQQBut! <- this stands out
> +	Adjusting with
> +	Qdifferent starting
> +	white spaces
> +	AnotherQoutlier
> +	QQIndented
> +	QQText across
> +	QQfive lines
> +	QQthat has similar lines
> +	QQto previous blocks, but with different indent
> +	QQYetQAnotherQoutlier
> +	EOF
> +
> +	git diff --color --color-moved --color-moved-ws=allow-indentation-change |
> +		grep -v "index" |
> +		test_decode_color >actual &&

Please use an intermediate file for the output of 'git diff', because
running it upstream of a pipe hides its exit code.

> +
> +	q_to_tab <<-\EOF >expected &&
> +		<BOLD>diff --git a/text.txt b/text.txt<RESET>
> +		<BOLD>--- a/text.txt<RESET>
> +		<BOLD>+++ b/text.txt<RESET>
> +		<CYAN>@@ -1,14 +1,14 @@<RESET>
> +		<BOLD;MAGENTA>-QIndented<RESET>
> +		<BOLD;MAGENTA>-QText across<RESET>
> +		<BOLD;MAGENTA>-Qsome lines<RESET>
> +		<RED>-QBut! <- this stands out<RESET>
> +		<BOLD;MAGENTA>-QAdjusting with<RESET>
> +		<BOLD;MAGENTA>-QQdifferent starting<RESET>
> +		<BOLD;MAGENTA>-Qwhite spaces<RESET>
> +		<RED>-QAnother outlier<RESET>
> +		<BOLD;MAGENTA>-QQQIndented<RESET>
> +		<BOLD;MAGENTA>-QQQText across<RESET>
> +		<BOLD;MAGENTA>-QQQfive lines<RESET>
> +		<BOLD;MAGENTA>-QQQthat has similar lines<RESET>
> +		<BOLD;MAGENTA>-QQQto previous blocks, but with different indent<RESET>
> +		<RED>-QQQYetQAnotherQoutlierQ<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>Indented<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>Text across<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>some lines<RESET>
> +		<GREEN>+<RESET>QQQ<GREEN>But! <- this stands out<RESET>
> +		<BOLD;CYAN>+<RESET><BOLD;CYAN>Adjusting with<RESET>
> +		<BOLD;CYAN>+<RESET>Q<BOLD;CYAN>different starting<RESET>
> +		<BOLD;CYAN>+<RESET><BOLD;CYAN>white spaces<RESET>
> +		<GREEN>+<RESET><GREEN>AnotherQoutlier<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>Indented<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>Text across<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>five lines<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>that has similar lines<RESET>
> +		<BOLD;CYAN>+<RESET>QQ<BOLD;CYAN>to previous blocks, but with different indent<RESET>
> +		<GREEN>+<RESET>QQ<GREEN>YetQAnotherQoutlier<RESET>
> +	EOF
> +
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'compare whitespace delta incompatible with other space options' '
> +	test_must_fail git diff \
> +		--color-moved-ws=allow-indentation-change,ignore-all-space \
> +		2>err &&
> +	grep allow-indentation-change err

A translated error message should be checked with 'test_i18ngrep'.