On Fri, Aug 04, 2017 at 06:21:47AM +0200, Martin Ågren wrote: > On 3 August 2017 at 21:29, Jeff King <peff@xxxxxxxx> wrote: > > On Wed, Aug 02, 2017 at 09:40:48PM +0200, Martin Ågren wrote: > > > >> This is the third version of my attempt to make `pager tag` useful (v1 > >> at [1], v2 at [2]). Thanks to Junio and Peff for comments on v2. > > > > This looks good to me overall. One minor question from the interdiff: > > > >> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh > >> index 8b2ffb1aa..9128ec5ac 100755 > >> --- a/t/t7006-pager.sh > >> +++ b/t/t7006-pager.sh > >> @@ -162,7 +162,7 @@ test_expect_success TTY 'git tag with no args defaults to paging' ' > >> test_expect_success TTY 'git tag with no args respects pager.tag' ' > >> # no args implies -l so this should page like -l > >> rm -f paginated.out && > >> - test_terminal git -c pager.tag=no tag && > >> + test_terminal git -c pager.tag=false tag && > >> ! test -e paginated.out > >> ' > > > > These should behave the same, right? So this is just a style/consistency > > fix, not a bugfix? > > Right. I realized I was using "false" everywhere else. It wouldn't have hurt > to exercise the config-parsing a tiny bit differently, but I assume that's > already being done explicitly in some other test, so I went for consistency. Right, that makes perfect sense. Thanks for confirming. > Thanks for all the feedback and thoughts throughout the different versions > of this series. It changed quite a bit since v1, so thanks a lot. You're very welcome. I hope we see more patches from you in the future. :) -Peff