Re: [PATCH v2] diff: allow --color-moved with --no-ext-diff

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <l.s.r@web.de> writes:

> * use echo instead of false as the (unused) external diff command to
>   avoid giving the wrong impression that diff.external is a boolean
>   option we turn off here.

I am fine with either "/bin/false" or "echo".  I kind of found it a
nice way to assert that --no-ext-diff is indeed in effect (if we did
not disable it, "false" would lead to "whoa, your external diff
driver returned a failure").

Thanks, will queue.

>  diff.c                     | 3 ++-
>  t/t4015-diff-whitespace.sh | 9 +++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index 6e432cb8fc..aa0fb77761 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4965,7 +4965,8 @@ void diff_setup_done(struct diff_options *options)
>  	if (options->flags.follow_renames)
>  		diff_check_follow_pathspec(&options->pathspec, 1);
>
> -	if (!options->use_color || external_diff())
> +	if (!options->use_color ||
> +	    (options->flags.allow_external && external_diff()))
>  		options->color_moved = 0;
>
>  	if (options->filter_not) {
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index b443626afd..851cfe4f32 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1184,6 +1184,15 @@ test_expect_success 'detect moved code, complete file' '
>  	test_cmp expected actual
>  '
>
> +test_expect_success '--color-moved with --no-ext-diff' '
> +	test_config color.diff.oldMoved "yellow" &&
> +	test_config color.diff.newMoved "blue" &&
> +	args="--color --color-moved=zebra --no-renames HEAD" &&
> +	git diff $args >expect &&
> +	git -c diff.external=echo diff --no-ext-diff $args >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'detect malicious moved code, inside file' '
>  	test_config color.diff.oldMoved "normal red" &&
>  	test_config color.diff.newMoved "normal green" &&
> --
> 2.45.2