Re: [PATCH] Revert "Stop starting pager recursively"

From: Jeff King <peff@peff.net>
Date: Apr 26, 2014, 17:35 UTC
Message-ID: <20140426173549.GB21493@sigill.intra.peff.net>
In-Reply-To: <7vtx9ghuhg.fsf@alter.siamese.dyndns.org>

[+cc Duy for real this time]

On Sat, Apr 26, 2014 at 10:27:07AM -0700, Junio C Hamano wrote:

Show 23 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > [+cc Duy, whose patch this is]
> >
> > On Fri, Apr 25, 2014 at 04:10:49PM -0400, Jörn Engel wrote:
> >
> >> A second option is to add a --pager (or rather --no-pager) option to
> >> the command line and allow the user to specify
> >>     GIT_PAGER="git --no-pager -p column --mode='dense color'" git -p branch
> >
> > I think we have "--no-pager" already. But the "-p" is turning _on_ the
> > pager, so you could also just omit it. IOW, I really don't understand
> > why the original command was not simply:
> >
> >   GIT_PAGER="git column --mode='dense color'" git -p branch
> >
> > The whole infinite loop that the original commit solved is caused by
> > specifying the "-p". So it sounds like the right solution is "don't do
> > that". Am I missing something useful that the "-p" does?
> 
> My reading of this is that Duy wanted to let "-p" kick in to pass
> the columnized output, which could be more than a page long, thru
> the usual "less" or whatever pager.

I thought that at first, too. But it doesn't work, because his patch actually _suppresses_ the pager. For example, something like:

  git config pager.column less
  git config pager.branch "git column --mode=dense"
  git branch

actively works without the original patch (and after Jörn's revert), but not with current git. However, you would not use "-p" there in any case, because it kicks in early and only respects $GIT_PAGER.

And I would also say that you are much better off there actually specifying your whole pipeline as a unit:

  git config pager.branch "git column --mode=dense | less"

I do a similar thing where I have PAGER=less, but set pager.log to "diff-highlight | less". I tried at one point to have it chain automatically, but the setup is just too complicated (as we're seeing here), and it's not _that_ big a deal to explicitly pipe to the next pager in the sequence.

-Peff