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".
The discussion mentioned above discusses various approaches. It also notes how the current pager.foo-configuration conflates _whether_ and _how_ to run a pager. Arguably, this series paints ourselves even further into that corner. The idea of `pager.foo.command` and `pager.foo.enabled` has been mentioned and this series might make such an approach slightly less clean, conceptually. We could introduce `paging.foo` as a "yes"/"no"/"maybe" to go alongside `pager.foo` which is then "less"/"cat"/.... That's definitely outside this series, but should not be prohibited by it...
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.
Martin
[1] https://public-inbox.org/git/nrmbrl$hsk$1@blaine.gmane.org/T/
Martin Ågren (7):
api-builtin.txt: document SUPPORT_SUPER_PREFIX
git.c: let builtins opt for handling `pager.foo` themselves
git.c: provide setup_auto_pager()
t7006: add tests for how git tag paginates
tag: handle `pager.tag`-configuration within the builtin
tag: make git tag -l consider new config `pager.tag.list`
tag: make git tag -l use pager by default
Documentation/git-tag.txt | 4 +++
Documentation/technical/api-builtin.txt | 10 ++++++
builtin.h | 14 +++++++++
builtin/tag.c | 4 +++
git.c | 44 +++++++++++++++++++++++++--
t/t7006-pager.sh | 54 +++++++++++++++++++++++++++++++++
6 files changed, 127 insertions(+), 3 deletions(-)