Re: [PATCH 6/7] tag: make git tag -l consider new config `pager.tag.list`

2017-07-11 Thread Martin Ågren
On 11 July 2017 at 12:41, Jeff King  wrote:
> On Mon, Jul 10, 2017 at 11:55:19PM +0200, Martin Ågren wrote:
>
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index e0f129872..e96ef7d70 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -446,6 +446,8 @@ int cmd_tag(int argc, const char **argv, const char 
>> *prefix)
>>
>>   argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
>>
>> + if (cmdmode == 'l')
>> + setup_auto_pager("tag.list", 0);
>>   setup_auto_pager("tag", 0);
>
> Ideally this would kick in whenever we are in list mode, even if the
> user didn't say "-l". So:
>
>   $ git tag
>
> should probably respect the list config. Likewise, certain options like
> "--contains" trigger list mode. I think the pager setup could just be
> bumped a few lines down, past the "if (!cmdmode)" block that sets up
> those defaults.

Oops, that's embarassing. Thanks. That means the paging starts
slightly later, but what happens in those few lines looks fairly
trivial, so there shouldn't be any real difference in behavior. I'll
add one or two tests.

Martin


Re: [PATCH 6/7] tag: make git tag -l consider new config `pager.tag.list`

2017-07-11 Thread Jeff King
On Mon, Jul 10, 2017 at 11:55:19PM +0200, Martin Ågren wrote:

> diff --git a/builtin/tag.c b/builtin/tag.c
> index e0f129872..e96ef7d70 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -446,6 +446,8 @@ int cmd_tag(int argc, const char **argv, const char 
> *prefix)
>  
>   argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
>  
> + if (cmdmode == 'l')
> + setup_auto_pager("tag.list", 0);
>   setup_auto_pager("tag", 0);

Ideally this would kick in whenever we are in list mode, even if the
user didn't say "-l". So:

  $ git tag

should probably respect the list config. Likewise, certain options like
"--contains" trigger list mode. I think the pager setup could just be
bumped a few lines down, past the "if (!cmdmode)" block that sets up
those defaults.

-Peff