{"thread":{"id":"57244","subject":"[RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","startedAt":"2022-01-14T04:24:31Z","lastAt":"2022-01-17T06:28:03Z","messageCount":18,"participants":["Teng Long","Junio C Hamano","Ævar Arnfjörð Bjarmason","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"446200","messageId":"cover.1642129840.git.dyroneteng@gmail.com","threadId":"57244","inReplyTo":null,"subject":"[RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-01-14T04:24:22Z","receivedAt":"2022-01-14T04:24:31Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"It has a different results by executing \"git ls-remote -heads origin\"\nand \"git ls-remote origin --heads\". A test for reproduction is as\nfollows:\n\n@@ -0,0 +1,17 @@\n+#!/bin/sh\n+ \n+test_description='test ls-tree for reproduction'\n+\n+. ./test-lib.sh\n+\n+\n+test_must_fail 'Exchange the order of \"--heads\" and ' '\n+ git --version &&\n+ git init \"test.git\" &&\n+ test_commit -C \"test.git\" one &&\n+ git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n+ git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n+ test_cmp result.1 result.2\n+'\n+\n+test_done\n--\n2.34.1.391.g9ef3d6f133\n\n\"git -C \"test.git\" ls-remote ./. --heads\" returns nothing here because\n\"ls-remote\" supports to treat args as patterns to filter the refs list\n, and \"--heads\" here as the pattern does not match any.\n\nIf a repo exists a ref which is named as \"refs/heads/--heads\", execute\n`mgit ls-remote origin --heads` will return (\"mgit\" is a alias means\nthe git binary that is built by \"master\" in my env):\n \n 0319b78d7c31e5203d79157fe82960db88cc4a54 refs/heads/--heads\n\nIf we execute again in terms of this patch by `git ls-remote origin --heads`:\n\n 0319b78d7c31e5203d79157fe82960db88cc4a54 refs/heads/--heads\n 0319b78d7c31e5203d79157fe82960db88cc4a54 refs/heads/master\n\nAnd if we hope to let \"--heads\" to be treated as a \"pattern\", we can execute\n`git ls-remote origin --heads -- \"--heads\"`:\n\n 0319b78d7c31e5203d79157fe82960db88cc4a54 refs/heads/--heads\n\nPersonally, I think the second option is probably a little better, but there\nare also possible backward compatibility issues, such as users ending branch\nnames with \"-heads\" and so on.\n\nSo I sent this patch to try to figure out if this was a requirement of the\noriginal design, or if it was something that could really be improved, or if\nwe can find a better way to deal this issue with compatibility.\n\nThanks.\n\nTeng Long (1):\n ls-remote: Make the output independent of the order of opts and\n \n\n builtin/ls-remote.c | 3 +--\n t/t5512-ls-remote.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\nRange-diff against v0:\n-: ---------- > 1: 9ef3d6f133 ls-remote: Make the output independent of the order of opts and \n-- \n2.34.1.391.g9ef3d6f133\n\n"},{"id":"446201","messageId":"9ef3d6f1338b06177ed22ed9453e2dcc9f1c3b7f.1642129840.git.dyroneteng@gmail.com","threadId":"57244","inReplyTo":"cover.1642129840.git.dyroneteng@gmail.com","subject":"[RFC PATCH v1 1/1] ls-remote: Make the output independent of the order of opts and ","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-01-14T04:24:23Z","receivedAt":"2022-01-14T04:24:35Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"This commit is used to solve the problem of inconsistent execution\nresults caused by switching the order of parameters and options\nin some scenarios. For example, if we execute the following commands:\n\n $/opt/git/master/bin/git ls-remote .git\n >3ea29ad061137e321853d64fb38b6c6e907546a0 HEAD\n >3ea29ad061137e321853d64fb38b6c6e907546a0 refs/heads/master\n >3 ea29ad061137e321853d64fb38b6c6e907546a0 refs/tags/v1.0\n\n $/opt/git/master/bin/git ls-remote --heads .git\n >3ea29ad061137e321853d64fb38b6c6e907546a0 refs/heads/master\n\n $/opt/git/master/bin/git ls-remote .git --heads\n >\n\n Please omit the \">\" because they are just to make the output more\n intuitive. The tests are based on 2ae0a9cb82 (The fifth batch, 2021-12-22).\n in master.\n\nSimilarily \"--tags\", \"--refs\" and \"--symref\" also produces the same result\n(no output print in this case).\n\nThe issue is caused by `ls-remote` command supports args as \"patterns\", which\nwill filter the refs list from through the trailing matching rule. In\nthese usages, Options can be differentiated and processed more accurately with\n\"patterns\", otherwise the result might lead to confusion if user has a loss of\nthe context.\n\nThis commit sets \"parse_opt_flags\" value in \"parse_options()\" to 0, this will\nlet the commands produce the same result in this scenario. At the same time,\nthis commit does not break the current test cases in t5512.\n\nSigned-off-by: Teng Long \n---\n builtin/ls-remote.c | 3 +--\n t/t5512-ls-remote.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 44448fa61d..45eeecdb71 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -80,8 +80,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \n \tmemset(&ref_array, 0, sizeof(ref_array));\n \n-\targc = parse_options(argc, argv, prefix, options, ls_remote_usage,\n-\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\targc = parse_options(argc, argv, prefix, options, ls_remote_usage, 0);\n \tdest = argv[0];\n \n \tpacket_trace_identity(\"ls-remote\");\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex f53f58895a..7c51cc23d1 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -349,4 +349,28 @@ test_expect_success 'ls-remote prefixes work with all protocol versions' '\n \ttest_cmp expect actual.v2\n '\n \n+test_expect_success 'ls-remote same result if exchange the order of --heads and ' '\n+\tgit ls-remote --heads self > expect.heads &&\n+\tgit ls-remote self --heads > actual.heads &&\n+\ttest_cmp expect.heads actual.heads\n+'\n+\n+test_expect_success 'ls-remote same result if exchange the order of --tags and ' '\n+\tgit ls-remote --tags self > expect.tags &&\n+\tgit ls-remote self --tags > actual.tags &&\n+\ttest_cmp expect.tags actual.tags\n+'\n+\n+test_expect_success 'ls-remote same result if exchange the order of --refs and ' '\n+\tgit ls-remote --refs self > expect.refs &&\n+\tgit ls-remote self --refs > actual.refs &&\n+\ttest_cmp expect.refs actual.refs\n+'\n+\n+test_expect_success 'ls-remote same result if exchange the order of --symref and ' '\n+\tgit ls-remote --symref self > expect.symref &&\n+\tgit ls-remote self --symref > actual.symref &&\n+\ttest_cmp expect.symref actual.symref\n+'\n+\n test_done\n-- \n2.34.1.391.g9ef3d6f133\n\n"},{"id":"446202","messageId":"xmqqfspqeun5.fsf@gitster.g","threadId":"57244","inReplyTo":"cover.1642129840.git.dyroneteng@gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-14T05:47:10Z","receivedAt":"2022-01-14T05:47:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> +test_must_fail 'Exchange the order of \"--heads\" and ' '\n> + git --version &&\n> + git init \"test.git\" &&\n> + test_commit -C \"test.git\" one &&\n> + git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n> + git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n\nI would say that this is working exactly as designed. As with the\nunix tradition, after the command name, first come options\n(e.g. \"--heads\", \"-v\", etc. that begin with a dash or two dashes),\nthen arguments like \"origin\", \"master\", \".\" that are not dashed\noptions/flags.\n\nThen among the arguments, we generally take revs first and then\npathspecs. \"git help cli\" explicitly mentions this, because it is\nspecific to \"git\" command suite, but it does not mention \"dashed\noptions/flags first and then args\", primarily because, at least back\nwhen the documentation was written, this was taken as granted, iow,\nthose who wrote the \"gitcli\" documentation thought it was a common\nknowledge among users that did not need to be spelled out.\n\nApparently, it is not a common knowledge at least for you (and\nprobably others). Perhaps we should add a paragraph to the cli help\nand explicitly mention \"options first and then args\", before we go\non to say \"among args, revs first and then pathspecs\".\n\n\n\n\n"},{"id":"446204","messageId":"xmqqy23ide7z.fsf@gitster.g","threadId":"57244","inReplyTo":"xmqqfspqeun5.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-14T06:27:12Z","receivedAt":"2022-01-14T06:27:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Teng Long writes:\n>\n>> +test_must_fail 'Exchange the order of \"--heads\" and ' '\n>> + git --version &&\n>> + git init \"test.git\" &&\n>> + test_commit -C \"test.git\" one &&\n>> + git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n>> + git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n>\n> I would say that this is working exactly as designed. As with the\n> unix tradition, after the command name, first come options\n> (e.g. \"--heads\", \"-v\", etc. that begin with a dash or two dashes),\n> then arguments like \"origin\", \"master\", \".\" that are not dashed\n> options/flags.\n\nI failed to say one important thing (I was again fooled by the \"it\nis too obvious to say\" that led \"gitcli\" not to mention this, too).\n\n\"dashed-options first and then args\" rule means that generally we\nscan the command line from left to right one by one, and as long as\nthe one we are looking at begins with \"-\", we try to interpret it as\na dashed option, possibly eat the next one as an argument given to\nthe option (e.g. \"--abbrev 7\"), and keep repeating, until the one we\nare at right now does not begin with \"-\". And then everything after\nthat, we do not interpret it as an option. That is how \"--heads\" on\nthe second example above, since we have seen . and took it as an\nargument (not a dashed option) is considered as a pattern.\n\n> ...\n> Apparently, it is not a common knowledge at least for you (and\n> probably others). Perhaps we should add a paragraph to the cli help\n> and explicitly mention \"options first and then args\", before we go\n> on to say \"among args, revs first and then pathspecs\".\n\n"},{"id":"446205","messageId":"CADMgQSQC4T+MBgbFROy0cmB8FFk=EgikPfErwpMbduaFvEebDA@mail.gmail.com","threadId":"57244","inReplyTo":"xmqqfspqeun5.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-01-14T06:42:36Z","receivedAt":"2022-01-14T06:42:49Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"On Fri, Jan 14, 2022 at 1:47 PM Junio C Hamano wrote:\n\n> Apparently, it is not a common knowledge at least for you (and\n> probably others). Perhaps we should add a paragraph to the cli help\n> and explicitly mention \"options first and then args\", before we go\n> on to say \"among args, revs first and then pathspecs\".\n\nIt's much clearer now, thanks for the detailed answer.\n\nAnother question, if I want to follow your advice and add a short\nparagraph in git CLI document, should this patch continue in the\ncurrent RFC patchset or launch a new patchset?\n\nThanks.\n"},{"id":"446245","messageId":"220114.867db2rs0n.gmgdl@evledraar.gmail.com","threadId":"57244","inReplyTo":"xmqqfspqeun5.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-14T19:57:17Z","receivedAt":"2022-01-14T20:17:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jan 13 2022, Junio C Hamano wrote:\n\n> Teng Long writes:\n>\n>> +test_must_fail 'Exchange the order of \"--heads\" and ' '\n>> + git --version &&\n>> + git init \"test.git\" &&\n>> + test_commit -C \"test.git\" one &&\n>> + git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n>> + git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n>\n> I would say that this is working exactly as designed. As with the\n> unix tradition, after the command name, first come options\n> (e.g. \"--heads\", \"-v\", etc. that begin with a dash or two dashes),\n> then arguments like \"origin\", \"master\", \".\" that are not dashed\n> options/flags.\n>\n> Then among the arguments, we generally take revs first and then\n> pathspecs. \"git help cli\" explicitly mentions this, because it is\n> specific to \"git\" command suite, but it does not mention \"dashed\n> options/flags first and then args\", primarily because, at least back\n> when the documentation was written, this was taken as granted, iow,\n> those who wrote the \"gitcli\" documentation thought it was a common\n> knowledge among users that did not need to be spelled out.\n>\n> Apparently, it is not a common knowledge at least for you (and\n> probably others). Perhaps we should add a paragraph to the cli help\n> and explicitly mention \"options first and then args\", before we go\n> on to say \"among args, revs first and then pathspecs\".\n\nI don't think this summary is accurate.\n\nWe have multiple commands that are in GNU-fashion loose about whether\nyou provide options first before no-option args, or after. E.g. we\naccept both of:\n\n git push --dry-run \n\nAnd:\n\n git push --dry-run\n\nThe \"tradition\" you're referring to accurately summarizes how POSIX\nspecifies that things should work.\n\nBut when GNU came around its option parser was generally happy to accept\noptions and args in either order. E.g. these both work with GNU\ncoreutils, but the latter will fail on FreeBSD and various other\ncapital-U UNIX-es:\n\n touch foo; rm -v foo\n touch foo; rm foo -v\n\nThe relevant documentation being\nhttps://www.gnu.org/prep/standards/standards.html#Standards-for-Command-Line-Interfaces;\nspecifically:\n\n \"Note that the GNU version of getopt will normally permit options\n anywhere among the arguments unless the special argument ‘--’ is\n used. This is not what POSIX specifies; it is a GNU extension.\"\n\nNow, as to git's behavior we mostly follow the GNU style. The default\nbehavior of parse_options() is to accept the looser GNU style. In this\ncase we're explicitly passing the PARSE_OPT_STOP_AT_NON_OPTION flag.\n\nBut why is that?\n\nMost things that do that are doing so because we're parsing things in\ntwo passes, i.e. for commands that have sub-commands, e.g.:\n\n git commit-graph [opts] [subcommand] [subcommand-opts]\n\nIn those cases we use PARSE_OPT_STOP_AT_NON_OPTION on our first pass, so\nthat we'll stop at \"subcommand\", then we'll generally continue the\nparsing as of \"[subcommand]\" without that flag.\n\nThe general exception to that rule, as you note, being things that take\npathspecs, although for some of those we'll apply other custom\ndisambiguation.\n\nIn this case a look at ba5f28bf79e (ls-remote: use parse-options api,\n2016-01-19) will show that it was faithfully converted from an ad-hoc\ncustom option parser using the \"BSD-like\" option parsing discussed\nabove.\n\nI really don't see why we wouldn't take this patch given that the\ncommand has one parse_options() invocation (so no subcommand\ncomplexity), and doesn't accept pathspecs etc.\n\nWe should aim to make the command line UX consistent when possible, not\nslavishly bug-for-bug follow whatever the behavior of the\npre-parse_options() code was.\n\nWe can't do that in some cases, but this case seems like a pretty clear\ncase where simply removing the PARSE_OPT_STOP_AT_NON_OPTION flag would\nbe an improvement. It's not ambiguous, and makes invocations that\ncurrently (and inconsistently v.s. our usual behavior) error out work.\n"},{"id":"446248","messageId":"xmqqbl0eaw2e.fsf@gitster.g","threadId":"57244","inReplyTo":"220114.867db2rs0n.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-14T20:42:17Z","receivedAt":"2022-01-14T20:42:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We have multiple commands that are in GNU-fashion loose about whether\n> you provide options first before no-option args, or after. E.g. we\n> accept both of:\n>\n> git push --dry-run \n>\n> And:\n>\n> git push --dry-run\n\nYes, but I consider that a bug that we cannot fix due to backward\ncompatibility issues.\n\nThat is why my preference is to encourage users to stick to the\nPOSIX way in gltcli, just like we recommend \"stuck\" form of options\nits parameter.\n\n> But when GNU came around its option parser was generally happy to accept\n> options and args in either order. E.g. these both work with GNU\n> coreutils, but the latter will fail on FreeBSD and various other\n> capital-U UNIX-es:\n>\n> touch foo; rm -v foo\n> touch foo; rm foo -v\n\nYes, among the harm GNU has done on mankind, this is one of the\nbiggest ones. We shouldn't waste our engineering time to support\nmore of them in our tools.\n\nAs long as users stick to the recommended \"dashed options first and\nthen args, among which revs come first and then pathspecs\", they\nwill be fine.\n"},{"id":"446251","messageId":"220114.86y23iqbbk.gmgdl@evledraar.gmail.com","threadId":"57244","inReplyTo":"xmqqbl0eaw2e.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-14T20:57:33Z","receivedAt":"2022-01-14T21:03:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nnOn Fri, Jan 14 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> We have multiple commands that are in GNU-fashion loose about whether\n>> you provide options first before no-option args, or after. E.g. we\n>> accept both of:\n>>\n>> git push --dry-run \n>>\n>> And:\n>>\n>> git push --dry-run\n>\n> Yes, but I consider that a bug that we cannot fix due to backward\n> compatibility issues.\n>\n> That is why my preference is to encourage users to stick to the\n> POSIX way in gltcli, just like we recommend \"stuck\" form of options\n> its parameter.\n>\n>> But when GNU came around its option parser was generally happy to accept\n>> options and args in either order. E.g. these both work with GNU\n>> coreutils, but the latter will fail on FreeBSD and various other\n>> capital-U UNIX-es:\n>>\n>> touch foo; rm -v foo\n>> touch foo; rm foo -v\n\nThis is only an approximate list, but:\n \n $ git grep -C3 'parse_options' -- 'builtin/*.c'|grep -c PARSE_OPT_STOP_AT_NON_OPTION\n 16\n $ git grep -C3 'parse_options' -- 'builtin/*.c'|grep -c -F ', 0);'\n 101\n\nThe GNU-like behavior is far more common in our codebase, and I think\nit's less surprising if commands work the same way for consistency.\n\nI manually looked through the PARSE_OPT_STOP_AT_NON_OPTION cases, and I\nthink this is the only one that's using it for no good reason. The\nothers (e.g. \"git config\") would become ambiguous or error out as a\nresult.\n\n> Yes, among the harm GNU has done on mankind, this is one of the\n> biggest ones. We shouldn't waste our engineering time to support\n> more of them in our tools.\n>\n> As long as users stick to the recommended \"dashed options first and\n> then args, among which revs come first and then pathspecs\", they\n> will be fine.\n\nI find it quite useful. E.g. if you typo a command or forget/want to remove an option:\n\n git push origin HEAD --dry-run\n\nYou can just (under readline) do C-p M-DEL, instead of the equivalent\nnavigating back a few words, or having to use more advanced readline\nfeatures like ^--dry-run^^ or whatever.\n\nAnecdotally, I've been surprised by the amount of regular terminal users\nwhose readline skills pretty much and at using the arrow keys to make\ncommand corrections. I think this GNU UX decision has probably saved\nseveral accumulated man-lifetimes by now :)\n\n"},{"id":"446252","messageId":"YeHnT1BcisbVvQHB@camp.crustytoothpaste.net","threadId":"57244","inReplyTo":"220114.867db2rs0n.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-01-14T21:12:47Z","receivedAt":"2022-01-14T21:12:54Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-01-14 at 19:57:17, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Jan 13 2022, Junio C Hamano wrote:\n> \n> > Teng Long writes:\n> >\n> >> +test_must_fail 'Exchange the order of \"--heads\" and ' '\n> >> + git --version &&\n> >> + git init \"test.git\" &&\n> >> + test_commit -C \"test.git\" one &&\n> >> + git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n> >> + git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n> >\n> > I would say that this is working exactly as designed. As with the\n> > unix tradition, after the command name, first come options\n> > (e.g. \"--heads\", \"-v\", etc. that begin with a dash or two dashes),\n> > then arguments like \"origin\", \"master\", \".\" that are not dashed\n> > options/flags.\n> >\n> > Then among the arguments, we generally take revs first and then\n> > pathspecs. \"git help cli\" explicitly mentions this, because it is\n> > specific to \"git\" command suite, but it does not mention \"dashed\n> > options/flags first and then args\", primarily because, at least back\n> > when the documentation was written, this was taken as granted, iow,\n> > those who wrote the \"gitcli\" documentation thought it was a common\n> > knowledge among users that did not need to be spelled out.\n> >\n> > Apparently, it is not a common knowledge at least for you (and\n> > probably others). Perhaps we should add a paragraph to the cli help\n> > and explicitly mention \"options first and then args\", before we go\n> > on to say \"among args, revs first and then pathspecs\".\n> \n> I don't think this summary is accurate.\n> \n> We have multiple commands that are in GNU-fashion loose about whether\n> you provide options first before no-option args, or after. E.g. we\n> accept both of:\n> \n> git push --dry-run \n> \n> And:\n> \n> git push --dry-run\n> \n> The \"tradition\" you're referring to accurately summarizes how POSIX\n> specifies that things should work.\n>\n> But when GNU came around its option parser was generally happy to accept\n> options and args in either order. E.g. these both work with GNU\n> coreutils, but the latter will fail on FreeBSD and various other\n> capital-U UNIX-es:\n> \n> touch foo; rm -v foo\n> touch foo; rm foo -v\n\nYes, POSIX specifies this is how it should work because it avoids\nambiguity. According to POSIX, -v is a file, and that's a valid name on\nUnix. If GNU rm fails to delete that file or provide a diagnostic about\nwhy it didn't, that's a bug.\n\nIn some cases, we do allow the GNU behavior of providing options\nanywhere on the command line, but we don't when it causes ambiguity,\nlike in this case. I think we should document the current behavior, but\nI also think it's a given when working on Unix because many tools don't\nwork that way. For example, test and find don't permit arbitrary\nlocation of options and arguments and they are found on all Unix\nsystems. You can't write \"test foo -f\".\n\nAnd to prove that this is ambiguous, I provide you the following\nexample:\n\n$ git update-ref refs/heads/--symref HEAD\n$ git ls-remote . --symref\n1ffcbaa1a5f10c9f706314d77f88de20a4a498c2 refs/heads/--symref\n\nThat prints something very different if I write \"git ls-remote --symref\n.\". And it is actually the case that people write this kind of syntax\nin scripts relying on the current behavior and then those scripts get\nused in a variety of situations with arbitrary ref names, so this should\ncontinue to work this way. I believe a former employer may have these\nkinds of scripts, for example.\n\nI'm not opposed to us building new tools which support the GNU behavior,\nbut I don't think we should change tools where we have the existing\nbehavior because it does lead to breakage in some scripting situations.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"446256","messageId":"xmqqo84e9e9g.fsf@gitster.g","threadId":"57244","inReplyTo":"220114.86y23iqbbk.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-14T21:52:11Z","receivedAt":"2022-01-14T21:52:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>>> But when GNU came around its option parser was generally happy to accept\n>>> options and args in either order. E.g. these both work with GNU\n>>> coreutils, but the latter will fail on FreeBSD and various other\n>>> capital-U UNIX-es:\n>>>\n>>> touch foo; rm -v foo\n>>> touch foo; rm foo -v\n>\n> This is only an approximate list, but:\n\nDon't waste your time on this, and instead spend on something more\nuseful. What I gave wrt gitcli.txt in an earlier message is final.\n"},{"id":"446270","messageId":"xmqqwnj17slr.fsf@gitster.g","threadId":"57244","inReplyTo":"CADMgQSQC4T+MBgbFROy0cmB8FFk=EgikPfErwpMbduaFvEebDA@mail.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-15T00:25:20Z","receivedAt":"2022-01-15T00:25:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> On Fri, Jan 14, 2022 at 1:47 PM Junio C Hamano wrote:\n>\n>> Apparently, it is not a common knowledge at least for you (and\n>> probably others). Perhaps we should add a paragraph to the cli help\n>> and explicitly mention \"options first and then args\", before we go\n>> on to say \"among args, revs first and then pathspecs\".\n>\n> It's much clearer now, thanks for the detailed answer.\n>\n> Another question, if I want to follow your advice and add a short\n> paragraph in git CLI document, should this patch continue in the\n> current RFC patchset or launch a new patchset?\n\nIf I were you, I would retract the ls-remote topic and create a\nbrand new topic that is about clarifying/enhancing the gitcli.txt\nfile, which has nothing to do with ls-remote, because the command\nline option/argument convention is not specific to a single command.\n"},{"id":"446271","messageId":"220115.86pmotrgjo.gmgdl@evledraar.gmail.com","threadId":"57244","inReplyTo":"YeHnT1BcisbVvQHB@camp.crustytoothpaste.net","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-15T00:13:48Z","receivedAt":"2022-01-15T00:25:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 14 2022, brian m. carlson wrote:\n\n> [[PGP Signed Part:Undecided]]\n> On 2022-01-14 at 19:57:17, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Thu, Jan 13 2022, Junio C Hamano wrote:\n>> \n>> > Teng Long writes:\n>> >\n>> >> +test_must_fail 'Exchange the order of \"--heads\" and ' '\n>> >> + git --version &&\n>> >> + git init \"test.git\" &&\n>> >> + test_commit -C \"test.git\" one &&\n>> >> + git -C \"test.git\" ls-remote --heads ./. > result.1 &&\n>> >> + git -C \"test.git\" ls-remote ./. --heads > result.2 &&\n>> >\n>> > I would say that this is working exactly as designed. As with the\n>> > unix tradition, after the command name, first come options\n>> > (e.g. \"--heads\", \"-v\", etc. that begin with a dash or two dashes),\n>> > then arguments like \"origin\", \"master\", \".\" that are not dashed\n>> > options/flags.\n>> >\n>> > Then among the arguments, we generally take revs first and then\n>> > pathspecs. \"git help cli\" explicitly mentions this, because it is\n>> > specific to \"git\" command suite, but it does not mention \"dashed\n>> > options/flags first and then args\", primarily because, at least back\n>> > when the documentation was written, this was taken as granted, iow,\n>> > those who wrote the \"gitcli\" documentation thought it was a common\n>> > knowledge among users that did not need to be spelled out.\n>> >\n>> > Apparently, it is not a common knowledge at least for you (and\n>> > probably others). Perhaps we should add a paragraph to the cli help\n>> > and explicitly mention \"options first and then args\", before we go\n>> > on to say \"among args, revs first and then pathspecs\".\n>> \n>> I don't think this summary is accurate.\n>> \n>> We have multiple commands that are in GNU-fashion loose about whether\n>> you provide options first before no-option args, or after. E.g. we\n>> accept both of:\n>> \n>> git push --dry-run \n>> \n>> And:\n>> \n>> git push --dry-run\n>> \n>> The \"tradition\" you're referring to accurately summarizes how POSIX\n>> specifies that things should work.\n>>\n>> But when GNU came around its option parser was generally happy to accept\n>> options and args in either order. E.g. these both work with GNU\n>> coreutils, but the latter will fail on FreeBSD and various other\n>> capital-U UNIX-es:\n>> \n>> touch foo; rm -v foo\n>> touch foo; rm foo -v\n>\n> Yes, POSIX specifies this is how it should work because it avoids\n> ambiguity. According to POSIX, -v is a file, and that's a valid name on\n> Unix. If GNU rm fails to delete that file or provide a diagnostic about\n> why it didn't, that's a bug.\n\nIt's not a bug that its default behavior isn't to slavishly follow\nPOSIX, but if you'd like you can turn it on:\n \n $ touch -- file -v; rm -v file -v\n removed 'file'\n $ touch -- file -v; POSIXLY_CORRECT=1 rm -v file -v\n removed 'file'\n removed '-v'\n\nThere's a good reason for this departure in behavior: If you have a\nsingle file called '-v' POSIX doesn't provide any way to remove it other\nthan something like:\n\n rm ./-v\n\nWhich can be painful when scripting things. I.e. you'll need to munge\nthe name itself, v.s. consistently supporting \"--\":\n\n rm -- -v\n\n> In some cases, we do allow the GNU behavior of providing options\n> anywhere on the command line, but we don't when it causes ambiguity,\n> like in this case. I think we should document the current behavior, but\n> I also think it's a given when working on Unix because many tools don't\n> work that way. For example, test and find don't permit arbitrary\n> location of options and arguments and they are found on all Unix\n> systems. You can't write \"test foo -f\".\n\nYes, *nix systems are all over the place with this. And then e.g. \"dd\"\nand the like accept arguments whose syntax pre-dates \"-\"-prefixed\narguments.\n\nI'm only talking about how our command collection should behave, and how\nwe should explain its behavior, or what behavior we might prefer.\n\n> And to prove that this is ambiguous, I provide you the following\n> example:\n>\n> $ git update-ref refs/heads/--symref HEAD\n> $ git ls-remote . --symref\n> 1ffcbaa1a5f10c9f706314d77f88de20a4a498c2 refs/heads/--symref\n>\n> That prints something very different if I write \"git ls-remote --symref\n> .\". And it is actually the case that people write this kind of syntax\n> in scripts relying on the current behavior and then those scripts get\n> used in a variety of situations with arbitrary ref names, so this should\n> continue to work this way. I believe a former employer may have these\n> kinds of scripts, for example.\n>\n> I'm not opposed to us building new tools which support the GNU behavior,\n> but I don't think we should change tools where we have the existing\n> behavior because it does lead to breakage in some scripting situations.\n\nYes, my claim that it's \"not ambiguous\" in\n<220114.867db2rs0n.gmgdl@evledraar.gmail.com> isn't correct, as you\npoint out.\n\nI suspect those cases are probably too obscure to worry about in\npractice, but yes, we might not want to convert any existing command due\nto those.\n\nBut we should really be recommending use of -- or --end-of-options\nwhenever possible, as it can be hard to know in Git's CLI whether\noptions after args are accepted or not.\n"},{"id":"446273","messageId":"220115.86lezhrg0o.gmgdl@evledraar.gmail.com","threadId":"57244","inReplyTo":"xmqqo84e9e9g.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-15T00:34:28Z","receivedAt":"2022-01-15T00:36:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 14 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>>> But when GNU came around its option parser was generally happy to accept\n>>>> options and args in either order. E.g. these both work with GNU\n>>>> coreutils, but the latter will fail on FreeBSD and various other\n>>>> capital-U UNIX-es:\n>>>>\n>>>> touch foo; rm -v foo\n>>>> touch foo; rm foo -v\n>>\n>> This is only an approximate list, but:\n>\n> Don't waste your time on this, and instead spend on something more\n> useful. What I gave wrt gitcli.txt in an earlier message is final.\n\nWhatever we do with git-ls-remote (which I don't really care all that\nmuch about) gitcli should really be documenting how our tooling behaves.\n\nWhich is what I was mainly pointing out upthread, that your summary of\noptions before other types of args omitted that many utilities support\nthe reverse. Or perhaps you were only describing an asthetic choice\n(which I don't think is worth debating). I'm just talking about what the\nground truth is.\n\nWhat do you think about something like this to clear this up?:\n\ndiff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt\nindex 92e4ba6a2fa..b1387c4fe68 100644\n--- a/Documentation/gitcli.txt\n+++ b/Documentation/gitcli.txt\n@@ -19,6 +19,13 @@ Many commands take revisions (most often \"commits\", but sometimes\n \"tree-ish\", depending on the context and command) and paths as their\n arguments. Here are the rules:\n \n+ * Options are (almost) universally accpted before other types of\n+ arguments, e.g. `git cat-file -t HEAD` or `git push --dry-run\n+ origin`, but in the case of those commands a GNU-style `git\n+ cat-file HEAD -t` and `git push origin --dry-run` would work just\n+ as well. The reverse is often not true, many commands do not accept\n+ options after non-option arguments.\n+\n * Revisions come first and then paths.\n E.g. in `git diff v1.0 v2.0 arch/x86 include/asm-x86`,\n `v1.0` and `v2.0` are revisions and `arch/x86` and `include/asm-x86`\n"},{"id":"446275","messageId":"xmqqr1996cw2.fsf@gitster.g","threadId":"57244","inReplyTo":"220115.86pmotrgjo.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-15T00:50:05Z","receivedAt":"2022-01-15T00:50:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> But we should really be recommending use of -- or --end-of-options\n> whenever possible, as it can be hard to know in Git's CLI whether\n> options after args are accepted or not.\n\nWay before doing so, we should recommend the \"options and then args\"\norder. Users do not have to know which subcommands accepts options\nafter args.\n"},{"id":"446278","messageId":"xmqqk0f16cd5.fsf@gitster.g","threadId":"57244","inReplyTo":"220115.86lezhrg0o.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-15T01:01:26Z","receivedAt":"2022-01-15T01:01:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Which is what I was mainly pointing out upthread, that your summary of\n> options before other types of args omitted that many utilities support\n> the reverse. Or perhaps you were only describing an asthetic choice\n> (which I don't think is worth debating). I'm just talking about what the\n> ground truth is.\n\nThe ground truth is that it is unlikely that we can fix some of our\ncommands so that they stop taking options that are given after args,\nbecause of inertia. But we can teach users especially the new ones\nto always use the canonical order to sidestep the whole \"some\nsubcommands imitate misguided GNUism to make it ambiguous, some\ndon't\" problem.\n\n> What do you think about something like this to clear this up?:\n\nAs we should aspire to fix the misguided \"options can come still\nafter we saw args\" eventually (don't talk back on this point to\nwaste any more of my time on a release day), I do not think it is a\ngood idea to say \"reverse is often not true\" and stopping there.\n\nIt will mislead people to think these \"not true\" commands should\nsomehow be updated to the GNUism in the future. It's the other way\naround.\n\n> + * Options are (almost) universally accpted before other types of\n> + arguments, e.g. `git cat-file -t HEAD` or `git push --dry-run\n> + origin`, but in the case of those commands a GNU-style `git\n> + cat-file HEAD -t` and `git push origin --dry-run` would work just\n> + as well. The reverse is often not true, many commands do not accept\n> + options after non-option arguments.\n\n * A subcommand may take dashed options (which may take their own\n arguments, e.g. \"--max-parents 2\") and arguments. You SHOULD\n give dashed options first and then arguments. Some commands may\n accept dashed options after you have already gave non-option\n arguments (which may make the command ambiguous), but you should\n not rely on it (because eventually we may find a way to fix\n these ambiguity by enforcing the \"options then args\" rule).\n\n"},{"id":"446279","messageId":"220115.86czktrelu.gmgdl@evledraar.gmail.com","threadId":"57244","inReplyTo":"xmqqr1996cw2.fsf@gitster.g","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-15T01:02:15Z","receivedAt":"2022-01-15T01:07:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 14 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> But we should really be recommending use of -- or --end-of-options\n>> whenever possible, as it can be hard to know in Git's CLI whether\n>> options after args are accepted or not.\n>\n> Way before doing so, we should recommend the \"options and then args\"\n> order. Users do not have to know which subcommands accepts options\n> after args.\n\nIf you want to script git to do e.g.:\n\n touch -- foo -r\n git add foo -r\n git rm foo -r\n git status foo -r\n git log foo -r\n\nYou do need to know that those won't work, if that \"foo -r\" is from\ne.g. scripted arguments that those commands will accept or interpret\nthat -r as an argument.\n\nHence suggesting that the user just add \"--\" to resolve the ambiguity,\nas gitcli already discusses.\n"},{"id":"446281","messageId":"xmqq8rvh6biw.fsf@gitster.g","threadId":"57244","inReplyTo":"220115.86czktrelu.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-15T01:19:35Z","receivedAt":"2022-01-15T01:19:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Hence suggesting that the user just add \"--\" to resolve the ambiguity,\n> as gitcli already discusses.\n\nSadly, requiring \"--end-of-options\" is a solution for a problem that\nwe didn't have to create. If we didn't take options written after\narg,\n\n git rm --end-of-options foo -r\n git rm foo ./-r\n\nto force \"-r\" to be interpreted as a filename wouldn't have been\nnecessary. The presence of \"foo\" before \"-r\" would have been\nsufficient.\n\nI agree with you that, unfortunately, we'd need to teach a way (i.e.\n\"--\" or \"--end-of-options\") to defeat this misguided GNUism in some\ncommands. Even if users stick to \"options and then args\", sadly,\nthey need to know it.\n\nBut we do already explain \"--\" and \"--end-of-options\" in gitcli.txt\nso we should be OK.\n"},{"id":"446337","messageId":"CADMgQSSprxWTQF+knZVOFgdOtV56UaHD9bG4jDbC2z_=bXmuRQ@mail.gmail.com","threadId":"57244","inReplyTo":"cover.1642129840.git.dyroneteng@gmail.com","subject":"Re: [RFC PATCH v1 0/1] ls-remote: inconsistency from the order of args and opts","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2022-01-17T06:27:48Z","receivedAt":"2022-01-17T06:28:03Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"On Fri, Jan 14, 2022 at 12:24 PM Teng Long wrote:\n\n> So I sent this patch to try to figure out if this was a requirement of the\n> original design, or if it was something that could really be improved, or if\n> we can find a better way to deal this issue with compatibility.\n\nI really appreciate your comments and explanations.\n\nNow I've cleared that up, personally. It's not a bad thing that there are some\nconventions for use, the point is whether the rules are clear to others. So,\nafter that, I will try to modify a little in \"git-cli.txt\" with a new patchset.\n\nThanks.\n"}]}