git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: lolligerhans@gmx.de, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2] diff: allow --color-moved with --no-ext-diff
Date: Mon, 24 Jun 2024 21:15:45 +0200	[thread overview]
Message-ID: <fee1815c-80bb-42a4-97f3-d3f8e9b3a6ca@web.de> (raw)
In-Reply-To: <trinity-acbdb8fc-3dc3-4dca-890c-8bcb37405782-1719050465639@msvc-mesg-gmx004>

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(-)

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

  parent reply	other threads:[~2024-06-24 19:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-22 10:01 Bug: diff.external --no-ext-diff suppresses --color-moved lolligerhans
2024-06-22 19:41 ` [PATCH] diff: allow --color-moved with --no-ext-diff René Scharfe
2024-06-23  9:17   ` Aw: " lolligerhans
2024-06-23  9:46     ` René Scharfe
2024-06-24 16:21   ` Junio C Hamano
2024-06-24 19:15     ` René Scharfe
2024-06-25  0:39       ` Junio C Hamano
2024-06-24 19:15 ` René Scharfe [this message]
2024-06-24 20:53   ` [PATCH v2] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fee1815c-80bb-42a4-97f3-d3f8e9b3a6ca@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=lolligerhans@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).