Re: [PATCH 0/7] tag: more fine-grained pager-configuration

[Junio C Hamano <gitster@pobox.com>]

Martin Ågren <martin.agren@gmail.com> writes:

> Using, e.g., `git -c pager.tag tag -a new-tag` results in errors
> such as "Vim: Warning: Output is not to a terminal" and a garbled
> terminal. A user who makes use of `git tag -a` and `git tag -l`
> will probably choose not to configure `pager.tag` or to set it to
> "no", so that `git tag -a` will actually work, at the cost of not
> getting the pager with `git tag -l`.
>
> In the discussion at [1], it was brought up that 1) the individual
> builtins should be in charge of setting up the paging (as opposed
> to git.c which has no knowledge about what the command is about to
> do) and that 2) there could then be a configuration
> `pager.tag.list` to address the specific problem of `git tag`.
>
> This is an attempt to implement something like that. I decided to
> let `pager.tag.list` fall back to `pager.tag` before falling back
> to "on". The default for `pager.tag` is still "off". I can see how
> that might seem confusing. However, my argument is that it would
> be awkward for `git tag -l` to ignore `pager.tag` -- we are after
> all running a subcommand of `git tag`. Also, this avoids a
> regression for someone who has set `pager.tag` and uses `git tag
> -l`.
>
> I am not moving all builtins to handling the pager on their own,
> but instead introduce a flag IGNORE_PAGER_CONFIG and use it only
> with the tag builtin. That means there's another flag to reason
> about, but it avoids moving all builtins to handling the paging
> themselves just to make one of them do something more "clever".

All of the above smells like taking us in a sensible direction.  I
agree with these design decisions described in the above, i.e.
'pager.tag.list falling back to pager.tag', making this an opt-in to
make code transition easier.

Even though it is purely internal thing, IGNORE_PAGER_CONFIG
probably is a bit confusion-inducing name.  After all, the
subcommand specific configuration is not being ignored---we are
merely delaying our reaction to it---instead of acting on it inside
git.c without giving the subcommand a chance to make a decision, we
are still letting (and we do expect) the subcommand to react to it.

But that is a fairly minor thing we can fix.

> A review would be much appreciated. Comments on the way I
> structured the series would be just as welcome as ones on the
> final result.

I see you CC'ed Peff who's passionate in this area, so these patches
are in good hands already ;-) I briefly skimmed your patches myself,
and did not spot anything glaringly wrong.

Thanks.