Thread (50 messages) flat view 50 messages, 3 authors, 2017-08-04

Re: [PATCH v2 05/10] git.c: provide setup_auto_pager()

From: Jeff King <hidden>
Date: 2017-07-31 03:34:36

On Mon, Jul 17, 2017 at 10:10:47PM +0200, Martin Ågren wrote:
The previous patch introduced a way for builtins to declare that they
will take responsibility for handling the `pager.foo`-config item. (See
the commit message of that patch for why that could be useful.)

Provide setup_auto_pager(), which builtins can call in order to handle
`pager.<cmd>`, including possibly starting the pager. Make this function
don't do anything if a pager has already been started, as indicated by
use_pager or pager_in_use().

Whenever this function is called from a builtin, git.c will already have
called commit_pager_choice(). Since commit_pager_choice() treats the
special value -1 as "punt" or "not yet decided", it is not a problem
that we might end up calling commit_pager_choice() once in git.c and
once (or more) in the builtin. Make the new function use -1 in the same
way and document it as "punt".
At first I wasn't sure if it would ever make sense to use "-1" here. The
"punt" that happens in earlier calls to commit_pager_choice() is there
because we might adjust our decision later. And this would generally be
the final decision, I would think. So I'd be surprised if we had
anything besides "0" or "1" in the "def" argument.

But thinking on it, the most plausible case is something like:

  setup_auto_pager("foo", -1);
  ...
  /* fallback to some historical compatibility name */
  setup_auto_pager("bar", 0);

And it's important for the "-1" there to be a true punt, and not do
anything in commit_pager_choice(). So it's probably worth documenting
the "-1" behavior as you did here as a possible value for "def".

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help