Re: [PATCH] git-rev-list: give better diagnostic for failed write

From: Jeff King <peff@peff.net>
Date: Jun 28, 2007, 21:34 UTC
Message-ID: <20070628213451.GB22455@coredump.intra.peff.net>
In-Reply-To: <20070628190406.GC29279@thunk.org>

On Thu, Jun 28, 2007 at 03:04:06PM -0400, Theodore Tso wrote:

Show 5 quoted lines

>     This patch skips the fflush() calls when stdout is a regular file, or
>     if the environment variable GIT_NEVER_FLUSH_STDOUT is set.  This
>     latter can speed up a command such as:
>     
>     	(export GIT_NEVER_FLUSH_STDOUT=t; git-rev-list HEAD | wc -l)

I wonder if this would be more natural in the opposite form:

  GIT_FLUSH_STDOUT=1 git-rev-list HEAD

In general, you don't want to do the flushing unless:
  - it's going to the pager
  - some program is reading incrementally

In the first case, we can just turn on GIT_FLUSH_STDOUT when we kick off the pager. In the second case, that program can just add the variable to its invocation.

On top of which, in your patch the type of output trumps the environment variable, which seems backwards. In other words, I can't do this:

GIT_FLUSH_EVEN_THOUGH_ITS_A_FILE=1 git-rev-list HEAD >file [in another window] tail -f file

I would think an explicit preference from a variable should override any guesses.

-Peff