All of lore.kernel.org
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Junio C Hamano <gitster@pobox.com>
Cc: Nazri Ramliy <ayiehere@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH (Eek!)] git diff does not honor --no-ext-diff
Date: Wed, 26 Nov 2008 08:59:13 +0100	[thread overview]
Message-ID: <492D01D1.7020702@lsrfire.ath.cx> (raw)
In-Reply-To: <7vprkihqk6.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> "Nazri Ramliy" <ayiehere@gmail.com> writes:
> 
>> git-diff does not honor the --no-ext-diff option in both cases when the external
>> diff program is set via diff.external and gitattributes.
>>
>> Is this intentional?
> 
> Judging from 72909be (Add diff-option --ext-diff, 2007-06-30), I think
> this was intended in the sense that --ext-diff and --no-ext-diff were
> meant to be no-op for "diff" itself when they were introduced.
> 
> Having said that, I do not know if I agree with the original intention.
> It looks more like an oversight that came from focusing only on what a new
> behaviour for the "log" family should be, than a logical design decision
> to exclude "diff" from this codepath.
> 
> Wouldn't this be a better patch?

Yes.  And feel free to squash in the following. :)

diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index dfe3fbc..ec787b4 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -43,6 +43,13 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment should apply only to diff' '
 
 '
 
+test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '
+
+	GIT_EXTERNAL_DIFF=echo git diff --no-ext-diff |
+	grep "^diff --git a/file b/file"
+
+'
+
 test_expect_success 'diff attribute' '
 
 	git config diff.parrot.command echo &&
@@ -68,6 +75,13 @@ test_expect_success 'diff attribute should apply only to diff' '
 
 '
 
+test_expect_success 'diff attribute and --no-ext-diff' '
+
+	git diff --no-ext-diff |
+	grep "^diff --git a/file b/file"
+
+'
+
 test_expect_success 'diff attribute' '
 
 	git config --unset diff.parrot.command &&
@@ -94,6 +108,13 @@ test_expect_success 'diff attribute should apply only to diff' '
 
 '
 
+test_expect_success 'diff attribute and --no-ext-diff' '
+
+	git diff --no-ext-diff |
+	grep "^diff --git a/file b/file"
+
+'
+
 test_expect_success 'no diff with -diff' '
 	echo >.gitattributes "file -diff" &&
 	git diff | grep Binary

  reply	other threads:[~2008-11-26  8:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-26  7:12 [PATCH (Eek!)] git diff does not honor --no-ext-diff Nazri Ramliy
2008-11-26  7:52 ` Junio C Hamano
2008-11-26  7:59   ` René Scharfe [this message]
2008-11-26 10:01     ` Nazri Ramliy

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=492D01D1.7020702@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=ayiehere@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.