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

[+cc Duy for real this time]

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

Jeff King [off-list ref] writes:

quoted

[+cc Duy, whose patch this is]

On Fri, Apr 25, 2014 at 04:10:49PM -0400, Jörn Engel wrote:

quoted

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