Re: [PATCH] pager: turn on "cat" optimization for DEFAULT_PAGER

From: Jeff King <peff@peff.net>
Date: Nov 20, 2013, 17:30 UTC
Message-ID: <20131120173054.GA15339@sigill.intra.peff.net>
In-Reply-To: <CABPQNSb6PD+oSw_LT6KaUYd8BTeN-WHJFodcuiLe=u76rFYFJw@mail.gmail.com>

On Wed, Nov 20, 2013 at 06:24:45PM +0100, Erik Faye-Lund wrote:

Show 14 quoted lines

> > diff --git a/pager.c b/pager.c
> > index c1ecf65..fa19765 100644
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -54,7 +54,7 @@ const char *git_pager(int stdout_is_tty)
> >                 pager = getenv("PAGER");
> >         if (!pager)
> >                 pager = DEFAULT_PAGER;
> > -       else if (!*pager || !strcmp(pager, "cat"))
> > +       if (!*pager || !strcmp(pager, "cat"))
> 
> Hmmpf. It's sometimes useful to actually pipe through cat rather than
> disabling the pager, as this changes the return-code from isatty. I
> sometimes use this for debugging-purposes. Does this patch break that?

My patch should not change the behavior of PAGER=cat, GIT_PAGER=cat, core.pager, etc. It should _only_ impact the case where DEFAULT_PAGER is set to "cat" (or NULL), and bring it in line with the other cases.

I am not clear on how you are using "cat", so I can't say whether it is broken. But if you are doing:

  PAGER=cat git log

that already is a no-op, and that is not changed by my patch. If you want to make stdout not a tty, I'd think:

  git log | cat

is the right way to do it (and anyway, when the pager is in effect git will pretend that stdout is a tty, since you would still want things like auto-color to go to the pager).

-Peff