Re: Bug: pager.<cmd> doesn't work well with editors

On Wed, Sep 21, 2016 at 09:15:09AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

And this isn't really limited to the editor. It's more _annoying_ with
the editor, but really "pager.tag" does not make any sense to set right
now, because it is handled outside of the "tag" command entirely, and
doesn't know what mode the tag command will be running in.

Stepping back even further, perhaps the whole pager.<cmd> was a bad
interim move.  For those who set "less" without "-F", being able to
set pager.<cmd> to false may still be necessary, but I am wondering
about setting it to true or a command string here.

It did mean well and may have helped when "git <cmd>" that produces
reams of output had not yet learned to auto-paginate as a stop-gap
measure by allowing users to set pager.<cmd>, but I wonder if the
ideal course of action was to identify (or "wait until people show
their desire") individual operating modes of various commands and
teach them to auto-paginate.  For example, "tag -l" may be one of
them that we would want to teach to.

I don't think it is a bad move overall. I use "pager.log" to pipe
through a specific command (that is different than I would use for other
commands).

So I like the idea of configurability; the problem is just that it is
happening at the wrong level. The individual commands should be in
charge of it, with something like:

  /*
   * See if pager.log is configured, falling back to "true" (due to the
   * second argument). If so, and stdout is a tty, run the pager.
   *
   * This would be run at the top of cmd_log().
   */
  setup_auto_pager("log", 1);

  ...

  /*
   * As above, but note that we run this in cmd_tag() only at the right
   * moment. We'd probably actually flip the "0" here to a "1", but
   * this represents the current default.
   */
  if (mode_list)
	  setup_auto_pager("tag.list", 0);

That's a lot more boilerplate (each command needs to decide if and when
it should support the pager), but a one-liner in each spot is not so
bad.  Both builtins and external could use the C interface, but it's
trickier for other languages to redirect their own stdout (we want the
pager to run as a separate process, but we also need to wait for it at
the end).

Though I suspect for most cases, external scripts are really paging the
output of some other command anyway. So it would be enough to provide
something like:

  if git pager --query bisect.log
  then
	bisect_log | git pager bisect.log
  else
	bisect_log
  fi

(you can lose the "--query" form if you don't mind _always_ piping
through "cat" when there's no pager in use, but that seems like a
pointless inefficiency).

So I think it's all workable, and for the most part would even remain
backwards compatible, with the exception that "pager.foo" would not work
for a third-party "git-foo" until the author updates it to call "git
pager".

I don't have a particular plan to work on it anytime soon, but maybe
somebody could pick it up as relatively low-hanging fruit.

-Peff