| Message ID | fee1815c-80bb-42a4-97f3-d3f8e9b3a6ca@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] diff: allow --color-moved with --no-ext-diff | expand |
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
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" &&
We ignore the option --color-moved if an external diff program is configured, presumably because its overhead is unnecessary in that case. Respect the option if we don't actually use the external diff, though. Reported-by: lolligerhans@gmx.de Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> --- Changes since v1: * use single word colors, leaving the background unchanged, as they are easier to read, * run git diff to generate the expected content instead of reusing the output of the previous test ... * ... and put the common arguments in a variable to make clear that we compare diff with and without a diff.external/--no-ext-diff dance, * 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. diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index a1478680b6..851cfe4f32 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -1185,11 +1185,11 @@ test_expect_success 'detect moved code, complete file' ' ' test_expect_success '--color-moved with --no-ext-diff' ' - test_config color.diff.oldMoved "normal red" && - test_config color.diff.newMoved "normal green" && - cp actual.raw expect && - git -c diff.external=false diff HEAD --no-ext-diff \ - --color-moved=zebra --color --no-renames >actual && + 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 ' diff.c | 3 ++- t/t4015-diff-whitespace.sh | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) -- 2.45.2