git-diff should not fire up $PAGER if there is no diff

git-diff should not fire up $PAGER if there is no diff

[jidanni]

git-diff should not fire up $PAGER if there is no diff output.
Just exit. The man page doesn't even mention $PAGER too.

Re: git-diff should not fire up $PAGER if there is no diff

[Jeff King]

On Tue, Dec 16, 2008 at 08:21:33AM +0800, jidanni@jidanni.org wrote:

> git-diff should not fire up $PAGER if there is no diff output.
> Just exit. The man page doesn't even mention $PAGER too.

I agree that would be nice, but it is a little difficult to implement.
The current behavior forks early and then pipes the output to the pager.
So we would have to:

  1. change that behavior to instead delay starting the pager until the
     first output. Which means intercepting every
     write/fwrite/printf/fputs/etc call.

  2. detect EOF before starting the pager. We in fact already delay
     running the pager in the forked process until we have some activity
     on the pipe, but I don't know if there is a portable way of
     detecting that that activity is EOF without performing an actual
     read() call (which is undesirable, since it eats the first byte of
     output that should go to the pager).

  3. a hacky solution to (2) above would be to make _2_ pipes, one of
     which signals to the pager sub-process either "exit now" or "proceed
     with running the pager".

The usual workaround is to ask the pager to exit immediately if the
output is small. I.e., putting "F" in your LESS variable (which git does
automatically if you don't already have LESS set).

-Peff

Re: git-diff should not fire up $PAGER if there is no diff

[Stefan Karpinski]

> On Mon, Dec 15, 2008 at 7:56 PM, Jeff King <peff@peff.net> wrote:
>  2. detect EOF before starting the pager. We in fact already delay
>     running the pager in the forked process until we have some activity
>     on the pipe, but I don't know if there is a portable way of
>     detecting that that activity is EOF without performing an actual
>     read() call (which is undesirable, since it eats the first byte of
>     output that should go to the pager).

Wouldn't ungetc work? Or is that not portable enough? (It would only
work here because the EOF has to be the first character.)

Re: git-diff should not fire up $PAGER if there is no diff

[Jeff King]

On Tue, Dec 16, 2008 at 01:35:53AM -0500, Stefan Karpinski wrote:

> > On Mon, Dec 15, 2008 at 7:56 PM, Jeff King <peff@peff.net> wrote:
> >  2. detect EOF before starting the pager. We in fact already delay
> >     running the pager in the forked process until we have some activity
> >     on the pipe, but I don't know if there is a portable way of
> >     detecting that that activity is EOF without performing an actual
> >     read() call (which is undesirable, since it eats the first byte of
> >     output that should go to the pager).
> 
> Wouldn't ungetc work? Or is that not portable enough? (It would only
> work here because the EOF has to be the first character.)

No, it won't work. ungetc works on the buffered stdio object, so it is
useful for pushing back characters onto the buffer to be read later in
the program from the same buffer. But in this case, we are going to
execv() (or on Windows, spawn) the pager, meaning it will throw away
anything that has been read() from the pipe and put in the buffer.

So we would need a system call to push a character back to the OS, so
that it was available for read() by the pager process.

-Peff

Re: git-diff should not fire up $PAGER if there is no diff

[Stefan Karpinski]

On Tue, Dec 16, 2008 at 2:44 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 16, 2008 at 01:35:53AM -0500, Stefan Karpinski wrote:
>
>> > On Mon, Dec 15, 2008 at 7:56 PM, Jeff King <peff@peff.net> wrote:
>> >  2. detect EOF before starting the pager. We in fact already delay
>> >     running the pager in the forked process until we have some activity
>> >     on the pipe, but I don't know if there is a portable way of
>> >     detecting that that activity is EOF without performing an actual
>> >     read() call (which is undesirable, since it eats the first byte of
>> >     output that should go to the pager).
>>
>> Wouldn't ungetc work? Or is that not portable enough? (It would only
>> work here because the EOF has to be the first character.)
>
> No, it won't work. ungetc works on the buffered stdio object, so it is
> useful for pushing back characters onto the buffer to be read later in
> the program from the same buffer. But in this case, we are going to
> execv() (or on Windows, spawn) the pager, meaning it will throw away
> anything that has been read() from the pipe and put in the buffer.
>
> So we would need a system call to push a character back to the OS, so
> that it was available for read() by the pager process.

