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

From: Jeff King <peff@peff.net>
Date: Sep 23, 2016, 03:49 UTC
Message-ID: <20160923034928.dneomzokebkuxg4z@sigill.intra.peff.net>
In-Reply-To: <xmqqfuor6ee3.fsf@gitster.mtv.corp.google.com>

On Thu, Sep 22, 2016 at 10:19:32AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> The level at which configurability happens might be one issue
> (i.e. you may want different pager for two operating modes for the
> same command, hence your need to use "tag.list" not just "tag"), but
> I think another issue is that it conflates if the output need to be
> paged (on/off) and what pager should be used when the output is
> paged.  When we see that a user sets "pager.tag", we should not have
> made it an instruction to Git that _all_ output from "git tag" must
> be paged.

Yes, we could have done it the other way, but I think this was a natural consequence of implementing it git.c. It _only_ knows about "all output from git-tag" and nothing else.

At any rate, I do not see much point in moving away from it even if we change the underlying implementation to be more flexible, if only because it would be a gratuitous incompatibility.

> So I think we are fundamentally on the same page; it is just you are
> aiming higher than I was, but we both recognize the need for separate 
> codepaths in a single command to decide if the output should be paged.

Yeah. In my examples there are really two proposed improvements:

  1. The decision over whether and when to start a pager is pushed down
     from git.c into individual commands.

  2. As a side effect of (1), commands must declare "this is who I am"
     to look up the correct config.  But "who I am" no longer needs to
     be a whole command, so we are free to slice up the namespace more
     finely. But we do not have to.

     This might also be an opportunity to add more conditions. Like "run
     the pager if I am doing log output with -p, but not otherwise" or
     something. I dunno. That does not sound useful to me, but maybe
     somebody else would find it so.

And I think you are getting at a (3), which is something like:

  3. The config namespace can be made richer, so that "whether" and
     "how" are split. E.g., "pager.log.command" and "pager.log.enabled"
     or something.

     I do not mind that, but we would probably want to keep "pager.log"
     for compatibility, at which point I wonder if the new system is
     worth the bother.

-Peff