Re: [PATCH] ls-remote: create '--count' option
- From
- Kamil Domański <kamil@domanski.co>
- Date
- Sep 18, 2019, 21:44 UTC
- Message-ID
- <21455bdb-431e-2842-0618-2f36a18bb60e@domanski.co>
- In-Reply-To
- <f643547f-54e0-fe4f-d8e5-95445431faf3@kdbg.org>
On 9/18/19 8:28 AM, Johannes Sixt wrote:
Show 29 quoted lines
> 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.
Show 17 quoted lines
>> >> 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