Re: [PATCH] diff: exit(1) if 'diff --quiet <repo file> <external file>' finds changes

From: Jeff King <peff@peff.net>
Date: Jun 15, 2012, 20:24 UTC
Message-ID: <20120615202441.GA12163@sigill.intra.peff.net>
In-Reply-To: <7vmx449w3j.fsf@alter.siamese.dyndns.org>

On Fri, Jun 15, 2012 at 01:08:32PM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Tim Henigan <tim.henigan@gmail.com> writes:
> 
> >>> @@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,
> >>>        * The return code for --no-index imitates diff(1):
> >>>        * 0 = no changes, 1 = changes, else error
> >>>        */
> >>> -     exit(revs->diffopt.found_changes);
> >>> +     result = !!diff_result_code(&revs->diffopt, 0);
> >>> +     exit(result);
> >
> > I assume the '!!' before 'diff_result_code' is a typo.
> 
> Not a typo.  I meant to use that idiom to turn 0 or not into
> boolean, as diff_result_code() can return values other than 0 or 1.

I wonder if that is a good idea, though. AFAICT, diff_result_code will only return a different exit code if "--check" is used. If we pass along the exit code fully, then:

  1. If --check is not used, we will be diff(1)-compatible.

  2. If --check is used, then we will not be compatible with diff(1) in
     our exit code. But diff(1) does not have --check in the first
     place, so there is no point in us trying to be a drop-in
     replacement.

-Peff