From: Martin Ågren <martin.agren@gmail.com>
Date: Fri, 04 Aug 2017 04:21:47 GMT
Subject: Re: [PATCH v3 0/7] tag: only respect `pager.tag` in list-mode
Message-ID: <CAN0heSpzg4jCMetyhdpQn2Zz7nDhv1X14SLQ26WvnRDe9NYCCg@mail.gmail.com>
In-Reply-To: <20170803192947.7x5tadwb7lxermdk@sigill.intra.peff.net>

On 3 August 2017 at 21:29, Jeff King <peff@peff.net> 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.

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.

Martin