From: Kamil Domański Date: Wed, 18 Sep 2019 21:44:39 GMT Subject: Re: [PATCH] ls-remote: create '--count' option Message-ID: <21455bdb-431e-2842-0618-2f36a18bb60e@domanski.co> In-Reply-To: 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 >> --- >> 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=] >> - [-q | --quiet] [--exit-code] [--get-url] [--sort=] >> - [--symref] [ [...]] >> +'git ls-remote' [--count=] [--heads] [--tags] [--refs] >> + [--upload-pack=] [-q | --quiet] [--exit-code] [--get-url] >> + [--sort=] [--symref] [ [...]] > 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 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=:: >> + By default the command shows all refs that match >> + ``. 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=` 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