Re: [PATCH v2 00/10] tag: only respect `pager.tag` in list-mode

Martin Ågren [off-list ref] writes:

This is the second version of "[PATCH 0/7] tag: more fine-grained
pager-configuration" [1]. That series introduced `pager.tag.list` to
address the fact that `pager.tag` can be useful with `git tag -l` but
actively hostile with `git tag -a`. Thanks to Junio, Peff and Brandon
for helpful feedback.

After that feedback, v2 drops `pager.tag.list` and instead teaches
`git tag` to only consider `pager.tag` in list-mode, as suggested by
Peff.

Patches 1-3/10 replace patch 1/7. They move Documentation/technical/
api-builtin.txt into builtin.h, tweak the formatting and bring it up to
date. I may have gone overboard making this 3 patches...

Patches 4-7/10 correspond to patches 2-5/7. `setup_auto_pager()' is now
much simpler since we do not need to handle "tag.list" with a clever
fallback strategy. IGNORE_PAGER_CONFIG is now called DELAY_PAGER_CONFIG.
I now check with pager_in_use() and I moved the handling of `pager.tag`
a bit further down.

I tend to agree with you that 1-3/10 may be better off being a
single patch (or 3/10 dropped, as Brandon is working on losing it
nearby).  I would have expected 7-8/10 to be a single patch, as by
the time a reader reaches 07/10, because of the groundwork laid by
04-06/10, it is obvious that the general direction is to allow the
caller, i.e. cmd_tag(), to make a call to setup_auto_pager() only in
some but not all circumstances, and 07/10 being faithful to the
original behaviour (only to be updated in 08/10) is somewhat counter
intuitive.  It is not wrong per-se; it was just unexpected. 

Patches 8-9/10 teach `git tag` to only respect `pager.tag` in list-mode
and flip the default value for that config to "on".

Patch 10/10 is somewhat similar to a hunk in patch 2/7, but is now a
bug-fix instead of a feature. It teaches `execv_dashed_external()` not
to check `pager.foo` when launching `git-foo` where foo is a builtin.
I waffled about where to put this patch. Putting it earlier in the
series as a preparatory step, I couldn't come up with a way of writing a
test. So patch 8/10 introduces a `test_expect_failure` which this patch
then fixes.

I haven't thought about ramifications of 9-10/10 to make a comment
yet, but overall the series was a pleasant read.

Thanks.