{"thread":{"id":"57824","subject":"[PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","startedAt":"2022-04-30T13:20:04Z","lastAt":"2022-05-16T15:39:15Z","messageCount":26,"participants":["Abhradeep Chakraborty via GitGitGadget","Junio C Hamano","Abhradeep Chakraborty","Philip Oakley","Philippe Blain","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454678","messageId":"pull.1227.git.1651324796892.gitgitgadget@gmail.com","threadId":"57824","inReplyTo":null,"subject":"[PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-04-30T13:19:56Z","receivedAt":"2022-04-30T13:20:04Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"From: Abhradeep Chakraborty \n\n`git remote -v` (`--verbose`) lists down the names of remotes along with\ntheir urls. It would be beneficial for users to also specify the filter\ntypes for promisor remotes. Something like this -\n\n\torigin\tremote-url (fetch) [blob:none]\n\torigin\tremote-url (push)\n\nTeach `git remote -v` to also specify the filters for promisor remotes.\n\nSigned-off-by: Abhradeep Chakraborty \n---\n builtin/remote.c: teach -v to list filters for promisor remotes\n \n Fixes #1211 [1]\n \n [1] https://github.com/gitgitgadget/git/issues/1211\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1227%2FAbhra303%2Fpromisor_remote-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1227/Abhra303/promisor_remote-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1227\n\n builtin/remote.c | 8 ++++++++\n t/t5616-partial-clone.sh | 11 +++++++++++\n 2 files changed, 19 insertions(+)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5f4cde9d784..95e28b534f4 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1190,7 +1190,15 @@ static int get_one_entry(struct remote *remote, void *priv)\n \tint i, url_nr;\n \n \tif (remote->url_nr > 0) {\n+\t\tstruct strbuf promisor_config = STRBUF_INIT;\n+\t\tconst char *partial_clone_filter = NULL;\n+\n+\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n \t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n+\t\t\tstrbuf_addf(&url_buf, \" [%s]\", partial_clone_filter);\n+\n+\t\tstrbuf_release(&promisor_config);\n \t\tstring_list_append(list, remote->name)->util =\n \t\t\t\tstrbuf_detach(&url_buf, NULL);\n \t} else\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 4a3778d04a8..bf8f3644d3c 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -49,6 +49,17 @@ test_expect_success 'do partial clone 1' '\n \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n '\n \n+test_expect_success 'filters for promisor remotes is listed by git remote -v' '\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"[blob:none]\" out &&\n+\n+\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"[object:type=commit]\" out &&\n+\trm -rf pc2\n+'\n+\n test_expect_success 'verify that .promisor file contains refs fetched' '\n \tls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&\n \ttest_line_count = 1 promisorlist &&\n\nbase-commit: 0f828332d5ac36fc63b7d8202652efa152809856\n-- \ngitgitgadget\n"},{"id":"454690","messageId":"xmqqczgy6zk5.fsf@gitster.g","threadId":"57824","inReplyTo":"pull.1227.git.1651324796892.gitgitgadget@gmail.com","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-30T21:17:46Z","receivedAt":"2022-04-30T21:17:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Abhradeep Chakraborty via GitGitGadget\" \nwrites:\n\n> \tif (remote->url_nr > 0) {\n> +\t\tstruct strbuf promisor_config = STRBUF_INIT;\n> +\t\tconst char *partial_clone_filter = NULL;\n> +\n> +\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n> \t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n> +\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n> +\t\t\tstrbuf_addf(&url_buf, \" [%s]\", partial_clone_filter);\n> +\n> +\t\tstrbuf_release(&promisor_config);\n> \t\tstring_list_append(list, remote->name)->util =\n> \t\t\t\tstrbuf_detach(&url_buf, NULL);\n\nThree comments and a half on the code:\n\n - Is it likely that to new readers it would be obvious that what is\n in the [square brackets] is the list-objects-filter used? When we\n want to add new kinds of information other than the URL and the\n list-objects-filter, what is our plan to add them?\n\n - The presentation order is then (fetch\n or push) and then optionally .\n\n (a) shouldn't the output format be described in the\n doucmentation?\n\n (b) does it make sense to append new information like this, or\n is it more logical to keep the at the end?\n\n - Now url_buf no longer contains the url of the remote, but it still\n is called url_buf. It is merely a \"temporary string\" now. Is it\n a good idea to either rename it, stop reusing the same thing for\n different purposes, or do something else?\n\n - By adding this unconditionally, we would break the scripts that\n read the output from this command and expect there won't be extra\n information after the . It may be a good thing (they\n are not prepared to see the list-objects-filter, and the breakage\n may serve as a reminder that they need to update these scripts\n when they see breakage), or it may be an irritating regression.\n\nBut stepping back a bit.\n\nWhy do we want to give this in the \"remote -v\" output in the first\nplace? When a reader really cares, they can ask \"git config\" for\nthis extra piece of information. When you have more than one\nremote, \"git remote -v\" that gives the URL is a good way to remind\nwhich nickname you'd want to give to \"git pull\" or \"git push\". If\nit makes sense to add the extra information,\nthat would mean that there are probably two remote nicknames that\nrefer to the same URL (i.e. \"remote -v\" readers cannot tell them\napart without extra information), but how likely is that, I wonder?\n\n> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\n> index 4a3778d04a8..bf8f3644d3c 100755\n> --- a/t/t5616-partial-clone.sh\n> +++ b/t/t5616-partial-clone.sh\n> @@ -49,6 +49,17 @@ test_expect_success 'do partial clone 1' '\n> \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n> '\n> \n> +test_expect_success 'filters for promisor remotes is listed by git remote -v' '\n> +\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n> +\tgit -C pc2 remote -v >out &&\n> +\tgrep \"[blob:none]\" out &&\n> +\n> +\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n> +\tgit -C pc2 remote -v >out &&\n> +\tgrep \"[object:type=commit]\" out &&\n> +\trm -rf pc2\n> +'\n> +\n> test_expect_success 'verify that .promisor file contains refs fetched' '\n> \tls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&\n> \ttest_line_count = 1 promisorlist &&\n>\n> base-commit: 0f828332d5ac36fc63b7d8202652efa152809856\n"},{"id":"454698","messageId":"20220501155725.93866-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"xmqqczgy6zk5.fsf@gitster.g","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-01T15:57:25Z","receivedAt":"2022-05-01T15:57:44Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Sorry for the late response.\n\nJunio C Hamano wrote:\n\n> Three comments and a half on the code:\n>\n> - Is it likely that to new readers it would be obvious that what is\n> in the [square brackets] is the list-objects-filter used? When we\n> want to add new kinds of information other than the URL and the\n> list-objects-filter, what is our plan to add them? \n\nI do think that new readers can easily understand the meaning of the\ntext inside the [square brackets]. These square brackets (with the\nlist-objects-filter inside it) will be shown only if the remote is\na promisor remote. So, users who don't use promisor remotes, will not\nbe affected. Those who used the filters can only notice the change.\nThey can easily understand it. In fact, I think it would give them an\noption to quickly check which are the promisor remotes and which are not.\nThough this change should be properly documented (which I forgot to\nadd) so that they can be sure about it.\n\n> - The presentation order is then (fetch\n> or push) and then optionally .\n>\n> (a) shouldn't the output format be described in the\n> doucmentation?\n>\n> (b) does it make sense to append new information like this, or\n> is it more logical to keep the at the end?\n\nYeah, it should be documented. I forgot it :|\nWill add it in the next version.\n\nI think it is better to keep at the end.\nBecause I think, people first want to check whether the remote\nis (fetch) or (push). After that, they might want to know about the\nfilter. Another point is that is optional\n(i.e. only for promisor remotes). It would not make sense to put an\noptional info in between two permanent info (in this case,\n and ). It would be difficult for scripts\nwhich parse the output of `git remote -v` on the basis of string\npositions.\n\n> - Now url_buf no longer contains the url of the remote, but it still\n> is called url_buf. It is merely a \"temporary string\" now. Is it\n> a good idea to either rename it, stop reusing the same thing for\n> different purposes, or do something else?\n\nHmm, this can be a subject for discussion. Yes, it is true that the\nname `url_buf` is not suitable for the additional info it contains ( in\nthe proposed change). I did it to use less memory. I think renaming it\nto `remote_info_buf` or similar is a better idea.\n\n> - By adding this unconditionally, we would break the scripts that\n> read the output from this command and expect there won't be extra\n> information after the . It may be a good thing (they\n> are not prepared to see the list-objects-filter, and the breakage\n> may serve as a reminder that they need to update these scripts\n> when they see breakage), or it may be an irritating regression.\n\nI agree. Frankly speaking, I have no counter argument for this. I can\ntell that the proposed change will be beneficial for the users who use\npromisor remotes along with other remotes. So, may be we can accept the\nshort term consequences of it. What we can do is we can provide a proper\ndocumentation so that if anything bad happen to those scripts, devs can\nsee the documentation and update the scripts accordingly.\n\n> But stepping back a bit.\n>\n> Why do we want to give this in the \"remote -v\" output in the first\n> place? When a reader really cares, they can ask \"git config\" for\n> this extra piece of information. When you have more than one\n> remote, \"git remote -v\" that gives the URL is a good way to remind\n> which nickname you'd want to give to \"git pull\" or \"git push\".\n\n`remote -v` helps users to get the overall idea of the remotes. We can\nsee how many remotes are there, which remote name corresponds to which\nurl etc. That is we can get a summary of remotes. Having that said, does\nnot it make sense to add the extra here? Users\ncan easily understand which are promisor remotes ( along with their\nfilter type) and which are not. Of course, they can use git config for\nthat. But it would be a tidious job to check the the type of remotes\n(i.e. which are promisor remotes and which are not) one by one. If the\nuser try to search for the promisor remotes in the config file, he/she\nhave to go through the other configuration settings (irrelevant to him/her\nat that time) to reach the `[remote]` section. Isn't it?\n\n> ... If\n> it makes sense to add the extra information,\n> that would mean that there are probably two remote nicknames that\n> refer to the same URL (i.e. \"remote -v\" readers cannot tell them\n> apart without extra information), but how likely is that, I wonder?\n\nI think, having a proper documentation about the new changes is the\nanswer to it.\n\n\nThanks :)\n"},{"id":"454699","messageId":"xmqqfslt44di.fsf@gitster.g","threadId":"57824","inReplyTo":"20220501155725.93866-1-chakrabortyabhradeep79@gmail.com","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-01T16:14:17Z","receivedAt":"2022-05-01T16:14:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhradeep Chakraborty writes:\n\n> Sorry for the late response.\n>\n> Junio C Hamano wrote:\n>\n>> Three comments and a half on the code:\n>>\n>> - Is it likely that to new readers it would be obvious that what is\n>> in the [square brackets] is the list-objects-filter used? When we\n>> want to add new kinds of information other than the URL and the\n>> list-objects-filter, what is our plan to add them? \n>\n> I do think that new readers can easily understand the meaning of the\n> text inside the [square brackets]. These square brackets (with the\n> list-objects-filter inside it) will be shown only if the remote is\n> a promisor remote. So, users who don't use promisor remotes, will not\n> be affected. Those who used the filters can only notice the change.\n> They can easily understand it. In fact, I think it would give them an\n> option to quickly check which are the promisor remotes and which are not.\n> Though this change should be properly documented (which I forgot to\n> add) so that they can be sure about it.\n\nYou forgot to answer more important half of the question. It would\nbe easy for you to know what the string inside brackets means\nbecause you are so obsessed with the promisor remote to write this\npatch ;-) But when we need to add even more pieces of information in\nthe future, will it stay so? Can \"[some-random-string]\" easily be\nidentified as a list-objects-filter by those who do not care\nparticularly about promisor remotes (e.g. those who wanted to see\nthe URL to tell multiple remote nicknames apart) when the line has\neven more piece of information in the future?\n\nAt some point, we'd need to either (1) stop adding too many details\nto avoid cluttering the output line, or (2) start labeling each\npiece of information to make it easy for the readers to identify\nwhich one is which [*]. We need to ask ourselves why now is not\nthat \"some point\" already.\n\n Side note: and the strategy to add new pieces of information\n need to take the same approach between the two, and that is why\n we need \"what is the plan to add new pieces of information?\"\n answered.\n\n> (i.e. which are promisor remotes and which are not) one by one. If the\n> user try to search for the promisor remotes in the config file, he/she\n> have to go through the other configuration settings (irrelevant to him/her\n> at that time) to reach the `[remote]` section. Isn't it?\n\nSorry, but the question does not make much sense to me. Why is a\npiece of information you get from \"git config\" irrelevant if you get\nit in order to figure out what you want to know, i.e. what promisor\nremote do we rely on?\n\n>> ... If\n>> it makes sense to add the extra information,\n>> that would mean that there are probably two remote nicknames that\n>> refer to the same URL (i.e. \"remote -v\" readers cannot tell them\n>> apart without extra information), but how likely is that, I wonder?\n>\n> I think, having a proper documentation about the new changes is the\n> answer to it.\n\nThe question is \"what can readers achieve by having this extra\ninformation in 'remote -v' output\". Do you have to duck the\nquestion because you cannot answer in a simple sentence, and instead\nreaders must read reams of documentation pages? I doubt it would be\nthat obscure.\n\nI wanted to like the patch, the changed text is simple enough, but\nquite honestly, the lack of clarity in the answers to the most basic\n\"why do we want this? what is this good for? how does this help the\nusers?\" questions, I am not yet succeeding to do so.\n\nThanks.\n"},{"id":"454702","messageId":"20220501193807.94369-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"xmqqfslt44di.fsf@gitster.g","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-01T19:38:07Z","receivedAt":"2022-05-01T19:38:31Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"\nJunio C Hamano wrote:\n\n> You forgot to answer more important half of the question. It would\n> be easy for you to know what the string inside brackets means\n> because you are so obsessed with the promisor remote to write this\n> patch ;-) But when we need to add even more pieces of information in\n> the future, will it stay so? Can \"[some-random-string]\" easily be\n> identified as a list-objects-filter by those who do not care\n> particularly about promisor remotes (e.g. those who wanted to see\n> the URL to tell multiple remote nicknames apart) when the line has\n> even more piece of information in the future?\n>\n> At some point, we'd need to either (1) stop adding too many details\n> to avoid cluttering the output line, or (2) start labeling each\n> piece of information to make it easy for the readers to identify\n> which one is which [*]. We need to ask ourselves why now is not\n> that \"some point\" already.\n>\n> Side note: and the strategy to add new pieces of information\n> need to take the same approach between the two, and that is why\n> we need \"what is the plan to add new pieces of information?\"\n> answered.\n\nI am sorry if I failed to explain you what I really wanted to mean.\nYes, I forgot to answer the last question which is \"When we\nwant to add new kinds of information other than the URL and the\nlist-objects-filter, what is our plan to add them?\".\n\nSo let me answer this now. As `-v` flag gives a kind of overall summary\nof the remotes, users expect that the most important and most basic\ninformation should be listed in the output of `remote -v`. So, there\nmay be some other more important informations in the future that we\nhave to add to `remote -v` output. In that case, method (1) would not\nbe a great idea I think (unless a new flag has been created). method\n(2) is better.\n\n> > (i.e. which are promisor remotes and which are not) one by one. If the\n> > user try to search for the promisor remotes in the config file, he/she\n> > have to go through the other configuration settings (irrelevant to him/her\n> > at that time) to reach the `[remote]` section. Isn't it?\n>\n> Sorry, but the question does not make much sense to me. Why is a\n> piece of information you get from \"git config\" irrelevant if you get\n> it in order to figure out what you want to know, i.e. what promisor\n> remote do we rely on?\n\nLet me explain what I really meant here - I am guessing that you have no\nproblem with the upper part of that para.\n\nIf we forget about my proposed change, there are two possible ways to find\nout the info about promisor remotes - \n\t(1) Use `git config --get remote..partialCloneFilter`\n\n\t This command gives an output only if is a promisor\n\t remote. So in case the user forget which one is a promisor\n\t remote, he/she has to try this command with each and every\n\t to find out which is/are the promisor remote(s).\n\n\t(2) Open the git config file (either manually or by running `git\n\t config --edit`\n\n\t In this case, the user has to go through all the settings until\n\t the [remote \"\"] section is found. E.g. let's say\n\t below is the config file - \n\n\t [core]\n \trepositoryformatversion = 0\n \tfilemode = true\n \tbare = false\n \tlogallrefupdates = true\n \tignorecase = true\n \tprecomposeunicode = true\n\t [remote \"upstream\"]\n \turl = https://github.com/git/git.git\n \tfetch = +refs/heads/*:refs/remotes/upstream/*\n\t [branch \"master\"]\n \tremote = upstream\n \tmerge = refs/heads/master\n\t [remote \"origin\"]\n \turl = https://github.com/Abhra303/git.git\n \tfetch = +refs/heads/*:refs/remotes/origin/*\n\t\tpartialCloneFilter = blob:none\n\n\t To find out whether \"origin\" is promisor or not, he has to go\n\t to the [remote \"origin\"] section. Here all the configuations\n\t under `[core]`, `[remote \"upstream\"]` and `[branch \"master\"]\n\t are irrelevant to him/her at that time (because he/she is not\n\t interested to know about those configuration settings at that\n\t time).\n\nThe proposed change is simpler compared to the above as it lists down all\nthe remotes along with their list-objects-filter. Another point is that\nit's important for an user to know which one is a promisor remote and what\nfilter type they use. If we go with the current implementation the output\nwould be let's say - \norigin (fetch)\norigin (push)\nupstream (fetch)\nupstream (push)\n\nBy seeing the above output anyone may assume that all the remotes are\nnormal remotes. If the user now try to run `git pull origin` and suddenly\nhe/she discover that some blobs are not downloaded. He/she run the above\nmentioned (1) command and find that this is a promisor remote!\n\nHere `remote -v` didn't warn the user about the origin remote being an\npromisor remote. Instead it makes him/her assume that all are normal\nremotes. Providing only these three info (i.e. , \nand ) is not sufficient - it only shows the half of the picture.\n\n\n> The question is \"what can readers achieve by having this extra\n> information in 'remote -v' output\". Do you have to duck the\n> question because you cannot answer in a simple sentence, and instead\n> readers must read reams of documentation pages? I doubt it would be\n> that obscure.\n\nSorry, I misunderstood that section of your first comment. I think\nI hopefully answered this question in the above portion of this comment.\nProviding only the three information about remotes is not sufficient\nas it do not distinguish between promisor remotes and normal remotes.\nIn that sense, it will add simplicity and the user would be much more\nclear about the remotes(i.e. which is promisor remote and which is not).\n\n> I wanted to like the patch, the changed text is simple enough, but\n> quite honestly, the lack of clarity in the answers to the most basic\n> \"why do we want this? what is this good for? how does this help the\n> users?\" questions, I am not yet succeeding to do so.\n\nMy bad! Hope I am now able to answer all the questions you asked. Let\nme know if you still struggle to get my point.\n\nThanks :)\n"},{"id":"454711","messageId":"ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email","threadId":"57824","inReplyTo":"20220501193807.94369-1-chakrabortyabhradeep79@gmail.com","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-02T10:33:08Z","receivedAt":"2022-05-02T10:33:14Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 01/05/2022 20:38, Abhradeep Chakraborty wrote:\n> Junio C Hamano wrote:\n>\n>> You forgot to answer more important half of the question. It would\n>> be easy for you to know what the string inside brackets means\n>> because you are so obsessed with the promisor remote to write this\n>> patch ;-) But when we need to add even more pieces of information in\n>> the future, will it stay so? Can \"[some-random-string]\" easily be\n>> identified as a list-objects-filter by those who do not care\n>> particularly about promisor remotes (e.g. those who wanted to see\n>> the URL to tell multiple remote nicknames apart) when the line has\n>> even more piece of information in the future?\n>>\n>> At some point, we'd need to either (1) stop adding too many details\n>> to avoid cluttering the output line, or (2) start labeling each\n>> piece of information to make it easy for the readers to identify\n>> which one is which [*]. We need to ask ourselves why now is not\n>> that \"some point\" already.\n>>\n>> Side note: and the strategy to add new pieces of information\n>> need to take the same approach between the two, and that is why\n>> we need \"what is the plan to add new pieces of information?\"\n>> answered.\n> I am sorry if I failed to explain you what I really wanted to mean.\n> Yes, I forgot to answer the last question which is \"When we\n> want to add new kinds of information other than the URL and the\n> list-objects-filter, what is our plan to add them?\".\n>\n> So let me answer this now. As `-v` flag gives a kind of overall summary\n> of the remotes, users expect that the most important and most basic\n> information should be listed in the output of `remote -v`. So, there\n> may be some other more important informations in the future that we\n> have to add to `remote -v` output. In that case, method (1) would not\n> be a great idea I think (unless a new flag has been created). method\n> (2) is better.\n\nWhen I use the `git remote` command, I use the -vv variant to what what\nI need, i.e. its more than `-v`, so maybe adding an extra\n`--show-partial-filter` option may be necessary (with a more compact\nname ;-).\n\nThere will also be a similar desire (IIUC) to match the sparse/cone mode\nrepos to their remotes, i.e. to remind a user that what is held at the\nremote isn't the same as held locally.\n>\n>>> (i.e. which are promisor remotes and which are not) one by one. If the\n>>> user try to search for the promisor remotes in the config file, he/she\n>>> have to go through the other configuration settings (irrelevant to him/her\n>>> at that time) to reach the `[remote]` section. Isn't it?\n>> Sorry, but the question does not make much sense to me. Why is a\n>> piece of information you get from \"git config\" irrelevant if you get\n>> it in order to figure out what you want to know, i.e. what promisor\n>> remote do we rely on?\n> Let me explain what I really meant here - I am guessing that you have no\n> problem with the upper part of that para.\n>\n> If we forget about my proposed change, there are two possible ways to find\n> out the info about promisor remotes - \n> \t(1) Use `git config --get remote..partialCloneFilter`\n>\n> \t This command gives an output only if is a promisor\n> \t remote. So in case the user forget which one is a promisor\n> \t remote, he/she has to try this command with each and every\n> \t to find out which is/are the promisor remote(s).\n\nI hear your pain here. I had the same issue with the branch description.\n(https://stackoverflow.com/questions/15058844/print-branch-description).\nIt's the same 'extract from config' problem.\n \n```You can display the branch description using a git config command.\n\nTo show all branch descriptions, I have the alias\n\nbrshow = config --get-regexp 'branch.*.description'\n\n, and for the current HEAD I have\n\nbrshow1 = !git config --get \"branch.$(git rev-parse --abbrev-ref\nHEAD).description\". ```\n\nso it is possible to generalise the config query, if hard to discover.\n\n>\n> \t(2) Open the git config file (either manually or by running `git\n> \t config --edit`\n>\n> \t In this case, the user has to go through all the settings until\n> \t the [remote \"\"] section is found. E.g. let's say\n> \t below is the config file - \n>\n> \t [core]\n> \trepositoryformatversion = 0\n> \tfilemode = true\n> \tbare = false\n> \tlogallrefupdates = true\n> \tignorecase = true\n> \tprecomposeunicode = true\n> \t [remote \"upstream\"]\n> \turl = https://github.com/git/git.git\n> \tfetch = +refs/heads/*:refs/remotes/upstream/*\n> \t [branch \"master\"]\n> \tremote = upstream\n> \tmerge = refs/heads/master\n> \t [remote \"origin\"]\n> \turl = https://github.com/Abhra303/git.git\n> \tfetch = +refs/heads/*:refs/remotes/origin/*\n> \t\tpartialCloneFilter = blob:none\n>\n> \t To find out whether \"origin\" is promisor or not, he has to go\n> \t to the [remote \"origin\"] section. Here all the configuations\n> \t under `[core]`, `[remote \"upstream\"]` and `[branch \"master\"]\n> \t are irrelevant to him/her at that time (because he/she is not\n> \t interested to know about those configuration settings at that\n> \t time).\n>\n> The proposed change is simpler compared to the above as it lists down all\n> the remotes along with their list-objects-filter. Another point is that\n> it's important for an user to know which one is a promisor remote and what\n> filter type they use. If we go with the current implementation the output\n> would be let's say - \n> origin (fetch)\n> origin (push)\n> upstream (fetch)\n> upstream (push)\n>\n> By seeing the above output anyone may assume that all the remotes are\n> normal remotes. If the user now try to run `git pull origin` and suddenly\n> he/she discover that some blobs are not downloaded. He/she run the above\n> mentioned (1) command and find that this is a promisor remote!\n>\n> Here `remote -v` didn't warn the user about the origin remote being an\n> promisor remote. Instead it makes him/her assume that all are normal\n> remotes. Providing only these three info (i.e. , \n> and ) is not sufficient - it only shows the half of the picture.\n>\n>\n>> The question is \"what can readers achieve by having this extra\n>> information in 'remote -v' output\". Do you have to duck the\n>> question because you cannot answer in a simple sentence, and instead\n>> readers must read reams of documentation pages? I doubt it would be\n>> that obscure.\n> Sorry, I misunderstood that section of your first comment. I think\n> I hopefully answered this question in the above portion of this comment.\n> Providing only the three information about remotes is not sufficient\n> as it do not distinguish between promisor remotes and normal remotes.\n> In that sense, it will add simplicity and the user would be much more\n> clear about the remotes(i.e. which is promisor remote and which is not).\n>\n>> I wanted to like the patch, the changed text is simple enough, but\n>> quite honestly, the lack of clarity in the answers to the most basic\n>> \"why do we want this? what is this good for? how does this help the\n>> users?\" questions, I am not yet succeeding to do so.\n> My bad! Hope I am now able to answer all the questions you asked. Let\n> me know if you still struggle to get my point.\n>\n> Thanks :)\n\n"},{"id":"454721","messageId":"20220502145624.12702-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email","subject":"Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-02T14:56:24Z","receivedAt":"2022-05-02T14:57:16Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"\nPhilip Oakley wrote:\n\n> When I use the `git remote` command, I use the -vv variant to what what\n> I need, i.e. its more than `-v`, so maybe adding an extra\n> `--show-partial-filter` option may be necessary (with a more compact\n> name ;-).\n\nIf adding new informations to `-v` is not possible (to avoid messy output),\natleast including it to `-vv` makes sense to me (though I am not sure if \n`git remote -vv` is currently implemented). \n\n> There will also be a similar desire (IIUC) to match the sparse/cone mode\n> repos to their remotes, i.e. to remind a user that what is held at the\n> remote isn't the same as held locally.\n\nYeah, maybe.\n\n> I hear your pain here. I had the same issue with the branch description.\n> (https://stackoverflow.com/questions/15058844/print-branch-description).\n> It's the same 'extract from config' problem.\n>\n> ```You can display the branch description using a git config command.\n>\n> To show all branch descriptions, I have the alias\n>\n> brshow = config --get-regexp 'branch.*.description'\n>\n> , and for the current HEAD I have\n>\n> brshow1 = !git config --get \"branch.$(git rev-parse --abbrev-ref\n> HEAD).description\". ```\n>\n> so it is possible to generalise the config query, if hard to discover.\n> \n\nThanks for the info. I tried your suggestion and it worked. But still,\nit is better to include in the output. This is\nbecause of the second point I mentioned in my previous comment. Users\ncan be much more clear about the types of available remotes overall.\nIMO specifying filters for remotes is far more important than the\nbranch description. The behaviour of `git fetch` depends on it. If\nwe can specify `(fetch)` in the output then why not the filter of that\n`fetch` on which the behaviour of `fetch` functionality highly depends?\n\nThanks :)\n"},{"id":"454793","messageId":"pull.1227.v2.git.1651591253333.gitgitgadget@gmail.com","threadId":"57824","inReplyTo":"pull.1227.git.1651324796892.gitgitgadget@gmail.com","subject":"[PATCH v2] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-03T15:20:53Z","receivedAt":"2022-05-03T15:21:26Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"From: Abhradeep Chakraborty \n\n`git remote -v` (`--verbose`) lists down the names of remotes along with\ntheir urls. It would be beneficial for users to also specify the filter\ntypes for promisor remotes. Something like this -\n\n\torigin\tremote-url (fetch) [blob:none]\n\torigin\tremote-url (push)\n\nTeach `git remote -v` to also specify the filters for promisor remotes.\n\nSigned-off-by: Abhradeep Chakraborty \n---\n builtin/remote.c: teach -v to list filters for promisor remotes\n \n Fixes #1211 [1]\n \n In this version, documentation is updated (describing the proposed\n change) and url_buf is renamed into remote_info_buf.\n \n [1] https://github.com/gitgitgadget/git/issues/1211\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1227%2FAbhra303%2Fpromisor_remote-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1227/Abhra303/promisor_remote-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1227\n\nRange-diff vs v1:\n\n 1: fe3bf755e63 ! 1: e7ced852fd5 builtin/remote.c: teach `-v` to list filters for promisor remotes\n @@ Commit message\n \n Signed-off-by: Abhradeep Chakraborty \n \n + ## Documentation/git-remote.txt ##\n +@@ Documentation/git-remote.txt: OPTIONS\n + -v::\n + --verbose::\n + \tBe a little more verbose and show remote url after name.\n ++ For promisor remotes it will show an extra information\n ++ (wrapped in square brackets) describing which filter\n ++ (`blob:none` etc.) that promisor remote use.\n + \tNOTE: This must be placed between `remote` and subcommand.\n + \n + \n +\n ## builtin/remote.c ##\n -@@ builtin/remote.c: static int get_one_entry(struct remote *remote, void *priv)\n +@@ builtin/remote.c: static int show_push_info_item(struct string_list_item *item, void *cb_data)\n + static int get_one_entry(struct remote *remote, void *priv)\n + {\n + \tstruct string_list *list = priv;\n +-\tstruct strbuf url_buf = STRBUF_INIT;\n ++\tstruct strbuf remote_info_buf = STRBUF_INIT;\n + \tconst char **url;\n \tint i, url_nr;\n \n \tif (remote->url_nr > 0) {\n +-\t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n +\t\tstruct strbuf promisor_config = STRBUF_INIT;\n +\t\tconst char *partial_clone_filter = NULL;\n +\n +\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n - \t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n ++\t\tstrbuf_addf(&remote_info_buf, \"%s (fetch)\", remote->url[0]);\n +\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n -+\t\t\tstrbuf_addf(&url_buf, \" [%s]\", partial_clone_filter);\n ++\t\t\tstrbuf_addf(&remote_info_buf, \" [%s]\", partial_clone_filter);\n +\n +\t\tstrbuf_release(&promisor_config);\n \t\tstring_list_append(list, remote->name)->util =\n - \t\t\t\tstrbuf_detach(&url_buf, NULL);\n +-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n ++\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t} else\n + \t\tstring_list_append(list, remote->name)->util = NULL;\n + \tif (remote->pushurl_nr) {\n +@@ builtin/remote.c: static int get_one_entry(struct remote *remote, void *priv)\n + \t}\n + \tfor (i = 0; i < url_nr; i++)\n + \t{\n +-\t\tstrbuf_addf(&url_buf, \"%s (push)\", url[i]);\n ++\t\tstrbuf_addf(&remote_info_buf, \"%s (push)\", url[i]);\n + \t\tstring_list_append(list, remote->name)->util =\n +-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n ++\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n + \t}\n + \n + \treturn 0;\n \n ## t/t5616-partial-clone.sh ##\n @@ t/t5616-partial-clone.sh: test_expect_success 'do partial clone 1' '\n\n\n Documentation/git-remote.txt | 3 +++\n builtin/remote.c | 18 +++++++++++++-----\n t/t5616-partial-clone.sh | 11 +++++++++++\n 3 files changed, 27 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex cde9614e362..71a0e85990d 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -35,6 +35,9 @@ OPTIONS\n -v::\n --verbose::\n \tBe a little more verbose and show remote url after name.\n+ For promisor remotes it will show an extra information\n+ (wrapped in square brackets) describing which filter\n+ (`blob:none` etc.) that promisor remote use.\n \tNOTE: This must be placed between `remote` and subcommand.\n \n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5f4cde9d784..d4b69fe7789 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1185,14 +1185,22 @@ static int show_push_info_item(struct string_list_item *item, void *cb_data)\n static int get_one_entry(struct remote *remote, void *priv)\n {\n \tstruct string_list *list = priv;\n-\tstruct strbuf url_buf = STRBUF_INIT;\n+\tstruct strbuf remote_info_buf = STRBUF_INIT;\n \tconst char **url;\n \tint i, url_nr;\n \n \tif (remote->url_nr > 0) {\n-\t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tstruct strbuf promisor_config = STRBUF_INIT;\n+\t\tconst char *partial_clone_filter = NULL;\n+\n+\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n+\t\t\tstrbuf_addf(&remote_info_buf, \" [%s]\", partial_clone_filter);\n+\n+\t\tstrbuf_release(&promisor_config);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t} else\n \t\tstring_list_append(list, remote->name)->util = NULL;\n \tif (remote->pushurl_nr) {\n@@ -1204,9 +1212,9 @@ static int get_one_entry(struct remote *remote, void *priv)\n \t}\n \tfor (i = 0; i < url_nr; i++)\n \t{\n-\t\tstrbuf_addf(&url_buf, \"%s (push)\", url[i]);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (push)\", url[i]);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t}\n \n \treturn 0;\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 4a3778d04a8..bf8f3644d3c 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -49,6 +49,17 @@ test_expect_success 'do partial clone 1' '\n \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n '\n \n+test_expect_success 'filters for promisor remotes is listed by git remote -v' '\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"[blob:none]\" out &&\n+\n+\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"[object:type=commit]\" out &&\n+\trm -rf pc2\n+'\n+\n test_expect_success 'verify that .promisor file contains refs fetched' '\n \tls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&\n \ttest_line_count = 1 promisorlist &&\n\nbase-commit: 0f828332d5ac36fc63b7d8202652efa152809856\n-- \ngitgitgadget\n"},{"id":"454832","messageId":"xmqqr159mdfh.fsf@gitster.g","threadId":"57824","inReplyTo":"pull.1227.v2.git.1651591253333.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-04T17:10:26Z","receivedAt":"2022-05-04T17:48:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Abhradeep Chakraborty via GitGitGadget\" \nwrites:\n\n> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\n> index cde9614e362..71a0e85990d 100644\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n> @@ -35,6 +35,9 @@ OPTIONS\n> -v::\n> --verbose::\n> \tBe a little more verbose and show remote url after name.\n> + For promisor remotes it will show an extra information\n> + (wrapped in square brackets) describing which filter\n> + (`blob:none` etc.) that promisor remote use.\n> \tNOTE: This must be placed between `remote` and subcommand.\n\nBroken indentation. You can save embarrassment by double checking\nwhat you committed by sending e-mail to yourself (or checking output\nfrom \"git show\") before sending it to the list.\n\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index 5f4cde9d784..d4b69fe7789 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -1185,14 +1185,22 @@ static int show_push_info_item(struct string_list_item *item, void *cb_data)\n> static int get_one_entry(struct remote *remote, void *priv)\n> {\n> \tstruct string_list *list = priv;\n> -\tstruct strbuf url_buf = STRBUF_INIT;\n> +\tstruct strbuf remote_info_buf = STRBUF_INIT;\n> \tconst char **url;\n> \tint i, url_nr;\n> \n> \tif (remote->url_nr > 0) {\n> -\t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n> +\t\tstruct strbuf promisor_config = STRBUF_INIT;\n> +\t\tconst char *partial_clone_filter = NULL;\n> +\n> +\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n> +\t\tstrbuf_addf(&remote_info_buf, \"%s (fetch)\", remote->url[0]);\n> +\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n> +\t\t\tstrbuf_addf(&remote_info_buf, \" [%s]\", partial_clone_filter);\n> +\n> +\t\tstrbuf_release(&promisor_config);\n> \t\tstring_list_append(list, remote->name)->util =\n> -\t\t\t\tstrbuf_detach(&url_buf, NULL);\n> +\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n\nIt is unfortunate that the \"we borrow without copying\" variant of\ngit_config_get_string() is called git_config_get_string_tmp(), which\nis an utterly misleading name that might confuse readers into\nmistaking it may make a temporary copy for the caller to release.\nPerhaps we would want to rename it to git_config_peek_string() or\nsomething, but that is totally outside the topic, of course.\n\nIn any case, what I wanted to say is that I just made sure that the\nvalue in the partial_clone_filter variable is not leaked.\n\nLooking good.\n\n> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\n> index 4a3778d04a8..bf8f3644d3c 100755\n> --- a/t/t5616-partial-clone.sh\n> +++ b/t/t5616-partial-clone.sh\n> @@ -49,6 +49,17 @@ test_expect_success 'do partial clone 1' '\n> \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n> '\n> \n> +test_expect_success 'filters for promisor remotes is listed by git remote -v' '\n> +\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n> +\tgit -C pc2 remote -v >out &&\n> +\tgrep \"[blob:none]\" out &&\n> +\n> +\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n> +\tgit -C pc2 remote -v >out &&\n> +\tgrep \"[object:type=commit]\" out &&\n> +\trm -rf pc2\n> +'\n\nI doubt that these \"grep\" do what you think it is doing. It would\nsay \"I am happy\" on any line that has one of these characters listed\ninside the [].\n\nDo not clean up with an extra \"&& clean up\" step at the end of\n&&-cascade. Instead use test_when_finished to make sure that after\nany failure in the cascade the clean-up step would still trigger.\n\n\ttest_expect_success 'title' '\n\t\ttest_when_finished \"rm -fr pc2\" &&\n\t\tgit clone ... &&\n\t\t...\n\t\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n\t'\n\nor something.\n\nHaving tests that show how this new feature works is of course\nnecessary, but we must have negative tests that ensure that it does\n*not* trigger when it should not. E.g. the new [filter-spec] should\nnot be given for a remote if the user didn't ask for \"-v\", or the\nremote is not a promisor.\n\nThanks.\n\n"},{"id":"454859","messageId":"20220505141202.65106-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"xmqqr159mdfh.fsf@gitster.g","subject":"Re: [PATCH v2] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-05T14:12:02Z","receivedAt":"2022-05-05T14:13:01Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Junio C Hamano wrote:\n\n> Broken indentation. You can save embarrassment by double checking\n> what you committed by sending e-mail to yourself (or checking output\n> from \"git show\") before sending it to the list.\n\nThanks for the suggestions. Will keep it in mind next time.\n\n> I doubt that these \"grep\" do what you think it is doing. It would\n> say \"I am happy\" on any line that has one of these characters listed\n> inside the [].\n>\n> Do not clean up with an extra \"&& clean up\" step at the end of\n> &&-cascade. Instead use test_when_finished to make sure that after\n> any failure in the cascade the clean-up step would still trigger.\n>\n>\ttest_expect_success 'title' '\n>\t\ttest_when_finished \"rm -fr pc2\" &&\n>\t\tgit clone ... &&\n>\t\t...\n> \t\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n> \t'\n>\n> or something.\n>\n> Having tests that show how this new feature works is of course\n> necessary, but we must have negative tests that ensure that it does\n> *not* trigger when it should not. E.g. the new [filter-spec] should\n> not be given for a remote if the user didn't ask for \"-v\", or the\n> remote is not a promisor.\n\nGot it. Will send the necessary changes by the day after tommorow.\n\nThanks :)\n"},{"id":"454970","messageId":"pull.1227.v3.git.1651933221216.gitgitgadget@gmail.com","threadId":"57824","inReplyTo":"pull.1227.v2.git.1651591253333.gitgitgadget@gmail.com","subject":"[PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-07T14:20:21Z","receivedAt":"2022-05-07T14:20:28Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"From: Abhradeep Chakraborty \n\n`git remote -v` (`--verbose`) lists down the names of remotes along with\ntheir urls. It would be beneficial for users to also specify the filter\ntypes for promisor remotes. Something like this -\n\n\torigin\tremote-url (fetch) [blob:none]\n\torigin\tremote-url (push)\n\nTeach `git remote -v` to also specify the filters for promisor remotes.\n\nSigned-off-by: Abhradeep Chakraborty \n---\n builtin/remote.c: teach -v to list filters for promisor remotes\n \n Fixes #1211 [1]\n \n In the previous version, documentation is updated (describing the\n proposed change) and url_buf is renamed into remote_info_buf. In this\n varsion, some more test cases are added and broken indentations are\n fixed.\n \n [1] https://github.com/gitgitgadget/git/issues/1211\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1227%2FAbhra303%2Fpromisor_remote-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1227/Abhra303/promisor_remote-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1227\n\nRange-diff vs v2:\n\n 1: e7ced852fd5 ! 1: 9ac6ca9a08e builtin/remote.c: teach `-v` to list filters for promisor remotes\n @@ Documentation/git-remote.txt: OPTIONS\n -v::\n --verbose::\n \tBe a little more verbose and show remote url after name.\n -+ For promisor remotes it will show an extra information\n -+ (wrapped in square brackets) describing which filter\n -+ (`blob:none` etc.) that promisor remote use.\n ++\tFor promisor remotes it will show an extra information\n ++\t(wrapped in square brackets) describing which filter\n ++\t(`blob:none` etc.) that promisor remote use.\n \tNOTE: This must be placed between `remote` and subcommand.\n \n \n @@ t/t5616-partial-clone.sh: test_expect_success 'do partial clone 1' '\n \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n '\n \n -+test_expect_success 'filters for promisor remotes is listed by git remote -v' '\n ++test_expect_success 'filters for promisor remotes are listed by git remote -v' '\n ++\ttest_when_finished \"rm -rf pc2\" &&\n +\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n +\tgit -C pc2 remote -v >out &&\n -+\tgrep \"[blob:none]\" out &&\n ++\tgrep \"srv.bare (fetch) \\[blob:none\\]\" out &&\n +\n +\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n +\tgit -C pc2 remote -v >out &&\n -+\tgrep \"[object:type=commit]\" out &&\n -+\trm -rf pc2\n ++\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n ++'\n ++\n ++test_expect_success 'filters should not be listed for non promisor remotes (remote -v)' '\n ++\ttest_when_finished \"rm -rf pc2\" &&\n ++\tgit clone \"file://$(pwd)/srv.bare\" pc2 &&\n ++\tgit -C pc2 remote -v >out &&\n ++\t! grep \"(fetch) \\[.*\\]\" out\n ++'\n ++\n ++test_expect_success 'filters are listed by git remote -v only' '\n ++\ttest_when_finished \"rm -rf pc2\" &&\n ++\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n ++\tgit -C pc2 remote >out &&\n ++\t! grep \"\\[blob:none\\]\" out &&\n ++\n ++\tgit -C pc2 remote show >out &&\n ++\t! grep \"\\[blob:none\\]\" out\n +'\n +\n test_expect_success 'verify that .promisor file contains refs fetched' '\n\n\n Documentation/git-remote.txt | 3 +++\n builtin/remote.c | 18 +++++++++++++-----\n t/t5616-partial-clone.sh | 28 ++++++++++++++++++++++++++++\n 3 files changed, 44 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex cde9614e362..a125bd839f7 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -35,6 +35,9 @@ OPTIONS\n -v::\n --verbose::\n \tBe a little more verbose and show remote url after name.\n+\tFor promisor remotes it will show an extra information\n+\t(wrapped in square brackets) describing which filter\n+\t(`blob:none` etc.) that promisor remote use.\n \tNOTE: This must be placed between `remote` and subcommand.\n \n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5f4cde9d784..d4b69fe7789 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1185,14 +1185,22 @@ static int show_push_info_item(struct string_list_item *item, void *cb_data)\n static int get_one_entry(struct remote *remote, void *priv)\n {\n \tstruct string_list *list = priv;\n-\tstruct strbuf url_buf = STRBUF_INIT;\n+\tstruct strbuf remote_info_buf = STRBUF_INIT;\n \tconst char **url;\n \tint i, url_nr;\n \n \tif (remote->url_nr > 0) {\n-\t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tstruct strbuf promisor_config = STRBUF_INIT;\n+\t\tconst char *partial_clone_filter = NULL;\n+\n+\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n+\t\t\tstrbuf_addf(&remote_info_buf, \" [%s]\", partial_clone_filter);\n+\n+\t\tstrbuf_release(&promisor_config);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t} else\n \t\tstring_list_append(list, remote->name)->util = NULL;\n \tif (remote->pushurl_nr) {\n@@ -1204,9 +1212,9 @@ static int get_one_entry(struct remote *remote, void *priv)\n \t}\n \tfor (i = 0; i < url_nr; i++)\n \t{\n-\t\tstrbuf_addf(&url_buf, \"%s (push)\", url[i]);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (push)\", url[i]);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t}\n \n \treturn 0;\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 4a3778d04a8..26756d616cd 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -49,6 +49,34 @@ test_expect_success 'do partial clone 1' '\n \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n '\n \n+test_expect_success 'filters for promisor remotes are listed by git remote -v' '\n+\ttest_when_finished \"rm -rf pc2\" &&\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"srv.bare (fetch) \\[blob:none\\]\" out &&\n+\n+\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n+\tgit -C pc2 remote -v >out &&\n+\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n+'\n+\n+test_expect_success 'filters should not be listed for non promisor remotes (remote -v)' '\n+\ttest_when_finished \"rm -rf pc2\" &&\n+\tgit clone \"file://$(pwd)/srv.bare\" pc2 &&\n+\tgit -C pc2 remote -v >out &&\n+\t! grep \"(fetch) \\[.*\\]\" out\n+'\n+\n+test_expect_success 'filters are listed by git remote -v only' '\n+\ttest_when_finished \"rm -rf pc2\" &&\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n+\tgit -C pc2 remote >out &&\n+\t! grep \"\\[blob:none\\]\" out &&\n+\n+\tgit -C pc2 remote show >out &&\n+\t! grep \"\\[blob:none\\]\" out\n+'\n+\n test_expect_success 'verify that .promisor file contains refs fetched' '\n \tls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&\n \ttest_line_count = 1 promisorlist &&\n\nbase-commit: 0f828332d5ac36fc63b7d8202652efa152809856\n-- \ngitgitgadget\n"},{"id":"454989","messageId":"aa9884d5-b69a-bfd2-4235-a30326bd65f6@gmail.com","threadId":"57824","inReplyTo":"pull.1227.v3.git.1651933221216.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-05-08T15:33:51Z","receivedAt":"2022-05-08T15:33:56Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Abhradeep,\n\nLe 2022-05-07 à 10:20, Abhradeep Chakraborty via GitGitGadget a écrit :\n> From: Abhradeep Chakraborty \n> \n> `git remote -v` (`--verbose`) lists down the names of remotes along with\n> their urls. \n\nsmall nit: I would capitalize URLs.\n\n> It would be beneficial for users to also specify the filter\n> types for promisor remotes. Something like this -\n> \n> \torigin\tremote-url (fetch) [blob:none]\n> \torigin\tremote-url (push)\n> \n> Teach `git remote -v` to also specify the filters for promisor remotes.\n> \n> Signed-off-by: Abhradeep Chakraborty \n> ---\n> builtin/remote.c: teach -v to list filters for promisor remotes\n> \n> Fixes #1211 [1]\n\nI don't think this matters much, but if Junio is OK with that, it would\nbe nice to include the reference to the GitGitGadget issue in the commit\nmessage itself, though with its full URL, something like:\n\nCloses: https://github.com/gitgitgadget/git/issues/1211\n\nas another trailer before your signed-off-by. By including it in the \ncommit message we allow the issue to be closed automatically when your topic\nbranch is merged to 'master'. By using the full link we make sure that GitHub \nknows we are targetting that issue specifically, not any other issue or PR in \nany fork of Git with the same number.\n\n> \n> In the previous version, documentation is updated (describing the\n> proposed change) and url_buf is renamed into remote_info_buf. In this\n> varsion, some more test cases are added and broken indentations are\n> fixed.\n\nAgain, small nit to make it easier for reviewers: usually we prefer to see\nwhat has changed since the previous version first, and then (if you want, \nit's not strictly necessary) what changed in the other previous versions. \nIt's not necessary since if we want that info we can refer to the cover letters\nof the previous iterations directly. And ideally, in bullet points. So something like:\n\nChanges since v2:\n- added more test cases\n- fixed broken indentations\n\nChanges since v1:\n- updated documentation\n- renamed url_buf into remote_info_buf\n\n> \n> [1] https://github.com/gitgitgadget/git/issues/1211\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1227%2FAbhra303%2Fpromisor_remote-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1227/Abhra303/promisor_remote-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1227\n\nThanks,\n\nPhilippe.\n"},{"id":"454990","messageId":"f15e2673-ddc3-27ff-d31c-7fa32af27ae7@gmail.com","threadId":"57824","inReplyTo":"pull.1227.v3.git.1651933221216.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-05-08T15:44:23Z","receivedAt":"2022-05-08T15:44:30Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Forgot to comment on the patch itself :P\n\nLe 2022-05-07 à 10:20, Abhradeep Chakraborty via GitGitGadget a écrit :\n> From: Abhradeep Chakraborty \n> \n\n> Documentation/git-remote.txt | 3 +++\n> builtin/remote.c | 18 +++++++++++++-----\n> t/t5616-partial-clone.sh | 28 ++++++++++++++++++++++++++++\n\nI think the tests woud fit better in t5505-remote.sh, since the patch really\nadds a feature to the 'git remote' command. \n\n> 3 files changed, 44 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\n> index cde9614e362..a125bd839f7 100644\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n> @@ -35,6 +35,9 @@ OPTIONS\n> -v::\n> --verbose::\n> \tBe a little more verbose and show remote url after name.\n> +\tFor promisor remotes it will show an extra information\n\nI found it sligtly awkward to use the future tense here. Maybe just:\n\n For promisor remotes, also show which filter\n (`blob:none` etc.) that promisor remote use, wrapped in square brackets.\n\nAnd technically, it's not really the remote that \"uses\" the filter, \nbut more the local Git client. So maybe something like this would\nbe more accurate and simpler:\n\n For promisor remotes, also show which filter (`blob:none` etc.)\n are configured, wrapped in square brackets.\n\nAnd even then \"wrapped in square brackets\" *could* be dropped, I \nthink.\n\nApart from that, the patch and test look good, thanks for working\non that!\n\nCheers,\nPhilippe.\n"},{"id":"454994","messageId":"20220509091315.13234-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"f15e2673-ddc3-27ff-d31c-7fa32af27ae7@gmail.com","subject":"Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-09T09:13:15Z","receivedAt":"2022-05-09T09:47:33Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Philippe Blain wrote:\n\n> I think the tests woud fit better in t5505-remote.sh, since the patch really\n> adds a feature to the 'git remote' command. \n\nI think you're right. Thanks!\n\n> I found it sligtly awkward to use the future tense here. Maybe just:\n>\n> For promisor remotes, also show which filter\n> (`blob:none` etc.) that promisor remote use, wrapped in square brackets.\n>\n> And technically, it's not really the remote that \"uses\" the filter, \n> but more the local Git client. So maybe something like this would\n> be more accurate and simpler:\n>\n> For promisor remotes, also show which filter (`blob:none` etc.)\n> are configured, wrapped in square brackets.\n>\n> And even then \"wrapped in square brackets\" *could* be dropped, I \n> think.\n\nGot it. Thanks for the suggestions about both the PR and the patch.\nWill update it.\n\nThanks :)\n"},{"id":"454998","messageId":"pull.1227.v4.git.1652095969026.gitgitgadget@gmail.com","threadId":"57824","inReplyTo":"pull.1227.v3.git.1651933221216.gitgitgadget@gmail.com","subject":"[PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-05-09T11:32:48Z","receivedAt":"2022-05-09T11:32:57Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"From: Abhradeep Chakraborty \n\n`git remote -v` (`--verbose`) lists down the names of remotes along with\ntheir URLs. It would be beneficial for users to also specify the filter\ntypes for promisor remotes. Something like this -\n\n\torigin\tremote-url (fetch) [blob:none]\n\torigin\tremote-url (push)\n\nTeach `git remote -v` to also specify the filters for promisor remotes.\n\nCloses: https://github.com/gitgitgadget/git/issues/1211\nSigned-off-by: Abhradeep Chakraborty \n---\n builtin/remote.c: teach -v to list filters for promisor remotes\n \n Fixes #1211 [1]\n \n Changes since v3:\n \n * tests are moved to t5505-remote.sh\n * Documentation improved\n * Added Closes trailer in the commit message\n \n Changes since v2:\n \n * added more test cases\n * fixed broken indentations\n \n Changes since v1:\n \n * updated documentation\n * renamed url_buf into remote_info_buf\n \n [1] https://github.com/gitgitgadget/git/issues/1211\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1227%2FAbhra303%2Fpromisor_remote-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1227/Abhra303/promisor_remote-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1227\n\nRange-diff vs v3:\n\n 1: 9ac6ca9a08e ! 1: a067435285b builtin/remote.c: teach `-v` to list filters for promisor remotes\n @@ Commit message\n builtin/remote.c: teach `-v` to list filters for promisor remotes\n \n `git remote -v` (`--verbose`) lists down the names of remotes along with\n - their urls. It would be beneficial for users to also specify the filter\n + their URLs. It would be beneficial for users to also specify the filter\n types for promisor remotes. Something like this -\n \n origin remote-url (fetch) [blob:none]\n @@ Commit message\n \n Teach `git remote -v` to also specify the filters for promisor remotes.\n \n + Closes: https://github.com/gitgitgadget/git/issues/1211\n Signed-off-by: Abhradeep Chakraborty \n \n ## Documentation/git-remote.txt ##\n @@ Documentation/git-remote.txt: OPTIONS\n -v::\n --verbose::\n \tBe a little more verbose and show remote url after name.\n -+\tFor promisor remotes it will show an extra information\n -+\t(wrapped in square brackets) describing which filter\n -+\t(`blob:none` etc.) that promisor remote use.\n ++\tFor promisor remotes, also show which filter (`blob:none` etc.)\n ++\tare configured.\n \tNOTE: This must be placed between `remote` and subcommand.\n \n \n @@ builtin/remote.c: static int get_one_entry(struct remote *remote, void *priv)\n \n \treturn 0;\n \n - ## t/t5616-partial-clone.sh ##\n -@@ t/t5616-partial-clone.sh: test_expect_success 'do partial clone 1' '\n - \ttest \"$(git -C pc1 config --local remote.origin.partialclonefilter)\" = \"blob:none\"\n + ## t/t5505-remote.sh ##\n +@@ t/t5505-remote.sh: test_expect_success 'add another remote' '\n + \t)\n '\n \n ++test_expect_success 'setup bare clone for server' '\n ++\tgit clone --bare \"file://$(pwd)/one\" srv.bare &&\n ++\tgit -C srv.bare config --local uploadpack.allowfilter 1 &&\n ++\tgit -C srv.bare config --local uploadpack.allowanysha1inwant 1\n ++'\n ++\n +test_expect_success 'filters for promisor remotes are listed by git remote -v' '\n -+\ttest_when_finished \"rm -rf pc2\" &&\n -+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n -+\tgit -C pc2 remote -v >out &&\n ++\ttest_when_finished \"rm -rf pc\" &&\n ++\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc &&\n ++\tgit -C pc remote -v >out &&\n +\tgrep \"srv.bare (fetch) \\[blob:none\\]\" out &&\n +\n -+\tgit -C pc2 config remote.origin.partialCloneFilter object:type=commit &&\n -+\tgit -C pc2 remote -v >out &&\n ++\tgit -C pc config remote.origin.partialCloneFilter object:type=commit &&\n ++\tgit -C pc remote -v >out &&\n +\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n +'\n +\n +test_expect_success 'filters should not be listed for non promisor remotes (remote -v)' '\n -+\ttest_when_finished \"rm -rf pc2\" &&\n -+\tgit clone \"file://$(pwd)/srv.bare\" pc2 &&\n -+\tgit -C pc2 remote -v >out &&\n ++\ttest_when_finished \"rm -rf pc\" &&\n ++\tgit clone one pc &&\n ++\tgit -C pc remote -v >out &&\n +\t! grep \"(fetch) \\[.*\\]\" out\n +'\n +\n +test_expect_success 'filters are listed by git remote -v only' '\n -+\ttest_when_finished \"rm -rf pc2\" &&\n -+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc2 &&\n -+\tgit -C pc2 remote >out &&\n ++\ttest_when_finished \"rm -rf pc\" &&\n ++\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc &&\n ++\tgit -C pc remote >out &&\n +\t! grep \"\\[blob:none\\]\" out &&\n +\n -+\tgit -C pc2 remote show >out &&\n ++\tgit -C pc remote show >out &&\n +\t! grep \"\\[blob:none\\]\" out\n +'\n +\n - test_expect_success 'verify that .promisor file contains refs fetched' '\n - \tls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&\n - \ttest_line_count = 1 promisorlist &&\n + test_expect_success 'check remote-tracking' '\n + \t(\n + \t\tcd test &&\n\n\n Documentation/git-remote.txt | 2 ++\n builtin/remote.c | 18 +++++++++++++-----\n t/t5505-remote.sh | 34 ++++++++++++++++++++++++++++++++++\n 3 files changed, 49 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex cde9614e362..1dec3148348 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -35,6 +35,8 @@ OPTIONS\n -v::\n --verbose::\n \tBe a little more verbose and show remote url after name.\n+\tFor promisor remotes, also show which filter (`blob:none` etc.)\n+\tare configured.\n \tNOTE: This must be placed between `remote` and subcommand.\n \n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5f4cde9d784..d4b69fe7789 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1185,14 +1185,22 @@ static int show_push_info_item(struct string_list_item *item, void *cb_data)\n static int get_one_entry(struct remote *remote, void *priv)\n {\n \tstruct string_list *list = priv;\n-\tstruct strbuf url_buf = STRBUF_INIT;\n+\tstruct strbuf remote_info_buf = STRBUF_INIT;\n \tconst char **url;\n \tint i, url_nr;\n \n \tif (remote->url_nr > 0) {\n-\t\tstrbuf_addf(&url_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tstruct strbuf promisor_config = STRBUF_INIT;\n+\t\tconst char *partial_clone_filter = NULL;\n+\n+\t\tstrbuf_addf(&promisor_config, \"remote.%s.partialclonefilter\", remote->name);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (fetch)\", remote->url[0]);\n+\t\tif (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))\n+\t\t\tstrbuf_addf(&remote_info_buf, \" [%s]\", partial_clone_filter);\n+\n+\t\tstrbuf_release(&promisor_config);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t} else\n \t\tstring_list_append(list, remote->name)->util = NULL;\n \tif (remote->pushurl_nr) {\n@@ -1204,9 +1212,9 @@ static int get_one_entry(struct remote *remote, void *priv)\n \t}\n \tfor (i = 0; i < url_nr; i++)\n \t{\n-\t\tstrbuf_addf(&url_buf, \"%s (push)\", url[i]);\n+\t\tstrbuf_addf(&remote_info_buf, \"%s (push)\", url[i]);\n \t\tstring_list_append(list, remote->name)->util =\n-\t\t\t\tstrbuf_detach(&url_buf, NULL);\n+\t\t\t\tstrbuf_detach(&remote_info_buf, NULL);\n \t}\n \n \treturn 0;\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c90cf47acdb..fff14e13ed4 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -78,6 +78,40 @@ test_expect_success 'add another remote' '\n \t)\n '\n \n+test_expect_success 'setup bare clone for server' '\n+\tgit clone --bare \"file://$(pwd)/one\" srv.bare &&\n+\tgit -C srv.bare config --local uploadpack.allowfilter 1 &&\n+\tgit -C srv.bare config --local uploadpack.allowanysha1inwant 1\n+'\n+\n+test_expect_success 'filters for promisor remotes are listed by git remote -v' '\n+\ttest_when_finished \"rm -rf pc\" &&\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc &&\n+\tgit -C pc remote -v >out &&\n+\tgrep \"srv.bare (fetch) \\[blob:none\\]\" out &&\n+\n+\tgit -C pc config remote.origin.partialCloneFilter object:type=commit &&\n+\tgit -C pc remote -v >out &&\n+\tgrep \"srv.bare (fetch) \\[object:type=commit\\]\" out\n+'\n+\n+test_expect_success 'filters should not be listed for non promisor remotes (remote -v)' '\n+\ttest_when_finished \"rm -rf pc\" &&\n+\tgit clone one pc &&\n+\tgit -C pc remote -v >out &&\n+\t! grep \"(fetch) \\[.*\\]\" out\n+'\n+\n+test_expect_success 'filters are listed by git remote -v only' '\n+\ttest_when_finished \"rm -rf pc\" &&\n+\tgit clone --filter=blob:none \"file://$(pwd)/srv.bare\" pc &&\n+\tgit -C pc remote >out &&\n+\t! grep \"\\[blob:none\\]\" out &&\n+\n+\tgit -C pc remote show >out &&\n+\t! grep \"\\[blob:none\\]\" out\n+'\n+\n test_expect_success 'check remote-tracking' '\n \t(\n \t\tcd test &&\n\nbase-commit: 0f828332d5ac36fc63b7d8202652efa152809856\n-- \ngitgitgadget\n"},{"id":"455008","messageId":"Ynk0mADTSJU/xVUd@nand.local","threadId":"57824","inReplyTo":"pull.1227.v4.git.1652095969026.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-05-09T15:34:48Z","receivedAt":"2022-05-09T15:34:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 09, 2022 at 11:32:48AM +0000, Abhradeep Chakraborty via GitGitGadget wrote:\n> From: Abhradeep Chakraborty \n>\n> `git remote -v` (`--verbose`) lists down the names of remotes along with\n> their URLs. It would be beneficial for users to also specify the filter\n> types for promisor remotes. Something like this -\n\nThis version looks like it has addressed many (all?) of the comments\npreviously discussed during review. On a quick scan, the code and tests\nlook good to my eyes, too.\n\nBut there was a good question raised by Phillip in\n\n https://lore.kernel.org/git/ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email/\n\nthat I didn't see addressed in your response, which was \"why not put\nthis behind a new `--show-partial-filter` option\"?\n\nI share (what I think is) Junio's feeling that having information that\nis readily available from e.g., running \"git config --get\nremote..partialObjectFilter\" seems redundant. I could understand\nforcing a user to know the config key's name feels like a hurdle. But\ncluttering the output of `git remote -v` seems like the wrong solution\nto that hurdle.\n\nBut I can see where it _would_ be useful. So it would be nice to be able\nto turn the extra output on in those cases, but _only_ those cases, and\na flag would be a nice way to go about doing that.\n\nThanks,\nTaylor\n"},{"id":"455021","messageId":"xmqqilqeekk2.fsf@gitster.g","threadId":"57824","inReplyTo":"aa9884d5-b69a-bfd2-4235-a30326bd65f6@gmail.com","subject":"Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-09T16:29:33Z","receivedAt":"2022-05-09T16:29:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n>> Signed-off-by: Abhradeep Chakraborty \n>> ---\n>> builtin/remote.c: teach -v to list filters for promisor remotes\n>> \n>> Fixes #1211 [1]\n>\n> I don't think this matters much, but if Junio is OK with that, it would\n> be nice to include the reference to the GitGitGadget issue in the commit\n> message itself, though with its full URL, something like:\n>\n> Closes: https://github.com/gitgitgadget/git/issues/1211\n>\n> as another trailer before your signed-off-by. By including it in the \n> commit message we allow the issue to be closed automatically when your topic\n> branch is merged to 'master'. By using the full link we make sure that GitHub \n> knows we are targetting that issue specifically, not any other issue or PR in \n> any fork of Git with the same number.\n\nNice to know. Is there a handy GGG users' guide that mentions these\n\"magic trailers\" (the other one I have seen used is \"Cc:\")?\n\n> Again, small nit to make it easier for reviewers: usually we prefer to see\n> what has changed since the previous version first, and then (if you want, \n> it's not strictly necessary) what changed in the other previous versions. \n\nYup. For a single-patch topic, the following may not apply, but for\na multi-patch topic, a full \"topic overview\" should also be\navailable in the cover letter of the latest version.\n\nA reviewer who was absent while older iterations were reviewed\nshould not have to fish for cover letters of previous iterations to\nlearn what the topic is about to decide if the topic is worth their\ntime to review. Once they get interested enough, they can of course\ndig older iterations, but the job of the cover letter in each\niteration is to allow them to become interested with the least\neffort.\n\nThanks.\n"},{"id":"455023","messageId":"94558e1c-7d0a-8d53-1304-2eecaf6e40fe@gmail.com","threadId":"57824","inReplyTo":"xmqqilqeekk2.fsf@gitster.g","subject":"Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-05-09T16:45:09Z","receivedAt":"2022-05-09T16:45:15Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio,\n\nLe 2022-05-09 à 12:29, Junio C Hamano a écrit :\n> Philippe Blain writes:\n> \n>>> Signed-off-by: Abhradeep Chakraborty \n>>> ---\n>>> builtin/remote.c: teach -v to list filters for promisor remotes\n>>> \n>>> Fixes #1211 [1]\n>>\n>> I don't think this matters much, but if Junio is OK with that, it would\n>> be nice to include the reference to the GitGitGadget issue in the commit\n>> message itself, though with its full URL, something like:\n>>\n>> Closes: https://github.com/gitgitgadget/git/issues/1211\n>>\n>> as another trailer before your signed-off-by. By including it in the \n>> commit message we allow the issue to be closed automatically when your topic\n>> branch is merged to 'master'. By using the full link we make sure that GitHub \n>> knows we are targetting that issue specifically, not any other issue or PR in \n>> any fork of Git with the same number.\n> \n> Nice to know. Is there a handy GGG users' guide that mentions these\n> \"magic trailers\" (the other one I have seen used is \"Cc:\")?\n\n\"CC:\" is GGG-specific, it is mentioned on the GGG homepage, \nhttps://gitgitgadget.github.io/, under \"How can you use GitGitGadget?\".\nIt's also mentioned on the Welcome message GGG adds to the PR for new \ncontributors [1].\n\n\"Fixes\", \"Closes\" etc. are GitHub features (though GitLab implements the same\nfeature), see [2], [3].\n\n[1] https://github.com/gitgitgadget/gitgitgadget/blob/main/res/WELCOME.md#welcome-to-gitgitgadget\n[2] https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword\n[3] https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#closing-issues-automatically\n\nPhilippe.\n"},{"id":"455025","messageId":"54aee42d-fe78-eef1-a371-7ca310a9319f@gmail.com","threadId":"57824","inReplyTo":"Ynk0mADTSJU/xVUd@nand.local","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-05-09T17:01:48Z","receivedAt":"2022-05-09T17:01:51Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Taylor,\n\nLe 2022-05-09 à 11:34, Taylor Blau a écrit :\n> On Mon, May 09, 2022 at 11:32:48AM +0000, Abhradeep Chakraborty via GitGitGadget wrote:\n>> From: Abhradeep Chakraborty \n>>\n>> `git remote -v` (`--verbose`) lists down the names of remotes along with\n>> their URLs. It would be beneficial for users to also specify the filter\n>> types for promisor remotes. Something like this -\n> \n> This version looks like it has addressed many (all?) of the comments\n> previously discussed during review. On a quick scan, the code and tests\n> look good to my eyes, too.\n> \n> But there was a good question raised by Phillip in\n> \n> https://lore.kernel.org/git/ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email/\n> \n> that I didn't see addressed in your response, which was \"why not put\n> this behind a new `--show-partial-filter` option\"?\n\nI originally opened the issue on GGG that this series adresses.\nMy justification, and asnwer to that question, is simple:\n'git remote -v', for me, is a way to ask Git to give me all the information it \nknows about my configured remotes. That's why I thought that it would \nbe really nice if partial clones filters would be shown. \n\nAfter all, 'git remote' is listed in the 'porcelain' section of the \nGit commands [1], and isn't the goal of declaring commands \"porcelain\"\nthat we can make their output more useful to the users without worrying as\nmuch about backwards compatibility than with plumbing commands?\n\n> I share (what I think is) Junio's feeling that having information that\n> is readily available from e.g., running \"git config --get\n> remote..partialObjectFilter\" seems redundant. I could understand\n> forcing a user to know the config key's name feels like a hurdle. But\n> cluttering the output of `git remote -v` seems like the wrong solution\n> to that hurdle.\n\nAs I said above, I have 'git rem' (my alias for 'git remote -v') in my muscle\nmemory and use it when I want to have an outline of my configured remotes.\nI think it would be really easier to add the filters info to the existing output.\nIt's really faster to type than using 'git config', and you do not have to remember\nwhich remote name to query. I think \"clutter\" is a little strong word here :)\n\n> But I can see where it _would_ be useful. So it would be nice to be able\n> to turn the extra output on in those cases, but _only_ those cases, and\n> a flag would be a nice way to go about doing that.\n\nIf really this topic is blocked by \"we do not want to change the default output\nof 'git remote -v'\", then I agree it would be nice to be able to set \n'remote.showFilters' (or similar) to get such output, I agree.\n\nOr, making 'git remote' act like 'git branch' and accept a second '-v', i.e.\n'git remote -vv' would list filters (then I would just adjust my alias :P). \nThen we can outright declare \"the output of 'git remote -vv' is subject to \nfuture changes to show more useful information\", or similar, so we do not\nhave to do the same dance the next time we want to add some other info.\n\nThe downside of hiding such new features behing config values or additional flags\nis that it really, really limits their discoverability. This is something that I \noften think about and think we should really do better in Git, in general. \nFor example, features like 'remote.pushDefault' or the 'diff=*' attribute\nfor language-aware hunk headers (and funcname-limited log/blame etc) are immensely \nuseful, but often even experienced and long-time Git users do not even know they exist, \nbecause they are not covered in \"regular\" Git tutorials...\n\nCheers,\n\nPhilippe.\n\n[1] https://git-scm.com/docs/git#Documentation/git.txt-ahrefdocsgit-remotegit-remote1a\n"},{"id":"455027","messageId":"20220509172157.28593-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"Ynk0mADTSJU/xVUd@nand.local","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-09T17:21:57Z","receivedAt":"2022-05-09T17:22:26Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Taylor Blau wrote:\n\n> But there was a good question raised by Phillip in\n>\n> https://lore.kernel.org/git/ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email/\n>\n> that I didn't see addressed in your response, which was \"why not put\n> this behind a new `--show-partial-filter` option\"?\n\nActually, I addressed it[1] -\n\n> ... Another point is that\n> it's important for an user to know which one is a promisor remote and what\n> filter type they use. If we go with the current implementation the output\n> would be let's say - \n> origin (fetch)\n> origin (push)\n> upstream (fetch)\n> upstream (push)\n>\n> By seeing the above output anyone may assume that all the remotes are\n> normal remotes. If the user now try to run `git pull origin` and suddenly\n> he/she discover that some blobs are not downloaded. He/she run the above\n> mentioned (1) command and find that this is a promisor remote!\n>\n> Here `remote -v` didn't warn the user about the origin remote being an\n> promisor remote. Instead it makes him/her assume that all are normal\n> remotes. Providing only these three info (i.e. , \n> and ) is not sufficient - it only shows the half of the picture.\n\nIf we use a new `--show-partial-clone` flag, users can get to know about\npromisor remotes only if he/she use this flag. As I said in the refered\ncomment, it may happen that the user unfortunately use the flag AFTER the\naccident - to know about if that was the promisor remote!\n\nSee this also[2] - \n\n> ... If\n> we can specify `(fetch)` in the output then why not the filter of that\n> `fetch` on which the behaviour of `fetch` functionality highly depends?\n\nTaylor Blau wrote:\n\n> But I can see where it _would_ be useful. So it would be nice to be able\n> to turn the extra output on in those cases, but _only_ those cases, and\n> a flag would be a nice way to go about doing that.\n\nAdding the extra flag is not a good approach to me due to the above reason.\nBut at the end of the day, all of you have a lots of experience in this field\nthan me. You all could better tell which one is better approach.\n\n\n[1] https://lore.kernel.org/git/20220501193807.94369-1-chakrabortyabhradeep79@gmail.com/\n[2] https://lore.kernel.org/git/20220502145624.12702-1-chakrabortyabhradeep79@gmail.com/\n\nThanks :)\n"},{"id":"455029","messageId":"20220509174442.28647-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"Ynk0mADTSJU/xVUd@nand.local","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-09T17:44:42Z","receivedAt":"2022-05-09T17:45:23Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Taylor Blau wrote:\n\n> This version looks like it has addressed many (all?) of the comments\npreviously discussed during review.\n\nTo my knowledge, yeah, I addressed all the comments :)\n\n\nThanks :)\n"},{"id":"455031","messageId":"xmqqmtfqd25h.fsf@gitster.g","threadId":"57824","inReplyTo":"54aee42d-fe78-eef1-a371-7ca310a9319f@gmail.com","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-09T17:52:26Z","receivedAt":"2022-05-09T17:52:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> Or, making 'git remote' act like 'git branch' and accept a second '-v', i.e.\n> 'git remote -vv' would list filters (then I would just adjust my alias :P). \n> Then we can outright declare \"the output of 'git remote -vv' is subject to \n> future changes to show more useful information\", or similar, so we do not\n> have to do the same dance the next time we want to add some other info.\n\nIsn't it where we already are with \"remote -v\", though? I am not\nsure addition of excess information that may not be universally\nuseful is a very welcome change, even with \"remote -v -v\". I am not\nworried about showing the \"list-object-filter\", but I worry about\nmanaging temptations of future developers to add other stuff.\n\n> The downside of hiding such new features behing config values or additional flags\n> is that it really, really limits their discoverability. This is something that I \n> often think about and think we should really do better in Git, in general. \n> For example, features like 'remote.pushDefault' or the 'diff=*' attribute\n> for language-aware hunk headers (and funcname-limited log/blame etc) are immensely \n> useful, but often even experienced and long-time Git users do not even know they exist, \n> because they are not covered in \"regular\" Git tutorials...\n\nUnfortunately, it is not exactly a solution for that to update the\ntutorial, because experienced and long-time users rightly consider\nthemselves beyond tutorials and sometimes documentation.\n"},{"id":"455059","messageId":"YnmUH5MKeKiafn94@nand.local","threadId":"57824","inReplyTo":"20220509172157.28593-1-chakrabortyabhradeep79@gmail.com","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-05-09T22:22:23Z","receivedAt":"2022-05-09T22:22:39Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 09, 2022 at 10:51:57PM +0530, Abhradeep Chakraborty wrote:\n> Taylor Blau wrote:\n>\n> > But there was a good question raised by Phillip in\n> >\n> > https://lore.kernel.org/git/ab047b4b-6037-af78-1af6-ad35ac6d7c90@iee.email/\n> >\n> > that I didn't see addressed in your response, which was \"why not put\n> > this behind a new `--show-partial-filter` option\"?\n>\n> Actually, I addressed it[1] -\n\nAh, sorry that I missed it! I think Phillipe's GGG issue is probably a\ngood signal that we are not making this information as discoverable to\nusers as we could be.\n\nI share Junio's concern that this change may tempt future contributors\nto add more output still to \"git remote\", but perhaps not. So I'd be OK\nwith this change as-is.\n\n> [1] https://lore.kernel.org/git/20220501193807.94369-1-chakrabortyabhradeep79@gmail.com/\n\nThanks,\nTaylor\n"},{"id":"455263","messageId":"20220513134946.1581-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"xmqqmtfqd25h.fsf@gitster.g","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-13T13:49:46Z","receivedAt":"2022-05-13T13:53:37Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Junio C Hamano wrote:\n\n> Isn't it where we already are with \"remote -v\", though? I am not\n> sure addition of excess information that may not be universally\n> useful is a very welcome change, even with \"remote -v -v\". I am not\n> worried about showing the \"list-object-filter\", but I worry about\n> managing temptations of future developers to add other stuff.\n\nIf future developers come up with some really useful stuff (i.e. \nuniversally useful), I think those should be added in the output\nirrespective of the no of existing info in the output. If the\noutput becomes messy, we should focus on how we can make the output\nclear may be using tabular format.\n\nElse you can drop the idea and suggest them to introduce a new flag\n(depending on the situation). If you still have some doubt about my\nPR i.e. if you can not determine which category my PR belongs to, I\ncan go with adding `show-partial-clone` flag. The downside would\nbe that `remote -v` will not give the full summary in case of partial\nclone.\n\nIf you like the tabular format approach, I am further going to propose\na table format -\n\n+---------------+----------------------------------------------+\n| remote name | remote info |\n+---------------+--------+--------+----------------------------+\n| | | url | https://blah.com/blah.git |\n| origin | +--------+----------------------------+\n| | | filter | blob:none |\n| | fetch +--------+----------------------------+\n| | | . |\n| | | . (some important data) |\n| +--------+--------+----------------------------+\n| | | url | https://blah.com/blah.git |\n| | push +--------+----------------------------+\n| | | ... (some important data) |\n+---------------+--------+-------------------------------------+\n\nIn this way, user can see the summary of all remotes with visual ease.\nOf course it is not suitable for scripting. In that case we can use\na new flag `--raw` which will let `-v` to provide a space/tab seperated\nsequence of info (similar to current format).\n\nLet me know if you (as in all) like/dislike my view and give your\narguments regarding my proposal.\n\nThanks :) \n"},{"id":"455271","messageId":"xmqqk0ap9t4f.fsf@gitster.g","threadId":"57824","inReplyTo":"20220513134946.1581-1-chakrabortyabhradeep79@gmail.com","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-13T18:37:04Z","receivedAt":"2022-05-13T18:37:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhradeep Chakraborty writes:\n\n> Else you can drop the idea and suggest them to introduce a new flag\n> (depending on the situation). If you still have some doubt about my\n> PR i.e. if you can not determine which category my PR belongs to, I\n> can go with adding `show-partial-clone` flag. The downside would\n> be that `remote -v` will not give the full summary in case of partial\n> clone.\n\nIf majority of partial-clone users find it unnecessary noise, then\nit may be an upside to give only reduced summary that is less than\nfull that may be given by `remote -v -v`.\n\nWorse downside of adding it as an option is that it invites more\noptions. It is less worse to add new ones to `remote -v -v` (or to\n`remote -v`, or not adding it at all) than adding another option, I\nwould think.\n\nPerhaps tagged output that can be easier to parse would be better\n\"extensible\" output format for adding more random pieces of\ninformation than going tabular. I dunno.\n"},{"id":"455323","messageId":"20220516153846.18092-1-chakrabortyabhradeep79@gmail.com","threadId":"57824","inReplyTo":"xmqqk0ap9t4f.fsf@gitster.g","subject":"Re: [PATCH v4] builtin/remote.c: teach `-v` to list filters for promisor remotes","fromName":"Abhradeep Chakraborty","fromEmail":"chakrabortyabhradeep79@gmail.com","sentAt":"2022-05-16T15:38:46Z","receivedAt":"2022-05-16T15:39:15Z","isPatch":true,"sender":{"key":"chakrabortyabhradeep79@gmail.com","avatar":"https://avatars.githubusercontent.com/u/75240995?v=4"},"body":"Junio C Hamano wrote:\n\n> If majority of partial-clone users find it unnecessary noise, then\n> it may be an upside to give only reduced summary that is less than\n> full that may be given by `remote -v -v`.\n\nShould I add this to `remote -v -v` then? `remote -vv` is currently\nnot implemented I guess.\n\n> Perhaps tagged output that can be easier to parse would be better\n> \"extensible\" output format for adding more random pieces of\n> information than going tabular. I dunno.\n\nI am not sure what exactly you are refering by 'tagged output' but\nit is true that tabular form is hard to parse. That's why I suggested\n`--raw` flag which would be used for parsing. It would give the info\nfollowing the currently implemented format.\n\nIf you like the tagged output format, then should we implement `-vv` which\nwould give the output as the tagged output format and also can be\nextended?\n\n"}]}