Martin Ågren <martin.agren@xxxxxxxxx> 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.
