{"thread":{"id":"51871","subject":"[PATCH] ls-remote: create '--count' option","startedAt":"2019-09-18T00:12:29Z","lastAt":"2019-09-19T18:40:11Z","messageCount":5,"participants":["Kamil Domański","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"382533","messageId":"20190918001134.20776-1-kamil@domanski.co","threadId":"51871","inReplyTo":null,"subject":"[PATCH] ls-remote: create '--count' option","fromName":"Kamil Domański","fromEmail":"kamil@domanski.co","sentAt":"2019-09-18T00:11:34Z","receivedAt":"2019-09-18T00:12:29Z","isPatch":true,"sender":{"key":"kamil@domanski.co","avatar":null},"body":"Create a '--count' option for ls-remote, based on the one from\nfor-each-ref. This allows e.g. to return only the first result\nfrom a sorted list of refs.\n\nSigned-off-by: Kamil Domański \n---\n Documentation/git-ls-remote.txt | 11 ++++++++---\n builtin/ls-remote.c | 16 ++++++++++++----\n t/t5512-ls-remote.sh | 9 +++++++++\n 3 files changed, 29 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\nindex 0b057cbb10..5adc1d676e 100644\n--- a/Documentation/git-ls-remote.txt\n+++ b/Documentation/git-ls-remote.txt\n@@ -9,9 +9,9 @@ git-ls-remote - List references in a remote repository\n SYNOPSIS\n --------\n [verse]\n-'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]\n-\t [-q | --quiet] [--exit-code] [--get-url] [--sort=]\n-\t [--symref] [ [...]]\n+'git ls-remote' [--count=] [--heads] [--tags] [--refs]\n+\t [--upload-pack=] [-q | --quiet] [--exit-code] [--get-url]\n+\t [--sort=] [--symref] [ [...]]\n \n DESCRIPTION\n -----------\n@@ -21,6 +21,11 @@ commit IDs.\n \n OPTIONS\n -------\n+--count=::\n+\tBy default the command shows all refs that match\n+\t``. This option makes it stop after showing\n+\tthat many refs.\n+\n -h::\n --heads::\n -t::\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 6ef519514b..482b8c5b3c 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -8,7 +8,7 @@\n static const char * const ls_remote_usage[] = {\n \tN_(\"git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]\\n\"\n \t \" [-q | --quiet] [--exit-code] [--get-url]\\n\"\n-\t \" [--symref] [ [...]]\"),\n+\t \" [--symref] [--count=] [ [...]]\"),\n \tNULL\n };\n \n@@ -43,6 +43,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tint status = 0;\n \tint show_symref_target = 0;\n+\tint maxcount = 0;\n \tconst char *uploadpack = NULL;\n \tconst char **pattern = NULL;\n \tstruct argv_array ref_prefixes = ARGV_ARRAY_INIT;\n@@ -55,7 +56,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tstruct ref_array ref_array;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n \n-\tstruct option options[] = {\n+\tstruct option opts[] = {\n \t\tOPT__QUIET(&quiet, N_(\"do not print remote URL\")),\n \t\tOPT_STRING(0, \"upload-pack\", &uploadpack, N_(\"exec\"),\n \t\t\t N_(\"path of git-upload-pack on the remote host\")),\n@@ -73,14 +74,19 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t\t 2, PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"symref\", &show_symref_target,\n \t\t\t N_(\"show underlying ref in addition to the object pointed by it\")),\n+\t\tOPT_INTEGER( 0 , \"count\", &maxcount, N_(\"show only matched refs\")),\n \t\tOPT_STRING_LIST('o', \"server-option\", &server_options, N_(\"server-specific\"), N_(\"option to transmit\")),\n \t\tOPT_END()\n \t};\n \n \tmemset(&ref_array, 0, sizeof(ref_array));\n \n-\targc = parse_options(argc, argv, prefix, options, ls_remote_usage,\n+\targc = parse_options(argc, argv, prefix, opts, ls_remote_usage,\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (maxcount < 0) {\n+\t\terror(\"invalid --count argument: `%d'\", maxcount);\n+\t\tusage_with_options(ls_remote_usage, opts);\n+\t}\n \tdest = argv[0];\n \n \tif (argc > 1) {\n@@ -138,7 +144,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tif (sorting)\n \t\tref_array_sort(sorting, &ref_array);\n \n-\tfor (i = 0; i < ref_array.nr; i++) {\n+\tif (!maxcount || ref_array.nr < maxcount)\n+\t\tmaxcount = ref_array.nr;\n+\tfor (i = 0; i < maxcount; i++) {\n \t\tconst struct ref_array_item *ref = ref_array.items[i];\n \t\tif (show_symref_target && ref->symref)\n \t\t\tprintf(\"ref: %s\\t%s\\n\", ref->symref, ref->refname);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 43e1d8d4d2..5c27a126bd 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -75,6 +75,15 @@ test_expect_success 'ls-remote --sort=\"-refname\" --tags self' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'ls-remote --count=2 --sort=\"refname\" --tags self' '\n+\tcat >expect <<-EOF &&\n+\t$(git rev-parse mark)\trefs/tags/mark\n+\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n+\tEOF\n+\tgit ls-remote --count=2 --sort=\"refname\" --tags self >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'dies when no remote specified and no default remotes found' '\n \ttest_must_fail git ls-remote\n '\n-- \n2.23.0\n\n"},{"id":"382540","messageId":"f643547f-54e0-fe4f-d8e5-95445431faf3@kdbg.org","threadId":"51871","inReplyTo":"20190918001134.20776-1-kamil@domanski.co","subject":"Re: [PATCH] ls-remote: create '--count' option","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-18T06:28:11Z","receivedAt":"2019-09-18T06:28:18Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 18.09.19 um 02:11 schrieb Kamil Domański:\n> Create a '--count' option for ls-remote, based on the one from\n> for-each-ref. This allows e.g. to return only the first result\n> from a sorted list of refs.\n> \n> Signed-off-by: Kamil Domański \n> ---\n> Documentation/git-ls-remote.txt | 11 ++++++++---\n> builtin/ls-remote.c | 16 ++++++++++++----\n> t/t5512-ls-remote.sh | 9 +++++++++\n> 3 files changed, 29 insertions(+), 7 deletions(-)\n> \n> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\n> index 0b057cbb10..5adc1d676e 100644\n> --- a/Documentation/git-ls-remote.txt\n> +++ b/Documentation/git-ls-remote.txt\n> @@ -9,9 +9,9 @@ git-ls-remote - List references in a remote repository\n> SYNOPSIS\n> --------\n> [verse]\n> -'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]\n> -\t [-q | --quiet] [--exit-code] [--get-url] [--sort=]\n> -\t [--symref] [ [...]]\n> +'git ls-remote' [--count=] [--heads] [--tags] [--refs]\n> +\t [--upload-pack=] [-q | --quiet] [--exit-code] [--get-url]\n> +\t [--sort=] [--symref] [ [...]]\n\nIt is understandable that the new option is important to _you_, but it\ndoes not seem important enough that it must be the first in the list.\nPlease add it between --symref and \n\n> \n> DESCRIPTION\n> -----------\n> @@ -21,6 +21,11 @@ commit IDs.\n> \n> OPTIONS\n> -------\n> +--count=::\n> +\tBy default the command shows all refs that match\n> +\t``. This option makes it stop after showing\n> +\tthat many refs.\n\nIs the meaning of this option perhaps:\n\n Stops after the specified count of refs have been listed.\n If `--sort=` is specified as well, refs are counted\n after sorting; otherwise, it is unspecified which subset\n of is listed.\n\nI do not know whether the \"otherwise\" part would be true (check it!),\nbut I am pretty certain that the \"If\" part must be true, otherwise the\noption would be pointless.\n\nThe comment about the ordering of this paragraph at the very beginning\nof the option list applies here, too, because the list is not sorted\nalphabetically.\n\n-- Hannes\n"},{"id":"382562","messageId":"xmqqo8zh5xab.fsf@gitster-ct.c.googlers.com","threadId":"51871","inReplyTo":"f643547f-54e0-fe4f-d8e5-95445431faf3@kdbg.org","subject":"Re: [PATCH] ls-remote: create '--count' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-18T17:28:28Z","receivedAt":"2019-09-18T17:28:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 18.09.19 um 02:11 schrieb Kamil Domański:\n>> Create a '--count' option for ls-remote, based on the one from\n>> for-each-ref. This allows e.g. to return only the first result\n>> from a sorted list of refs.\n>> \n>> Signed-off-by: Kamil Domański \n>> ---\n>> Documentation/git-ls-remote.txt | 11 ++++++++---\n>> builtin/ls-remote.c | 16 ++++++++++++----\n>> t/t5512-ls-remote.sh | 9 +++++++++\n>> 3 files changed, 29 insertions(+), 7 deletions(-)\n>> \n>> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\n>> index 0b057cbb10..5adc1d676e 100644\n>> --- a/Documentation/git-ls-remote.txt\n>> +++ b/Documentation/git-ls-remote.txt\n>> @@ -9,9 +9,9 @@ git-ls-remote - List references in a remote repository\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> -'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]\n>> -\t [-q | --quiet] [--exit-code] [--get-url] [--sort=]\n>> -\t [--symref] [ [...]]\n>> +'git ls-remote' [--count=] [--heads] [--tags] [--refs]\n>> +\t [--upload-pack=] [-q | --quiet] [--exit-code] [--get-url]\n>> +\t [--sort=] [--symref] [ [...]]\n>\n> It is understandable that the new option is important to _you_, but it\n> does not seem important enough that it must be the first in the list.\n> Please add it between --symref and \n>\n>> \n>> DESCRIPTION\n>> -----------\n>> @@ -21,6 +21,11 @@ commit IDs.\n>> \n>> OPTIONS\n>> -------\n>> +--count=::\n>> +\tBy default the command shows all refs that match\n>> +\t``. This option makes it stop after showing\n>> +\tthat many refs.\n>\n> Is the meaning of this option perhaps:\n>\n> Stops after the specified count of refs have been listed.\n> If `--sort=` is specified as well, refs are counted\n> after sorting; otherwise, it is unspecified which subset\n> of is listed.\n>\n> I do not know whether the \"otherwise\" part would be true (check it!),\n> but I am pretty certain that the \"If\" part must be true, otherwise the\n> option would be pointless.\n>\n> The comment about the ordering of this paragraph at the very beginning\n> of the option list applies here, too, because the list is not sorted\n> alphabetically.\n\nAll sensible comments and suggestions. I am not sure what's so hard\nto pipe the output to \"head -n 20\" or something like that, though.\n"},{"id":"382575","messageId":"21455bdb-431e-2842-0618-2f36a18bb60e@domanski.co","threadId":"51871","inReplyTo":"f643547f-54e0-fe4f-d8e5-95445431faf3@kdbg.org","subject":"Re: [PATCH] ls-remote: create '--count' option","fromName":"Kamil Domański","fromEmail":"kamil@domanski.co","sentAt":"2019-09-18T21:44:39Z","receivedAt":"2019-09-18T21:44:44Z","isPatch":true,"sender":{"key":"kamil@domanski.co","avatar":null},"body":"On 9/18/19 8:28 AM, Johannes Sixt wrote:\n> Am 18.09.19 um 02:11 schrieb Kamil Domański:\n>> Create a '--count' option for ls-remote, based on the one from\n>> for-each-ref. This allows e.g. to return only the first result\n>> from a sorted list of refs.\n>>\n>> Signed-off-by: Kamil Domański \n>> ---\n>> Documentation/git-ls-remote.txt | 11 ++++++++---\n>> builtin/ls-remote.c | 16 ++++++++++++----\n>> t/t5512-ls-remote.sh | 9 +++++++++\n>> 3 files changed, 29 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt\n>> index 0b057cbb10..5adc1d676e 100644\n>> --- a/Documentation/git-ls-remote.txt\n>> +++ b/Documentation/git-ls-remote.txt\n>> @@ -9,9 +9,9 @@ git-ls-remote - List references in a remote repository\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> -'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]\n>> -\t [-q | --quiet] [--exit-code] [--get-url] [--sort=]\n>> -\t [--symref] [ [...]]\n>> +'git ls-remote' [--count=] [--heads] [--tags] [--refs]\n>> +\t [--upload-pack=] [-q | --quiet] [--exit-code] [--get-url]\n>> +\t [--sort=] [--symref] [ [...]]\n> It is understandable that the new option is important to _you_, but it\n> does not seem important enough that it must be the first in the list.\n> Please add it between --symref and \nThe assumption is quite unnecessary. I merely tried to reflect the \nposition of this parameter in the 'for-each-ref' command, on which I \nbased the patch.\n>> \n>> DESCRIPTION\n>> -----------\n>> @@ -21,6 +21,11 @@ commit IDs.\n>> \n>> OPTIONS\n>> -------\n>> +--count=::\n>> +\tBy default the command shows all refs that match\n>> +\t``. This option makes it stop after showing\n>> +\tthat many refs.\n> Is the meaning of this option perhaps:\n>\n> Stops after the specified count of refs have been listed.\n> If `--sort=` is specified as well, refs are counted\n> after sorting; otherwise, it is unspecified which subset\n> of is listed.\n\nSimilarly, I merely copied the description used by 'for-each-ref'. I \nlike your version as well, since it clarifies the relation to sorting. \nShould the description for 'for-each-ref' be changed as well then?\n\n> I do not know whether the \"otherwise\" part would be true (check it!),\n> but I am pretty certain that the \"If\" part must be true, otherwise the\n> option would be pointless.\n\nYes, both in 'for-each-ref' as well as this patch, cutting off the \nresults takes place after the whole set has been already sorted. As for \nthe \"otherwise\" part, it appears that 'for-each-ref' defaults to sorting \nby 'refname' (through ref_default_sorting() ) if no alternative sorting \nis provided, while 'ls-remote' does no such defaulting. Do you figure it \nwould be a good idea to add another patch which would introduce a \nsimilar default in 'ls-remote'?\n\n\nRegards,\n\nKamil\n\n\n\n"},{"id":"382600","messageId":"c839c38e-00ac-a1de-a246-8e92f16cfb4a@kdbg.org","threadId":"51871","inReplyTo":"21455bdb-431e-2842-0618-2f36a18bb60e@domanski.co","subject":"Re: [PATCH] ls-remote: create '--count' option","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-19T18:40:06Z","receivedAt":"2019-09-19T18:40:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 18.09.19 um 23:44 schrieb Kamil Domański:\n> On 9/18/19 8:28 AM, Johannes Sixt wrote:\n>> Am 18.09.19 um 02:11 schrieb Kamil Domański:\n>>>     DESCRIPTION\n>>>   -----------\n>>> @@ -21,6 +21,11 @@ commit IDs.\n>>>     OPTIONS\n>>>   -------\n>>> +--count=::\n>>> +    By default the command shows all refs that match\n>>> +    ``.  This option makes it stop after showing\n>>> +    that many refs.\n>> Is the meaning of this option perhaps:\n>>\n>>      Stops after the specified count of refs have been listed.\n>>      If `--sort=` is specified as well, refs are counted\n>>      after sorting; otherwise, it is unspecified which subset\n>>      of is listed.\n> \n> Similarly, I merely copied the description used by 'for-each-ref'. I\n> like your version as well, since it clarifies the relation to sorting.\n> Should the description for 'for-each-ref' be changed as well then?\n\nI am neutral. If you do it, then it should not happen in the same patch\nas this change.\n\n> ... it appears that 'for-each-ref' defaults to sorting\n> by 'refname' (through ref_default_sorting() ) if no alternative sorting\n> is provided, while 'ls-remote' does no such defaulting. Do you figure it\n> would be a good idea to add another patch which would introduce a\n> similar default in 'ls-remote'?\n\nI don't think so. I would think that in the case of ls-remote it is\npreferable to stream refs to the output as soon as they are received\nfrom the remote. If refs must be sorted, it would have to wait until the\nremote has sent all refs before it can do anything.\n\nThat said, Junio's question remains open: why is piping though head -n\nnot good enough?\n\n-- Hannes\n"}]}