Re: [PATCH] Allow git-diff exit with codes similar to diff(1)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 14, 2007, 12:05 UTC
Message-ID: <Pine.LNX.4.63.0703141301210.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <81b0412b0703140137i6d0df44aua9205e81249570ea@mail.gmail.com>

Hi,

On Wed, 14 Mar 2007, Alex Riesen wrote:

Show 32 quoted lines

> On 3/14/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > So, big master to hom everybody bows, how to return the correct value
> > > > when executing a pager? Because this _has_ to be done if we go that
> > > > way.
> > >
> > > Why? If you execute the pager, nobody cares about the error value
> > > anyway.
> > >
> > > I don't see why you would mix in a pager here. If you do
> > >
> > >       diff -u file1 file2 | less -S
> > >
> > > the return value of the pipe will not only generally be totally
> > > uninteresting and never used, but it will be the return value of "less"
> > > anyway. Which is what we'd get quite naturally.
> > 
> > The thing is, most people do not realize that
> > 
> >         git diff file1 file2
> > 
> > _will_ execute a pager. As foreground process. And the return value is
> > that of the pager.
> 
> In this example this is obviously (sometimes it is obscurely) interactive.
> The return code is seldom expected.
> 
> More sneaky case could be this:
> 
>  git diff file1 file2 > tmp && do_something
>  rm -f tmp
> 
> But we have isatty in setup_pager, so this works properly.

The problem is test cases. I think that we pipe the output of the test case _anyway_, so the isatty() call is helping us there.

If we did not (this applies to test cases _outside_ of Git, too), then a simple

	git diff bla || exit

would not work as expected. Even worse, as long as it is piped somewhere, even cat, it works. But once you no longer pipe it (to get the nice pager, for example), it stops working.

Ciao, Dscho