Re: [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}

From: Jeff King <peff@peff.net>
Date: Nov 6, 2017, 10:48 UTC
Message-ID: <20171106104801.73n27fgmn43y6m3v@sigill.intra.peff.net>
In-Reply-To: <cover.1509879269.git.martin.agren@gmail.com>

On Sun, Nov 05, 2017 at 12:58:18PM +0100, Martin Ågren wrote:

Show 13 quoted lines

> On 4 November 2017 at 10:28, Jeff King <peff@peff.net> wrote:
> >  - the pager.<cmd> config is mis-designed, because our config keys
> >    cannot represent all possible command names (e.g., case folding and
> >    illegal characters). This should be pager.<cmd>.enable or similar.
> >    Some discussion in (this message and the surrounding thread):
> >
> >      https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/
> >
> >    But I think you could find more by searching the archive.
> 
> I'm posting four patches I have on this to save others from redoing my
> work and findings. These patches feel a bit incomplete, which is why I
> put them to the side some time ago (and eventually forgot about them).

Thanks for sharing these; it's nice if we can avoid duplicating work. We'll see if somebody at the event wants to pick up this topic.

Show 6 quoted lines

> In particular, they do not teach `--paginate` to use the pager
> configured by `pager.foo.command`. It is already now possible to use
> `pager.foo` to say "I don't want you to page, but if I later give you
> `pager.foo=true`, this is the pager I want you to use". That does not
> work with `--paginate`, but this can all be explained -- indeed, we
> document that `--paginate` overrules `pager.foo`.

Hmm. I think the current behavior is actually buggy. I assume the documentation you mean is from git.txt:

  --paginate::
          Pipe all output into 'less' (or if set, $PAGER) if standard
          output is a terminal.  This overrides the `pager.<cmd>`
          configuration options (see the "Configuration Mechanism" section
          below).

That comes from 06300d9753 (git.1: Clarify the behavior of the --paginate option, 2010-02-14). But I think that commit was just trying to clarify that "--paginate" overrides the defaults and config, but not does say "always paginate".

I suspect nobody really noticed it in practice because once you've configured "pager.foo", there's basically no need to ever use "--paginate".

Show 6 quoted lines

> If we teach `--paginate` to respect `pager.foo.command`, it seems that
> we would either 1) introduce a small (and possibly hard to understand
> and explain) difference between the old-style and the new-style
> pager-configuration or 2) knowingly change the behavior of `--paginate`
> with `pager.foo` or 3) knowingly change the behavior of
> `pager.foo=false` as documented in the first patch.

I think I'm suggesting (2), then.

-Peff