Re: [PATCH] ls-remote: create '--count' option

["Kamil Domański" <kamil@domanski.co>]

On 9/18/19 8:28 AM, Johannes Sixt wrote:
> Am 18.09.19 um 02:11 schrieb Kamil Domański:
>> Create a '--count' option for ls-remote, based on the one from
>> for-each-ref. This allows e.g. to return only the first result
>> from a sorted list of refs.
>>
>> Signed-off-by: Kamil Domański <kamil@domanski.co>
>> ---
>>   Documentation/git-ls-remote.txt | 11 ++++++++---
>>   builtin/ls-remote.c             | 16 ++++++++++++----
>>   t/t5512-ls-remote.sh            |  9 +++++++++
>>   3 files changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
>> index 0b057cbb10..5adc1d676e 100644
>> --- a/Documentation/git-ls-remote.txt
>> +++ b/Documentation/git-ls-remote.txt
>> @@ -9,9 +9,9 @@ git-ls-remote - List references in a remote repository
>>   SYNOPSIS
>>   --------
>>   [verse]
>> -'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
>> -	      [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
>> -	      [--symref] [<repository> [<refs>...]]
>> +'git ls-remote' [--count=<count>] [--heads] [--tags] [--refs]
>> +	      [--upload-pack=<exec>] [-q | --quiet] [--exit-code] [--get-url]
>> +	      [--sort=<key>] [--symref] [<repository> [<refs>...]]
> It is understandable that the new option is important to _you_, but it
> does not seem important enough that it must be the first in the list.
> Please add it between --symref and <repository>
The assumption is quite unnecessary. I merely tried to reflect the 
position of this parameter in the 'for-each-ref' command, on which I 
based the patch.
>>   
>>   DESCRIPTION
>>   -----------
>> @@ -21,6 +21,11 @@ commit IDs.
>>   
>>   OPTIONS
>>   -------
>> +--count=<count>::
>> +	By default the command shows all refs that match
>> +	`<pattern>`.  This option makes it stop after showing
>> +	that many refs.
> Is the meaning of this option perhaps:
>
>      Stops after the specified count of refs have been listed.
>      If `--sort=<key>` is specified as well, refs are counted
>      after sorting; otherwise, it is unspecified which subset
>      of is listed.

Similarly, I merely copied the description used by 'for-each-ref'. I 
like your version as well, since it clarifies the relation to sorting. 
Should the description for 'for-each-ref' be changed as well then?

> I do not know whether the "otherwise" part would be true (check it!),
> but I am pretty certain that the "If" part must be true, otherwise the
> option would be pointless.

Yes, both in 'for-each-ref' as well as this patch, cutting off the 
results takes place after the whole set has been already sorted. As for 
the "otherwise" part, it appears that 'for-each-ref' defaults to sorting 
by 'refname' (through ref_default_sorting() ) if no alternative sorting 
is provided, while 'ls-remote' does no such defaulting. Do you figure it 
would be a good idea to add another patch which would introduce a 
similar default in 'ls-remote'?


Regards,

Kamil