Yeah, I realized that after I sent the message. Late night sending bad!

git-diff should not fire up $PAGER, period!

[jidanni]

Gentlemen, I have found the solution to your problem.

Unbundle git-diff and $PAGER.

Ask yourself, does diff(1) call $PAGER?

No. That's because the Unix designers were smart enough not to glue
everything together.

Now's your chance to repent, as you haven't even yet mentioned $PAGER
on the git-diff man page. Yes, do mention it: "EXAMPLES: git-diff|less"
I.e., the user can page the output if he feels inclined, just like any
other output. I mean one already has a wallet. The bank need not give
the user one every time they make a withdraw.

I mean here I am in emacs, and

-*- mode: compilation; default-directory: "...coreutils/" -*-
Compilation started at Thu Dec 18 03:15:14
git-diff
WARNING: terminal is not fully functional^M
^M-  (press RETURN)

"It's all emacs' fault for emulating a tty too well"... no, it's all
your fault for gumming things together. No I don't want my cookies
with obligatory milk. I'll using git-diff|cat for now instead of
complaining that emacs is all wrong. Even using git-diff|cat|less is
better than messing with the LESS=F bug. Repent, whippersnappers!

OK, doing test x$EMACS = xt && PAGER=cat in .bashrc. That will help
for emacs' shell buffers, but not compilation mode buffers... "then
just make a hook"... 13 hooks to combat one poor design choice.
And one notices git-show is gummed up too.

Hmm, looking in changelogs, we see

 * Error messages used to be sent to stderr, only to get hidden,
   when $PAGER was in use.  They now are sent to stdout along
   with the command output to be shown in the $PAGER.

Well, if you had left paging to the user, no one would have blamed you
for making error messages disappear, and you could have left stderr as
the elders intended.

Wait,
$ git-config --global core.pager ""
Cool. Bye.

Re: git-diff should not fire up $PAGER, period!

[Junio C Hamano]

jidanni@jidanni.org writes:

> I mean here I am in emacs, and
>
> -*- mode: compilation; default-directory: "...coreutils/" -*-
> Compilation started at Thu Dec 18 03:15:14
> git-diff
> WARNING: terminal is not fully functional^M
> ^M-  (press RETURN)

Any semi-good emacs users (let alone hackers) export PAGER=cat to be used
in compilation mode (and possibly shell mode), so this is not a problem in
practice.

I have something like this in my .emacs:

    (setenv "PAGER" "cat")

I suspect (I am just a user not a hacker) this will have bad interaction
with emacs terminal emulation mode, but I do not use the mode, so it is
enough for me.

Re: git-diff should not fire up $PAGER, period!

[Linus Torvalds]

On Thu, 18 Dec 2008, jidanni@jidanni.org wrote:
>
> Gentlemen, I have found the solution to your problem.

Umm. YOUR problem.

Do that whole

	[core]
		pager = 

and you can get the behaviour you want.

Or just get rid of emacs. Your problem has nothing to do with git, and 
everything to do with emacs. And then you have the _gall_ to talk about 
"unix design" and not gumming programs together, when you yourself use the 
most gummed-up piece of absolute sh*t there is!

			Linus

Re: git-diff should not fire up $PAGER, period!

[Miles Bader]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> And then you have the _gall_ to talk about "unix design"...

Another beer?

-Miles

-- 
Suburbia: where they tear out the trees and then name streets after them.

Re: git-diff should not fire up $PAGER, period!

[Jeff King]

On Thu, Dec 18, 2008 at 05:45:35AM +0800, jidanni@jidanni.org wrote:

> Gentlemen, I have found the solution to your problem.
> 
> Unbundle git-diff and $PAGER.

If you are going to argue this, please at least go back and read the
numerous times it has been brought up in the past on the list archive.

The last discussion ended up showing that some people really like the
automatic pager for some commands, and some people really detest it.
So I implemented 4e10738 (Allow per-command pager config, 2008-07-03),
and now you can do:

  git config pager.diff false

and be happy (or, as you obviously disocvered, simply unsetting
core.pager will disable all).

-Peff

Re: git-diff should not fire up $PAGER, period!

[Miles Bader]

Just (setenv "PAGER" "cat") in .emacs.

