Re: Odd "git diff" breakage

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2014, 18:38 UTC
Message-ID: <xmqq38hy6wnk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqq7g7a6x0g.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 21 quoted lines

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> I hit this oddity when not remembering the right syntax for --color-words..
>>
>> Try this (outside of a git repository):
>>
>>    touch a b
>>    git diff -u --color=words a b
>>
>> and watch it scroll (infinitely) printing out
>>
>>    error: option `color' expects "always", "auto", or "never"
>>
>> forever.
>>
>> I haven't tried to root-cause it, since I'm supposed to be merging stuff..
>>
>>                 Linus
>
> Hmph, interesting.  "outside a repository" is the key, it seems.
> And I can see it all the way down to 1.7.3 (at least).

diff_opt_parse() returns the number of options parsed, or often returns error() which is defined to return -1. Yes, return value of 0 is "I don't understand that option", which should cause the caller to say that, but negative return should not be forgotten.

 diff-no-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index c554691..265709b 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -198,7 +198,7 @@ void diff_no_index(struct rev_info *revs,
 			i++;
 		else {
 			j = diff_opt_parse(&revs->diffopt, argv + i, argc - i);
-			if (!j)
+			if (j <= 0)
 				die("invalid diff option/value: %s", argv[i]);
 			i += j;
 		}