Re: fatal output from git-show really wants a terminal

Junio C Hamano [off-list ref] writes:

"Boyd Stephen Smith Jr." [off-list ref] writes:

quoted

quoted

 $ git log >foo.out

and start a pager, which makes no sense.

Good point, I'll try and consider that while I investgate the history of the 
issue.

Isn't the issue about 61b8050 (sending errors to stdout under $PAGER,
2008-02-16)?  With that commit, we changed things so that when we send the
standard output to the $PAGER, we dup stderr to the $PAGER as well,
because otherwise any output to stderr will be wiped out by whatever the
pager does and the user will not notice the breakage.  E.g.

	$ git log

will just show reams of output, and you won't see any errors and warnings
even if there were any encountered during the process.

Unfortunately we did it unconditionally.  There is no reason to dup stderr
to the $PAGER if the command line was:

	$ git log 2>error.log

in which case you would want to view the normal output in your $PAGER and
you are keeping the log of the error output in a separate file.

I haven't heard anything more about this, but if you were indeed
discussing the change made by 61b8050, I think the fix should just be like
this.

 pager.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git c/pager.c w/pager.c
index aa0966c..f19ddbc 100644
--- c/pager.c
+++ w/pager.c

@@ -70,7 +70,8 @@ void setup_pager(void)
 
 	/* original process continues, but writes to the pipe */
 	dup2(pager_process.in, 1);
-	dup2(pager_process.in, 2);
+	if (isatty(2))
+		dup2(pager_process.in, 2);
 	close(pager_process.in);
 
 	/* this makes sure that the parent terminates after the pager */

On Mon, Dec 15, 2008 at 12:15:38AM -0800, Junio C Hamano wrote:

I haven't heard anything more about this, but if you were indeed
discussing the change made by 61b8050, I think the fix should just be like
this.
[...]
-	dup2(pager_process.in, 2);
+	if (isatty(2))
+		dup2(pager_process.in, 2);

I can't speak for the original poster, but I do think this behavior
should be less surprising to users who perform the one particular
redirection (and in the other 99% of cases, should behave exactly the
same). So I think it's a positive change.

-Peff