[I actually have it set to /bin/cat, not sure if that's meaningful or not.]

-Miles

-- 
.Numeric stability is probably not all that important when you're guessing.

Re: git-diff should not fire up $PAGER, period!

[Johannes Sixt]

Miles Bader schrieb:
> Just (setenv "PAGER" "cat") in .emacs.
> 
> [I actually have it set to /bin/cat, not sure if that's meaningful or not.]

No, really, you should set it to plain "cat": As a special case git
recognizes this token and does not run any pager. If you set it to
"/bin/cat" it does run a pager, namely /bin/cat.

-- Hannes

Re: git-diff should not fire up $PAGER, period!

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Miles Bader schrieb:
>> Just (setenv "PAGER" "cat") in .emacs.
>> 
>> [I actually have it set to /bin/cat, not sure if that's meaningful or not.]
>
> No, really, you should set it to plain "cat": As a special case git
> recognizes this token and does not run any pager. If you set it to
> "/bin/cat" it does run a pager, namely /bin/cat.

But that would not hurt ;-).

On the other hand, using PAGER=cat or PAGER=/bin/cat is the right thing to
do in compilation and shell modes in Emacs, regardless of your use of git.

Re: git-diff should not fire up $PAGER, period!

[Mike Coleman]

It's so cold here my car wouldn't start this morning, so I feel
fortunate to have these flames to keep me warm!  :-)


I still find git-diff's unsolicited invocation of $PAGER a bit
jarring, but I also find that I like it.  It has a sort of Windows-ish
feel to it (am I straying from the true path?).  It's convenient,
though honestly "git-diff|l" would only be two extra characters.  From
a UI perspective, the oddest thing about it is that all (not, in
fact?) of the other git programs which might also produce lengthy
output *don't* invoke $PAGER--git-status particularly comes to mind.
There are a large number of Unix programs that would be more
convenient if their output was automatically paged, but I'm not sure
it'd be better if they were all changed to do this.  (The best analog
I can think of where this seems desirable is man(1).)

To me, the most important nugget from the original complaint was that
git-diff sends its error messages to stdout.  I understand why it
might be done, but I'd worry about losing the stderr diagnostic for a
command that matters.  [I've been playing around with this for a few
minutes trying to see errors going to stdout and I can't reproduce
it--I wonder if they really do.]


Regarding the emacs complaint, as an emacs user I'd say I'm not
surprised this didn't quite work right.  In the particular case of the
compilation buffer, I wonder if the solution isn't to just unset TERM,
the existence of which (together with the fact that there's a pty)
could be taken as an invitation for the subordinate process to start
doing awful raw-terminal things.  (Similar logic applies to the
DISPLAY variable.)  Like Junio, I also eschew doing terminal emulation
inside of emacs.

Good evening from the icy midwest,
Mike

Re: git-diff should not fire up $PAGER, period!

[Junio C Hamano]

"Mike Coleman" <tutufan@gmail.com> writes:

> To me, the most important nugget from the original complaint was that
> git-diff sends its error messages to stdout.  I understand why it
> might be done, but I'd worry about losing the stderr diagnostic for a
> command that matters.  [I've been playing around with this for a few
> minutes trying to see errors going to stdout and I can't reproduce
> it--I wonder if they really do.]

They indeed did but it has hopefully been fixed.  See a833502 (pager: do
not dup2 stderr if it is already redirected, 2008-12-15).

> ...  Like Junio, I also eschew doing terminal emulation
> inside of emacs.

Come to think of it, it may have been from you that I picked up the trick
of setting PAGER to cat inside an Emacs environment.

> Good evening from the icy midwest,

Good evening from rainy and chilly SoCal.

Re: git-diff should not fire up $PAGER, period!

[Johannes Schindelin]

Hi,

On Wed, 17 Dec 2008, Mike Coleman wrote:

> I still find git-diff's unsolicited invocation of $PAGER a bit
> jarring, but I also find that I like it.

It might have its reason in that it follows the common flow, where 
interactive use of _any_ diff program is _just useless_ unless piped to a 
pager.

And no, redirecting to a file is not interactive.

Can we please stop with those bogus and pointless, not to mention 
uninformed, discussions about design decisions that have been verified to 
be useful, or at least not harmful in any way, for a _long_, _long_ time?

The git list is high volume already, but at least so far it was high 
signal/noise ratio.

Let's keep it that way,
Dscho