{"thread":{"id":"35144","subject":"[PATCH] rev-parse --parseopt: fix handling of optional arguments","startedAt":"2013-10-15T12:00:25Z","lastAt":"2013-10-31T19:35:33Z","messageCount":25,"participants":["Nicolas Vigier","Junio C Hamano","Jonathan Nieder","Johannes Sixt","Jeff King","Philip Oakley","Michael Haggerty"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"229003","messageId":"1381838425-18244-1-git-send-email-boklm@mars-attacks.org","threadId":"35144","inReplyTo":null,"subject":"[PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-15T12:00:25Z","receivedAt":"2013-10-15T12:00:25Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"git rev-parse --parseopt does not allow us to see the difference\nbetween an option with an optional argument starting with a dash, and an\noption with an unset optional argument followed by an other option.\n\nIf I use this script :\n\n  $ cat /tmp/opt.sh\n  #!/bin/sh\n  OPTIONS_SPEC=\"\\\n  git [options]\n  --\n  q,quiet         be quiet\n  S,gpg-sign?     GPG-sign commit\"\n  echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n\nThen the following two commands give us the same result :\n\n  $ /tmp/opt.sh -S -q\n  set -- -S -q --\n  $ /tmp/opt.sh -S-q\n  set -- -S '-q' --\n\nWe cannot know if '-q' is an argument to '-S' or a new option.\n\nWith this patch, rev-parse --parseopt will always give an argument to\noptional options, as an empty string if the argument is unset.\n\nThe same two commands now give us :\n\n  $ /tmp/opt.sh -S -q\n  set -- -S '' -q --\n  $ /tmp/opt.sh -S-q\n  set -- -S '-q' --\n\nWe can now see if '-q' is an argument to '-S' or an other option.\n\nAlso adding two tests in t1502.\n\nThere does not seem to be any shell script git command included in git\nsources tree that is currently using optional arguments and could be\naffected by this change.\n\nSigned-off-by: Nicolas Vigier <boklm@mars-attacks.org>\n---\n builtin/rev-parse.c           |  3 +++\n t/t1502-rev-parse-parseopt.sh | 18 ++++++++++++++++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex de894c7..25e8c74 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -327,6 +327,9 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)\n \tif (arg) {\n \t\tstrbuf_addch(parsed, ' ');\n \t\tsq_quote_buf(parsed, arg);\n+\t} else if (o->flags & PARSE_OPT_OPTARG) {\n+\t\tconst char empty_arg[] = \" ''\";\n+\t\tstrbuf_add(parsed, empty_arg, strlen(empty_arg));\n \t}\n \treturn 0;\n }\ndiff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh\nindex 13c88c9..abe7c2f 100755\n--- a/t/t1502-rev-parse-parseopt.sh\n+++ b/t/t1502-rev-parse-parseopt.sh\n@@ -99,4 +99,22 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou\n \ttest_cmp expect output\n '\n \n+cat > expect <<EOF\n+set -- -C '' --foo --\n+EOF\n+\n+test_expect_success 'test --parseopt -C --foo' '\n+\tgit rev-parse --parseopt -- -C --foo <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <<EOF\n+set -- -C '--foo' --\n+EOF\n+\n+test_expect_success 'test --parseopt -C--foo' '\n+\tgit rev-parse --parseopt -- -C--foo <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n test_done\n-- \n1.8.4\n"},{"id":"229039","messageId":"xmqq4n8ib0uv.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"1381838425-18244-1-git-send-email-boklm@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T22:55:36Z","receivedAt":"2013-10-15T22:55:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Vigier <boklm@mars-attacks.org> writes:\n\n> git rev-parse --parseopt does not allow us to see the difference\n> between an option with an optional argument starting with a dash, and an\n> option with an unset optional argument followed by an other option.\n>\n> If I use this script :\n>\n>   $ cat /tmp/opt.sh\n>   #!/bin/sh\n>   OPTIONS_SPEC=\"\\\n>   git [options]\n>   --\n>   q,quiet         be quiet\n>   S,gpg-sign?     GPG-sign commit\"\n>   echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n>\n> Then the following two commands give us the same result :\n>\n>   $ /tmp/opt.sh -S -q\n>   set -- -S -q --\n>   $ /tmp/opt.sh -S-q\n>   set -- -S '-q' --\n>\n> We cannot know if '-q' is an argument to '-S' or a new option.\n>\n> With this patch, rev-parse --parseopt will always give an argument to\n> optional options, as an empty string if the argument is unset.\n>\n> The same two commands now give us :\n>\n>   $ /tmp/opt.sh -S -q\n>   set -- -S '' -q --\n>   $ /tmp/opt.sh -S-q\n>   set -- -S '-q' --\n\nTwo are different, but the former \"set -- -S '' -q --\" is not what\nyou want, either, no?  -S with an explicit empty argument and -S\nalone without argument may mean two totally different things, which\nis the whole point of \"option with an optional parameter\".  If some\ncode that have been using \"rev-parse --parseopt\" was happy with\n\n\t$ /tmp/opt.sh -S\n        set -- -S --\n\nand then your updated version gave it this instead:\n\n\t$ /tmp/opt.sh -S\n        set -- -S '' --\n\nwouldn't it be a regression to them?\n"},{"id":"229042","messageId":"20131015231427.GF9464@google.com","threadId":"35144","inReplyTo":"1381838425-18244-1-git-send-email-boklm@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-15T23:14:27Z","receivedAt":"2013-10-15T23:14:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nicolas Vigier wrote:\n\n>   $ cat /tmp/opt.sh\n>   #!/bin/sh\n>   OPTIONS_SPEC=\"\\\n>   git [options]\n>   --\n>   q,quiet         be quiet\n>   S,gpg-sign?     GPG-sign commit\"\n>   echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n>\n> Then the following two commands give us the same result :\n>\n>   $ /tmp/opt.sh -S -q\n>   set -- -S -q --\n>   $ /tmp/opt.sh -S-q\n>   set -- -S '-q' --\n>\n> We cannot know if '-q' is an argument to '-S' or a new option.\n\nHmph.\n\nAs Junio mentioned, inserting '' would be a backward-incompatible\nchange.  I don't think it's worth breaking existing scripts.  Probably\nwhat is needed is a new parseopt special character with the new\nsemantics (e.g.,\n\n\tUse ?? to mean the option has an optional argument.  If the\n\toption is supplied without its argument, the argument is taken\n\tto be ''.\n\nor something like\n\n\tUse ?<default> to mean the option has an optional argument.  If\n\tthe option is supplied without its argument and <default> is\n\tnonempty, the argument is taken to be <default>.\n\n).\n\nSensible?\n\nThanks,\nJonathan\n"},{"id":"229044","messageId":"xmqqfvs29kjc.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"20131015231427.GF9464@google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T23:33:27Z","receivedAt":"2013-10-15T23:33:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Nicolas Vigier wrote:\n>\n>>   $ cat /tmp/opt.sh\n>>   #!/bin/sh\n>>   OPTIONS_SPEC=\"\\\n>>   git [options]\n>>   --\n>>   q,quiet         be quiet\n>>   S,gpg-sign?     GPG-sign commit\"\n>>   echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n>>\n>> Then the following two commands give us the same result :\n>>\n>>   $ /tmp/opt.sh -S -q\n>>   set -- -S -q --\n>>   $ /tmp/opt.sh -S-q\n>>   set -- -S '-q' --\n>>\n>> We cannot know if '-q' is an argument to '-S' or a new option.\n>\n> Hmph.\n>\n> As Junio mentioned, inserting '' would be a backward-incompatible\n> change.  I don't think it's worth breaking existing scripts.  Probably\n> what is needed is a new parseopt special character with the new\n> semantics (e.g.,\n>\n> \tUse ?? to mean the option has an optional argument.  If the\n> \toption is supplied without its argument, the argument is taken\n> \tto be ''.\n>\n> or something like\n>\n> \tUse ?<default> to mean the option has an optional argument.  If\n> \tthe option is supplied without its argument and <default> is\n> \tnonempty, the argument is taken to be <default>.\n>\n> ).\n>\n> Sensible?\n\nYou just made these two that the user clearly meant to express two\ndifferent things indistinguishable.\n\n\topt.sh -S\n        opt.sh -S ''\n\nSo I do not think it is sensible. In fact, I do not think there is\nany sensible way to handle a shortopt with optional parameter that\nis not at the end of the command line.\n\nAnd that is exactly why gitcli.txt tells users to use the 'sticked'\nform, and ends the bullet point with:\n\n   An option that takes optional option-argument must be written in\n   the 'sticked' form.\n\nThat still does not give the command line a way to express an option\nthat could take an optional argument without the optional argument\nin the middle of the command line, though.\n"},{"id":"229046","messageId":"20131015234701.GY4589@mars-attacks.org","threadId":"35144","inReplyTo":"xmqq4n8ib0uv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-15T23:47:01Z","receivedAt":"2013-10-15T23:47:01Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"On Tue, 15 Oct 2013, Junio C Hamano wrote:\n\n> Nicolas Vigier <boklm@mars-attacks.org> writes:\n> \n> > git rev-parse --parseopt does not allow us to see the difference\n> > between an option with an optional argument starting with a dash, and an\n> > option with an unset optional argument followed by an other option.\n> >\n> > If I use this script :\n> >\n> >   $ cat /tmp/opt.sh\n> >   #!/bin/sh\n> >   OPTIONS_SPEC=\"\\\n> >   git [options]\n> >   --\n> >   q,quiet         be quiet\n> >   S,gpg-sign?     GPG-sign commit\"\n> >   echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n> >\n> > Then the following two commands give us the same result :\n> >\n> >   $ /tmp/opt.sh -S -q\n> >   set -- -S -q --\n> >   $ /tmp/opt.sh -S-q\n> >   set -- -S '-q' --\n> >\n> > We cannot know if '-q' is an argument to '-S' or a new option.\n> >\n> > With this patch, rev-parse --parseopt will always give an argument to\n> > optional options, as an empty string if the argument is unset.\n> >\n> > The same two commands now give us :\n> >\n> >   $ /tmp/opt.sh -S -q\n> >   set -- -S '' -q --\n> >   $ /tmp/opt.sh -S-q\n> >   set -- -S '-q' --\n> \n> Two are different, but the former \"set -- -S '' -q --\" is not what\n> you want, either, no?  -S with an explicit empty argument and -S\n> alone without argument may mean two totally different things, which\n> is the whole point of \"option with an optional parameter\".  If some\n> code that have been using \"rev-parse --parseopt\" was happy with\n> \n> \t$ /tmp/opt.sh -S\n>         set -- -S --\n> \n> and then your updated version gave it this instead:\n> \n> \t$ /tmp/opt.sh -S\n>         set -- -S '' --\n> \n> wouldn't it be a regression to them?\n\nIndeed, this could be a regression to them. I couldn't find any script\nusing \"rev-parse --parseopt\" with an option with an optional argument,\nbut yes, it doesn't mean that nobody uses that.\n"},{"id":"229047","messageId":"20131015235314.GZ4589@mars-attacks.org","threadId":"35144","inReplyTo":"20131015231427.GF9464@google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-15T23:53:14Z","receivedAt":"2013-10-15T23:53:14Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"On Tue, 15 Oct 2013, Jonathan Nieder wrote:\n\n> Nicolas Vigier wrote:\n> \n> >   $ cat /tmp/opt.sh\n> >   #!/bin/sh\n> >   OPTIONS_SPEC=\"\\\n> >   git [options]\n> >   --\n> >   q,quiet         be quiet\n> >   S,gpg-sign?     GPG-sign commit\"\n> >   echo \"$OPTIONS_SPEC\" | git rev-parse --parseopt $parseopt_extra -- \"$@\"\n> >\n> > Then the following two commands give us the same result :\n> >\n> >   $ /tmp/opt.sh -S -q\n> >   set -- -S -q --\n> >   $ /tmp/opt.sh -S-q\n> >   set -- -S '-q' --\n> >\n> > We cannot know if '-q' is an argument to '-S' or a new option.\n> \n> Hmph.\n> \n> As Junio mentioned, inserting '' would be a backward-incompatible\n> change.  I don't think it's worth breaking existing scripts.  Probably\n> what is needed is a new parseopt special character with the new\n> semantics (e.g.,\n> \n> \tUse ?? to mean the option has an optional argument.  If the\n> \toption is supplied without its argument, the argument is taken\n> \tto be ''.\n> \n> or something like\n> \n> \tUse ?<default> to mean the option has an optional argument.  If\n> \tthe option is supplied without its argument and <default> is\n> \tnonempty, the argument is taken to be <default>.\n> \n> ).\n> \n> Sensible?\n\nYes, I think it's sensible. I will look at this and propose an other\npatch. Thanks.\n"},{"id":"229048","messageId":"20131015235739.GI9464@google.com","threadId":"35144","inReplyTo":"xmqqfvs29kjc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-15T23:57:39Z","receivedAt":"2013-10-15T23:57:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> You just made these two that the user clearly meant to express two\n> different things indistinguishable.\n>\n> \topt.sh -S\n>       opt.sh -S ''\n[...]\n> And that is exactly why gitcli.txt tells users to use the 'sticked'\n> form, and ends the bullet point with:\n>\n>    An option that takes optional option-argument must be written in\n>    the 'sticked' form.\n\nYes, another possibility in that vein would be to teach rev-parse\n--parseopt an OPTIONS_LONG_STICKED output format, and then parse with\n\n\twhile :\n\tdo\n\t\tcase $1 in\n\t\t--gpg-sign)\n\t\t\t... no keyid ...\n\t\t\t;;\n\t\t--gpg-sign=*)\n\t\t\tkeyid=${1#--gpg-sign=}\n\t\t\t...\n\t\t\t;;\n\t\tesac\n\t\tshift\n\tdone\n\nThis still leaves\n\n\topt.sh -S\n\t\nand\n\n\topt.sh -S''\n\nindistinguishable.  Given what the shell passes to execve, I think\nthat's ok.\n\nThe analagous method without preferring long options could work almost\nas well:\n\n\twhile :\n\tdo\n\t\tcase $1 in\n\t\t-S)\n\t\t\t... no keyid ...\n\t\t\t;;\n\t\t-S?*)\n\t\t\tkeyid=${1#-S}\n\t\t\t...\n\t\t\t;;\n\t\tesac\n\t\tshift\n\tdone\n\nbut it mishandles \"--gpg-sign=\" with empty argument.\n\nJonathan\n"},{"id":"229060","messageId":"525E3A80.5000500@viscovery.net","threadId":"35144","inReplyTo":"20131015235739.GI9464@google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-10-16T07:04:32Z","receivedAt":"2013-10-16T07:04:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/16/2013 1:57, schrieb Jonathan Nieder:\n> Junio C Hamano wrote:\n> \n>> You just made these two that the user clearly meant to express two\n>> different things indistinguishable.\n>>\n>> \topt.sh -S\n>>       opt.sh -S ''\n> [...]\n>> And that is exactly why gitcli.txt tells users to use the 'sticked'\n>> form, and ends the bullet point with:\n>>\n>>    An option that takes optional option-argument must be written in\n>>    the 'sticked' form.\n> \n> Yes, another possibility in that vein would be to teach rev-parse\n> --parseopt an OPTIONS_LONG_STICKED output format, and then parse with\n\nAren't you people trying to solve something that can't besolved? What does\n\n  git commit -S LICENSE\n\nmean? Commit the index and sign with the key-id \"LICENSE\" or commit just\nthe file \"LICENSE\" with the default key-id?\n\n-- Hannes\n"},{"id":"229064","messageId":"20131016085316.GA31314@sigill.intra.peff.net","threadId":"35144","inReplyTo":"525E3A80.5000500@viscovery.net","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-16T08:53:16Z","receivedAt":"2013-10-16T08:53:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 16, 2013 at 09:04:32AM +0200, Johannes Sixt wrote:\n\n> > Yes, another possibility in that vein would be to teach rev-parse\n> > --parseopt an OPTIONS_LONG_STICKED output format, and then parse with\n> \n> Aren't you people trying to solve something that can't besolved? What does\n> \n>   git commit -S LICENSE\n> \n> mean? Commit the index and sign with the key-id \"LICENSE\" or commit just\n> the file \"LICENSE\" with the default key-id?\n\nIt means the latter. Because the argument is optional, you must use the\n\"sticked\" form:\n\n  git commit -SLICENSE\n\nIf the caller does not know whether the argument is optional or not,\nthey should use the sticked form to be on the safe side.\n\nThe problem, as I understand it, arises from the fact that shell scripts\ncan use \"git rev-parse --parseopt\" to check and normalize their\narguments. So for example:\n\n  # pretend we got \"-bs\" on our command line\n  set -- -bs\n\n  git rev-parse --parseopt -- \"$@\" <<\\EOF\n  usage...\n  --\n  b!    the \"b\" option\n  s!    the \"s\" option\n  EOF\n\nwould produce:\n\n  set -- -b -s --\n\nThe latter is much easier to parse in the shell, because options are\nsplit, it ends with \"--\", etc. But what is the normalized form for an\noptional argument? It either needs to be consistently \"sticked\" or\n\"unsticked\", either:\n\n  set -- -S '' --     ;# default\n  set -- -S 'foo' --  ;# not default\n\nor\n\n  set -- -S --    ;# default\n  set -- -Sfoo -- ;# not default\n\nso that reading the normalized form is unambiguous.\n\n-Peff\n"},{"id":"229069","messageId":"20131016105811.GA4589@mars-attacks.org","threadId":"35144","inReplyTo":"525E3A80.5000500@viscovery.net","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-16T10:58:11Z","receivedAt":"2013-10-16T10:58:11Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"On Wed, 16 Oct 2013, Johannes Sixt wrote:\n\n> Am 10/16/2013 1:57, schrieb Jonathan Nieder:\n> > Junio C Hamano wrote:\n> > \n> >> You just made these two that the user clearly meant to express two\n> >> different things indistinguishable.\n> >>\n> >> \topt.sh -S\n> >>       opt.sh -S ''\n> > [...]\n> >> And that is exactly why gitcli.txt tells users to use the 'sticked'\n> >> form, and ends the bullet point with:\n> >>\n> >>    An option that takes optional option-argument must be written in\n> >>    the 'sticked' form.\n> > \n> > Yes, another possibility in that vein would be to teach rev-parse\n> > --parseopt an OPTIONS_LONG_STICKED output format, and then parse with\n> \n> Aren't you people trying to solve something that can't besolved? What does\n> \n>   git commit -S LICENSE\n> \n> mean? Commit the index and sign with the key-id \"LICENSE\" or commit just\n> the file \"LICENSE\" with the default key-id?\n\nThe later, as optional arguments needs to be sticked, but I think this\nis not related to the problems with --parseopt.\n\nHere is a summary the problems I think we have with --parseopt and\nproposed solutions. This command makes it possible to parse arguments\nwith a loop like this :\n\n  while test $# != 0\n  do\n        case \"$1\" in\n        -q)\n                GIT_QUIET=t ;;\n        -S)\n                # do something with $2\n                # shift if you think $2 is an optional arg\n                ;;\n        --)\n                shift; break ;;\n        esac\n        shift\n  done\n  # do something with the other arguments\n\nAnd I think the problems with the '?' flag when using this kind of loop\nare :\n\n - You don't know if $2 is the optional argument for -S, or the next option\n \n - You can't use '--' as an optional argument to -S, because you don't\n   know if '--' is the optional argument to -S, or the separator between\n   options and non options.\n\nTo fix this, solution 1) is to always include the optional argument in\n$2, and set it to '' if it is not set. However this brings the problem\nthat you can't distinguish between unset argument and empty argument.\nThe following two become the same :\n  ./opt.sh --gpg-id\n  ./opt.sh --gpg-id=\n\nSolution 2) is to use a flag like ?<default> as suggested by Jonathan.\nNow you can distinguish between unset and empty args, but you can't\nbetween <default> and unset, but this is probably not a big problem as\nyou can select <default> so that it is a value nobody would want to use.\n\nSolution 3) is the OPTIONS_LONG_STICKED output format suggested by\nJonathan. I can't see any problem with this one.\n"},{"id":"229071","messageId":"20131016141411.GB4589@mars-attacks.org","threadId":"35144","inReplyTo":"20131015235739.GI9464@google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-16T14:14:11Z","receivedAt":"2013-10-16T14:14:11Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"On Tue, 15 Oct 2013, Jonathan Nieder wrote:\n\n> Junio C Hamano wrote:\n> \n> > You just made these two that the user clearly meant to express two\n> > different things indistinguishable.\n> >\n> > \topt.sh -S\n> >       opt.sh -S ''\n> [...]\n> > And that is exactly why gitcli.txt tells users to use the 'sticked'\n> > form, and ends the bullet point with:\n> >\n> >    An option that takes optional option-argument must be written in\n> >    the 'sticked' form.\n> \n> Yes, another possibility in that vein would be to teach rev-parse\n> --parseopt an OPTIONS_LONG_STICKED output format, and then parse with\n> \n> \twhile :\n> \tdo\n> \t\tcase $1 in\n> \t\t--gpg-sign)\n> \t\t\t... no keyid ...\n> \t\t\t;;\n> \t\t--gpg-sign=*)\n> \t\t\tkeyid=${1#--gpg-sign=}\n> \t\t\t...\n> \t\t\t;;\n> \t\tesac\n> \t\tshift\n> \tdone\n> \n> This still leaves\n> \n> \topt.sh -S\n> \t\n> and\n> \n> \topt.sh -S''\n> \n> indistinguishable.  Given what the shell passes to execve, I think\n> that's ok.\n> \n> The analagous method without preferring long options could work almost\n> as well:\n> \n> \twhile :\n> \tdo\n> \t\tcase $1 in\n> \t\t-S)\n> \t\t\t... no keyid ...\n> \t\t\t;;\n> \t\t-S?*)\n> \t\t\tkeyid=${1#-S}\n> \t\t\t...\n> \t\t\t;;\n> \t\tesac\n> \t\tshift\n> \tdone\n> \n> but it mishandles \"--gpg-sign=\" with empty argument.\n\nI'm thinking about a patch to add the following two options to rev-parse :\n\n--sticked-opt-args::\n\tOnly meaningful in --parseopt mode. Tells the options parser to\n\toutput options with optional arguments in sticked form. The\n\tdefault is to output them in non-sticked mode, which can be\n\tdifficult to parse unambiguously.\n\n--long-options::\n\tOnly meaningful in --parseopt mode. Tells the options parser to\n\toutput long option names, when available. The default is to use\n\tshort option names when available.\n\n\nWhen you want to handle optional args unambiguously, you use the\n--sticked-opt-args option. And if you think an empty value can be\na meaningful value, you add the --long-options option to be able to\ndistinguish them.\n\nWould it make sense ?\n"},{"id":"229094","messageId":"xmqqsiw051zc.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"20131016085316.GA31314@sigill.intra.peff.net","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-16T21:40:07Z","receivedAt":"2013-10-16T21:40:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... But what is the normalized form for an\n> optional argument? It either needs to be consistently \"sticked\" or\n> \"unsticked\", either:\n>\n>   set -- -S '' --     ;# default\n>   set -- -S 'foo' --  ;# not default\n>\n> or\n>\n>   set -- -S --    ;# default\n>   set -- -Sfoo -- ;# not default\n>\n> so that reading the normalized form is unambiguous.\n\nThe analysis makes sense.  Either form do not let you distinguish\nbetween the case where the end user wanted to explicitly pass \"\" as\nthe optional parameter to -S and the case where she gave -S without\nany optional parameter, though.\n\nWhich pretty much agrees with j6t's (and my earlier) comment that\nthere is no way to solve this issue completely, I think.\n\nIt is an acceptable compromise to use your suggestion as a solution\nthat works for cases other than passing an explicit empty string as\nan optional parameter, I would say, if the limitation is clearly\ndocumented.\n\nThanks.\n"},{"id":"229099","messageId":"20131016215047.GA10260@sigill.intra.peff.net","threadId":"35144","inReplyTo":"xmqqsiw051zc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-16T21:50:47Z","receivedAt":"2013-10-16T21:50:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 16, 2013 at 02:40:07PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > ... But what is the normalized form for an\n> > optional argument? It either needs to be consistently \"sticked\" or\n> > \"unsticked\", either:\n> >\n> >   set -- -S '' --     ;# default\n> >   set -- -S 'foo' --  ;# not default\n> >\n> > or\n> >\n> >   set -- -S --    ;# default\n> >   set -- -Sfoo -- ;# not default\n> >\n> > so that reading the normalized form is unambiguous.\n> \n> The analysis makes sense.  Either form do not let you distinguish\n> between the case where the end user wanted to explicitly pass \"\" as\n> the optional parameter to -S and the case where she gave -S without\n> any optional parameter, though.\n\nI almost mentioned that, but I am not sure that it matters. Keep in mind\nthat:\n\n  git my-script -S foo\n\nalready does not involve \"foo\" with S, because it is not \"sticked\". So\nthere is no way for the _user_ to distinguish between \"I want the\ndefault\" and \"I passed you an empty string\"; because the argument must\nbe sticked they both look like \"-S\". And that distinction is already\nimpossible in the definition of optional arguments, and is not a problem\nwith going through the \"git rev-parse --parseopt\" channel at all.\n\nSo the only bug is the ambiguity in the current normalized form. Of the\ntwo forms above, I think I prefer:\n\n  set -- -S '' --\n\nbecause it more closely matches the non-optional form we produce\ntoday, and because it is slightly less work to parse (you can check that\n$1 is \"-S\", and then store or check \"$2\", rather than having to match\n\"-S*\" and parse off the beginning).\n\n> Which pretty much agrees with j6t's (and my earlier) comment that\n> there is no way to solve this issue completely, I think.\n\nI guess it depends on what the issue is. :)\n\nNo, I do not think you can ever \"fix\" the options to let those two cases\nbe distinguishable. But I do not think anybody is really asking for\nthat; the real concern is that the \"rev-parse --parseopt\" normalization\nis ambiguous, and that is easily fixable.\n\n-Peff\n"},{"id":"229104","messageId":"20131016223306.GN9464@google.com","threadId":"35144","inReplyTo":"20131016141411.GB4589@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-16T22:33:06Z","receivedAt":"2013-10-16T22:33:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Nicolas Vigier wrote:\n\n> I'm thinking about a patch to add the following two options to rev-parse :\n>\n> --sticked-opt-args::\n> \tOnly meaningful in --parseopt mode. Tells the options parser to\n> \toutput options with optional arguments in sticked form. The\n> \tdefault is to output them in non-sticked mode, which can be\n> \tdifficult to parse unambiguously.\n>\n> --long-options::\n> \tOnly meaningful in --parseopt mode. Tells the options parser to\n> \toutput long option names, when available. The default is to use\n> \tshort option names when available.\n>\n> When you want to handle optional args unambiguously, you use the\n> --sticked-opt-args option. And if you think an empty value can be\n> a meaningful value, you add the --long-options option to be able to\n> distinguish them.\n>\n> Would it make sense ?\n\nThat would make four distinct output formats:\n\n --sticked --long:\n\tDoesn't lose any information that normal use of C parse_options\n\twould have kept, as long as every short option with optional\n\targument has a corresponding long option.\n\n\n --sticked --no-long:\n\tLoses the distinction between --gpg-sign and --gpg-sign=\n\n --no-sticked --long:\n\tSemantically equivalent to the existing output, just noisier.\n\n --no-sticked --no-long:\n \tThe existing output.\n\nAre all of them needed?  Is it worth tempting people to use --sticked\n--no-long when we know its pitfalls?\n\nI would think that only the current normalized form and --sticked\n--long would need to be supported.\n\nThe fix you originally proposed seems tolerable to me, too --- it is\nnot very invasive, and while it doesn't distinguish the empty-argument\nform \"--gpg-sign=\", that's a bit of an edge case.\n\nThe main reason I slightly prefer the solution that makes the output\nuse long, sticked options on request is that the \"normalized\"\ncommandline would start being an actual equivalent command line the\ncommand expects, instead of a weird, subtly different syntax.  (That\nproblem already exists with or without your patch --- the patch just\ndraws attention to it.)\n"},{"id":"229524","messageId":"1382732291-5701-1-git-send-email-boklm@mars-attacks.org","threadId":"35144","inReplyTo":"20131016223306.GN9464@google.com","subject":"[PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-25T20:18:11Z","receivedAt":"2013-10-25T20:18:11Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"Add the --sticked-long option to output the options in their long form\nif available, and with their arguments sticked.\n\nContrary to the default form (non sticked arguments and short options),\nthis can be parsed unambiguously when using options with optional\narguments :\n\n - in the non sticked form, when an option is taking an optional argument\n   you cannot know if the next argument is its optional argument, or the\n   next option.\n\n - the long options form allows to differenciate between an empty argument\n   '--option=' and an unset argument '--option', which is not possible\n   with short options.\n\nSigned-off-by: Nicolas Vigier <boklm@mars-attacks.org>\n---\n Documentation/git-rev-parse.txt |  8 +++++++-\n builtin/rev-parse.c             | 11 +++++++++--\n t/t1502-rev-parse-parseopt.sh   | 42 ++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 55 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex d068a65..d3bad9d 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -50,6 +50,10 @@ Options for --parseopt\n \tthe first non-option argument.  This can be used to parse sub-commands\n \tthat take options themselves.\n \n+--sticked-long::\n+\tOnly meaningful in `--parseopt` mode. Output the options in their\n+\tlong form if available, and with their arguments sticked.\n+\n Options for Filtering\n ~~~~~~~~~~~~~~~~~~~~~\n \n@@ -285,7 +289,9 @@ Each line of options has this format:\n \t`<flags>` are of `*`, `=`, `?` or `!`.\n \t* Use `=` if the option takes an argument.\n \n-\t* Use `?` to mean that the option is optional (though its use is discouraged).\n+\t* Use `?` to mean that the option takes an optional argument. You\n+\t  probably want to use the `--sticked-long` mode to be able to\n+\t  unambiguously parse the optional argument.\n \n \t* Use `*` to mean that this option should not be listed in the usage\n \t  generated for the `-h` argument. It's shown for `--help-all` as\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex c76b89d..418b7f7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -30,6 +30,8 @@ static int abbrev_ref;\n static int abbrev_ref_strict;\n static int output_sq;\n \n+static int sticked_long;\n+\n /*\n  * Some arguments are relevant \"revision\" arguments,\n  * others are about output format or other details.\n@@ -320,12 +322,15 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)\n \tstruct strbuf *parsed = o->value;\n \tif (unset)\n \t\tstrbuf_addf(parsed, \" --no-%s\", o->long_name);\n-\telse if (o->short_name)\n+\telse if (o->short_name && (o->long_name == NULL || !sticked_long))\n \t\tstrbuf_addf(parsed, \" -%c\", o->short_name);\n \telse\n \t\tstrbuf_addf(parsed, \" --%s\", o->long_name);\n \tif (arg) {\n-\t\tstrbuf_addch(parsed, ' ');\n+\t\tif (!sticked_long)\n+\t\t\tstrbuf_addch(parsed, ' ');\n+\t\telse if (o->long_name)\n+\t\t\tstrbuf_addch(parsed, '=');\n \t\tsq_quote_buf(parsed, arg);\n \t}\n \treturn 0;\n@@ -351,6 +356,8 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"stop-at-non-option\", &stop_at_non_option,\n \t\t\t\t\tN_(\"stop parsing after the \"\n \t\t\t\t\t   \"first non-option argument\")),\n+\t\tOPT_BOOL(0, \"sticked-long\", &sticked_long,\n+\t\t\t\t\tN_(\"output in sticked long form\")),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh\nindex 13c88c9..7e12d9b 100755\n--- a/t/t1502-rev-parse-parseopt.sh\n+++ b/t/t1502-rev-parse-parseopt.sh\n@@ -12,9 +12,11 @@ usage: some-command [options] <args>...\n     -h, --help            show the help\n     --foo                 some nifty option --foo\n     --bar ...             some cool option --bar with an argument\n+    -b, --baz             a short and long option\n \n An option group Header\n     -C[...]               option C with an optional argument\n+    -d, --data[=...]      short and long option with an optional argument\n \n Extras\n     --extra1              line above used to cause a segfault but no longer does\n@@ -31,9 +33,11 @@ h,help    show the help\n \n foo       some nifty option --foo\n bar=      some cool option --bar with an argument\n+b,baz     a short and long option\n \n  An option group Header\n C?        option C with an optional argument\n+d,data?   short and long option with an optional argument\n \n Extras\n extra1    line above used to cause a segfault but no longer does\n@@ -45,16 +49,16 @@ test_expect_success 'test --parseopt help output' '\n '\n \n cat > expect <<EOF\n-set -- --foo --bar 'ham' -- 'arg'\n+set -- --foo --bar 'ham' -b -- 'arg'\n EOF\n \n test_expect_success 'test --parseopt' '\n-\tgit rev-parse --parseopt -- --foo --bar=ham arg < optionspec > output &&\n+\tgit rev-parse --parseopt -- --foo --bar=ham --baz arg < optionspec > output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'test --parseopt with mixed options and arguments' '\n-\tgit rev-parse --parseopt -- --foo arg --bar=ham < optionspec > output &&\n+\tgit rev-parse --parseopt -- --foo arg --bar=ham --baz < optionspec > output &&\n \ttest_cmp expect output\n '\n \n@@ -99,4 +103,36 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou\n \ttest_cmp expect output\n '\n \n+cat > expect <<EOF\n+set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --sticked-long' '\n+\tgit rev-parse --parseopt --sticked-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <<EOF\n+set -- --data='' -C --baz -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --sticked-long and empty optional argument' '\n+\tgit rev-parse --parseopt --sticked-long -- --data= arg -C -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <<EOF\n+set -- --data --baz -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --sticked-long and long option with unset optional argument' '\n+\tgit rev-parse --parseopt --sticked-long -- --data arg -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'test --parseopt --sticked-long and short option with unset optional argument' '\n+\tgit rev-parse --parseopt --sticked-long -- -d arg -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n test_done\n-- \n1.8.4\n"},{"id":"229530","messageId":"xmqq1u39j9hw.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"1382732291-5701-1-git-send-email-boklm@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T22:01:15Z","receivedAt":"2013-10-25T22:01:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Vigier <boklm@mars-attacks.org> writes:\n\n> Add the --sticked-long option to output the options in their long form\n> if available, and with their arguments sticked.\n\nHmph, doesn't verb \"stick\" conjugate to \"(present) stick (past) stuck\n(pp) stuck\"?\n"},{"id":"229534","messageId":"20131025225222.GL4589@mars-attacks.org","threadId":"35144","inReplyTo":"xmqq1u39j9hw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-25T22:52:23Z","receivedAt":"2013-10-25T22:52:23Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"On Fri, 25 Oct 2013, Junio C Hamano wrote:\n\n> Nicolas Vigier <boklm@mars-attacks.org> writes:\n> \n> > Add the --sticked-long option to output the options in their long form\n> > if available, and with their arguments sticked.\n> \n> Hmph, doesn't verb \"stick\" conjugate to \"(present) stick (past) stuck\n> (pp) stuck\"?\n\nAh, yes it seems. I don't know if 'sticked' is also correct or not.\n\nHowever, 'sticked' is the word that is used in Documentation/gitcli.txt\nand Documentation/technical/api-parse-options.txt.\n"},{"id":"229535","messageId":"xmqqsivphsel.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"20131025225222.GL4589@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T22:55:46Z","receivedAt":"2013-10-25T22:55:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Vigier <boklm@mars-attacks.org> writes:\n\n> On Fri, 25 Oct 2013, Junio C Hamano wrote:\n>\n>> Nicolas Vigier <boklm@mars-attacks.org> writes:\n>> \n>> > Add the --sticked-long option to output the options in their long form\n>> > if available, and with their arguments sticked.\n>> \n>> Hmph, doesn't verb \"stick\" conjugate to \"(present) stick (past) stuck\n>> (pp) stuck\"?\n>\n> Ah, yes it seems. I don't know if 'sticked' is also correct or not.\n>\n> However, 'sticked' is the word that is used in Documentation/gitcli.txt\n> and Documentation/technical/api-parse-options.txt.\n\nYes, I know. That is why I brought it up before we inflict more\ndamage to us.\n"},{"id":"229577","messageId":"7B64920A599843F9A122740DFA7A402A@PhilipOakley","threadId":"35144","inReplyTo":"xmqqsivphsel.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-10-26T21:55:10Z","receivedAt":"2013-10-26T21:55:10Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> Nicolas Vigier <boklm@mars-attacks.org> writes:\n>\n>> On Fri, 25 Oct 2013, Junio C Hamano wrote:\n>>\n>>> Nicolas Vigier <boklm@mars-attacks.org> writes:\n>>>\n>>> > Add the --sticked-long option to output the options in their long \n>>> > form\n>>> > if available, and with their arguments sticked.\n>>>\n>>> Hmph, doesn't verb \"stick\" conjugate to \"(present) stick (past) \n>>> stuck\n>>> (pp) stuck\"?\n>>\n>> Ah, yes it seems. I don't know if 'sticked' is also correct or not.\n>>\n>> However, 'sticked' is the word that is used in \n>> Documentation/gitcli.txt\n>> and Documentation/technical/api-parse-options.txt.\n>\n> Yes, I know. That is why I brought it up before we inflict more\n> damage to us.\n> --\n\nIsn't the origin of the description that it looks like a stick (cane), \nand 'sticked' is a modern verbing of that form? That's what I'd assumed \nanyway.\n\nGoogleing \"Sticked option\" only linked back to Git.\n\nPhilip \n"},{"id":"229586","messageId":"526CA88E.90802@alum.mit.edu","threadId":"35144","inReplyTo":"1382732291-5701-1-git-send-email-boklm@mars-attacks.org","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-10-27T05:45:50Z","receivedAt":"2013-10-27T05:45:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/25/2013 10:18 PM, Nicolas Vigier wrote:\n> Add the --sticked-long option to output the options in their long form\n> if available, and with their arguments sticked.\n> \n> Contrary to the default form (non sticked arguments and short options),\n> this can be parsed unambiguously when using options with optional\n> arguments :\n> \n>  - in the non sticked form, when an option is taking an optional argument\n>    you cannot know if the next argument is its optional argument, or the\n>    next option.\n> \n>  - the long options form allows to differenciate between an empty argument\n>    '--option=' and an unset argument '--option', which is not possible\n>    with short options.\n\nTrivial: s/differenciate/differentiate/\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"229644","messageId":"xmqqa9htgzx8.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"7B64920A599843F9A122740DFA7A402A@PhilipOakley","subject":"Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-28T15:47:47Z","receivedAt":"2013-10-28T15:47:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> Isn't the origin of the description that it looks like a stick (cane),\n> and 'sticked' is a modern verbing of that form? That's what I'd\n> assumed anyway.\n>\n> Googleing \"Sticked option\" only linked back to Git.\n\nI know web is not the authoritative source of information ;-) but it\nseems that looking for \"sticked vs stuck\" seems to give me many\nexplanations that boils down to what this says:\n\n    http://en.wiktionary.org/wiki/Talk:sticked\n\nand http://en.wiktionary.org/wiki/stick#English lists \"sticked\" as\n\"archaic\" when the word is used for \"To glue; to attach; to adhere\".\n"},{"id":"229912","messageId":"1383217709-5518-1-git-send-email-boklm@mars-attacks.org","threadId":"35144","inReplyTo":"xmqq1u39j9hw.fsf@gitster.dls.corp.google.com","subject":"sticked -> stuck","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-31T11:08:27Z","receivedAt":"2013-10-31T11:08:27Z","isPatch":false,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"Here is a patch to replace the word 'sticked' with 'stuck' in existing\ndocumentation. And the patch for nv/parseopt-opt-arg changed to use the\nword 'stuck' too.\n"},{"id":"229913","messageId":"1383217709-5518-2-git-send-email-boklm@mars-attacks.org","threadId":"35144","inReplyTo":"1383217709-5518-1-git-send-email-boklm@mars-attacks.org","subject":"[PATCH 1/2] Use the word 'stuck' instead of 'sticked'","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-31T11:08:28Z","receivedAt":"2013-10-31T11:08:28Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"The past participle of 'stick' is 'stuck'.\n\nSigned-off-by: Nicolas Vigier <boklm@mars-attacks.org>\n---\n Documentation/gitcli.txt                      | 6 +++---\n Documentation/technical/api-parse-options.txt | 6 +++---\n diff.c                                        | 2 +-\n diff.h                                        | 2 +-\n 4 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt\nindex 3146413cce0d..c87f0ae88d6c 100644\n--- a/Documentation/gitcli.txt\n+++ b/Documentation/gitcli.txt\n@@ -72,11 +72,11 @@ scripting Git:\n  * splitting short options to separate words (prefer `git foo -a -b`\n    to `git foo -ab`, the latter may not even work).\n \n- * when a command line option takes an argument, use the 'sticked' form.  In\n+ * when a command line option takes an argument, use the 'stuck' form.  In\n    other words, write `git foo -oArg` instead of `git foo -o Arg` for short\n    options, and `git foo --long-opt=Arg` instead of `git foo --long-opt Arg`\n    for long options.  An option that takes optional option-argument must be\n-   written in the 'sticked' form.\n+   written in the 'stuck' form.\n \n  * when you give a revision parameter to a command, make sure the parameter is\n    not ambiguous with a name of a file in the work tree.  E.g. do not write\n@@ -165,7 +165,7 @@ $ git foo -o Arg\n ----------------------------\n \n However, this is *NOT* allowed for switches with an optional value, where the\n-'sticked' form must be used:\n+'stuck' form must be used:\n ----------------------------\n $ git describe --abbrev HEAD     # correct\n $ git describe --abbrev=10 HEAD  # correct\ndiff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\nindex 0be2b5159f1b..be50cf4de35c 100644\n--- a/Documentation/technical/api-parse-options.txt\n+++ b/Documentation/technical/api-parse-options.txt\n@@ -29,9 +29,9 @@ that allow to change the behavior of a command.\n \n The parse-options API allows:\n \n-* 'sticked' and 'separate form' of options with arguments.\n-  `-oArg` is sticked, `-o Arg` is separate form.\n-  `--option=Arg` is sticked, `--option Arg` is separate form.\n+* 'stuck' and 'separate form' of options with arguments.\n+  `-oArg` is stuck, `-o Arg` is separate form.\n+  `--option=Arg` is stuck, `--option Arg` is separate form.\n \n * Long options may be 'abbreviated', as long as the abbreviation\n   is unambiguous.\ndiff --git a/diff.c b/diff.c\nindex e34bf971207f..3950e0191067 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3394,7 +3394,7 @@ int parse_long_opt(const char *opt, const char **argv,\n \tif (prefixcmp(arg, opt))\n \t\treturn 0;\n \targ += strlen(opt);\n-\tif (*arg == '=') { /* sticked form: --option=value */\n+\tif (*arg == '=') { /* stuck form: --option=value */\n \t\t*optarg = arg + 1;\n \t\treturn 1;\n \t}\ndiff --git a/diff.h b/diff.h\nindex e34232501ee8..1c05b3a6bec6 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -244,7 +244,7 @@ extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *pat\n #define DIFF_SETUP_USE_SIZE_CACHE\t4\n \n /*\n- * Poor man's alternative to parse-option, to allow both sticked form\n+ * Poor man's alternative to parse-option, to allow both stuck form\n  * (--option=value) and separate form (--option value).\n  */\n extern int parse_long_opt(const char *opt, const char **argv,\n-- \n1.8.4\n"},{"id":"229914","messageId":"1383217709-5518-3-git-send-email-boklm@mars-attacks.org","threadId":"35144","inReplyTo":"1383217709-5518-1-git-send-email-boklm@mars-attacks.org","subject":"[PATCH 2/2] rev-parse --parseopt: add the --stuck-long mode","fromName":"Nicolas Vigier","fromEmail":"boklm@mars-attacks.org","sentAt":"2013-10-31T11:08:29Z","receivedAt":"2013-10-31T11:08:29Z","isPatch":true,"sender":{"key":"boklm@mars-attacks.org","avatar":"https://avatars.githubusercontent.com/u/405910?v=4"},"body":"Add the --stuck-long option to output the options in their long form\nif available, and with their arguments stuck.\n\nContrary to the default form (non stuck arguments and short options),\nthis can be parsed unambiguously when using options with optional\narguments :\n\n - in the non stuck form, when an option is taking an optional argument\n   you cannot know if the next argument is its optional argument, or the\n   next option.\n\n - the long options form allows to differentiate between an empty argument\n   '--option=' and an unset argument '--option', which is not possible\n   with short options.\n\nSigned-off-by: Nicolas Vigier <boklm@mars-attacks.org>\n---\n Documentation/git-rev-parse.txt |  8 +++++++-\n builtin/rev-parse.c             | 11 +++++++++--\n t/t1502-rev-parse-parseopt.sh   | 42 ++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 55 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex d068a653778d..a436b24cc406 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -50,6 +50,10 @@ Options for --parseopt\n \tthe first non-option argument.  This can be used to parse sub-commands\n \tthat take options themselves.\n \n+--stuck-long::\n+\tOnly meaningful in `--parseopt` mode. Output the options in their\n+\tlong form if available, and with their arguments stuck.\n+\n Options for Filtering\n ~~~~~~~~~~~~~~~~~~~~~\n \n@@ -285,7 +289,9 @@ Each line of options has this format:\n \t`<flags>` are of `*`, `=`, `?` or `!`.\n \t* Use `=` if the option takes an argument.\n \n-\t* Use `?` to mean that the option is optional (though its use is discouraged).\n+\t* Use `?` to mean that the option takes an optional argument. You\n+\t  probably want to use the `--stuck-long` mode to be able to\n+\t  unambiguously parse the optional argument.\n \n \t* Use `*` to mean that this option should not be listed in the usage\n \t  generated for the `-h` argument. It's shown for `--help-all` as\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex c76b89dc5bcc..3e8c4cce060e 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -30,6 +30,8 @@ static int abbrev_ref;\n static int abbrev_ref_strict;\n static int output_sq;\n \n+static int stuck_long;\n+\n /*\n  * Some arguments are relevant \"revision\" arguments,\n  * others are about output format or other details.\n@@ -320,12 +322,15 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)\n \tstruct strbuf *parsed = o->value;\n \tif (unset)\n \t\tstrbuf_addf(parsed, \" --no-%s\", o->long_name);\n-\telse if (o->short_name)\n+\telse if (o->short_name && (o->long_name == NULL || !stuck_long))\n \t\tstrbuf_addf(parsed, \" -%c\", o->short_name);\n \telse\n \t\tstrbuf_addf(parsed, \" --%s\", o->long_name);\n \tif (arg) {\n-\t\tstrbuf_addch(parsed, ' ');\n+\t\tif (!stuck_long)\n+\t\t\tstrbuf_addch(parsed, ' ');\n+\t\telse if (o->long_name)\n+\t\t\tstrbuf_addch(parsed, '=');\n \t\tsq_quote_buf(parsed, arg);\n \t}\n \treturn 0;\n@@ -351,6 +356,8 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"stop-at-non-option\", &stop_at_non_option,\n \t\t\t\t\tN_(\"stop parsing after the \"\n \t\t\t\t\t   \"first non-option argument\")),\n+\t\tOPT_BOOL(0, \"stuck-long\", &stuck_long,\n+\t\t\t\t\tN_(\"output in stuck long form\")),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh\nindex 13c88c9aae7f..83b1300cef91 100755\n--- a/t/t1502-rev-parse-parseopt.sh\n+++ b/t/t1502-rev-parse-parseopt.sh\n@@ -12,9 +12,11 @@ usage: some-command [options] <args>...\n     -h, --help            show the help\n     --foo                 some nifty option --foo\n     --bar ...             some cool option --bar with an argument\n+    -b, --baz             a short and long option\n \n An option group Header\n     -C[...]               option C with an optional argument\n+    -d, --data[=...]      short and long option with an optional argument\n \n Extras\n     --extra1              line above used to cause a segfault but no longer does\n@@ -31,9 +33,11 @@ h,help    show the help\n \n foo       some nifty option --foo\n bar=      some cool option --bar with an argument\n+b,baz     a short and long option\n \n  An option group Header\n C?        option C with an optional argument\n+d,data?   short and long option with an optional argument\n \n Extras\n extra1    line above used to cause a segfault but no longer does\n@@ -45,16 +49,16 @@ test_expect_success 'test --parseopt help output' '\n '\n \n cat > expect <<EOF\n-set -- --foo --bar 'ham' -- 'arg'\n+set -- --foo --bar 'ham' -b -- 'arg'\n EOF\n \n test_expect_success 'test --parseopt' '\n-\tgit rev-parse --parseopt -- --foo --bar=ham arg < optionspec > output &&\n+\tgit rev-parse --parseopt -- --foo --bar=ham --baz arg < optionspec > output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'test --parseopt with mixed options and arguments' '\n-\tgit rev-parse --parseopt -- --foo arg --bar=ham < optionspec > output &&\n+\tgit rev-parse --parseopt -- --foo arg --bar=ham --baz < optionspec > output &&\n \ttest_cmp expect output\n '\n \n@@ -99,4 +103,36 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou\n \ttest_cmp expect output\n '\n \n+cat > expect <<EOF\n+set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --stuck-long' '\n+\tgit rev-parse --parseopt --stuck-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <<EOF\n+set -- --data='' -C --baz -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --stuck-long and empty optional argument' '\n+\tgit rev-parse --parseopt --stuck-long -- --data= arg -C -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect <<EOF\n+set -- --data --baz -- 'arg'\n+EOF\n+\n+test_expect_success 'test --parseopt --stuck-long and long option with unset optional argument' '\n+\tgit rev-parse --parseopt --stuck-long -- --data arg -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'test --parseopt --stuck-long and short option with unset optional argument' '\n+\tgit rev-parse --parseopt --stuck-long -- -d arg -b <optionspec >output &&\n+\ttest_cmp expect output\n+'\n+\n test_done\n-- \n1.8.4\n"},{"id":"229959","messageId":"xmqqbo255j3u.fsf@gitster.dls.corp.google.com","threadId":"35144","inReplyTo":"1383217709-5518-2-git-send-email-boklm@mars-attacks.org","subject":"Re: [PATCH 1/2] Use the word 'stuck' instead of 'sticked'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T19:35:33Z","receivedAt":"2013-10-31T19:35:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Vigier <boklm@mars-attacks.org> writes:\n\n> The past participle of 'stick' is 'stuck'.\n>\n> Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>\n> ---\n\nThanks.\n\nIt was good that we caught this before introducing the option; the\ndocumentation update does not hurt the users much, but if we unleash\na misspelled option in a release, it is much harder to correct it\nlater.\n\nWill queue both patches.\n\n>  Documentation/gitcli.txt                      | 6 +++---\n>  Documentation/technical/api-parse-options.txt | 6 +++---\n>  diff.c                                        | 2 +-\n>  diff.h                                        | 2 +-\n>  4 files changed, 8 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt\n> index 3146413cce0d..c87f0ae88d6c 100644\n> --- a/Documentation/gitcli.txt\n> +++ b/Documentation/gitcli.txt\n> @@ -72,11 +72,11 @@ scripting Git:\n>   * splitting short options to separate words (prefer `git foo -a -b`\n>     to `git foo -ab`, the latter may not even work).\n>  \n> - * when a command line option takes an argument, use the 'sticked' form.  In\n> + * when a command line option takes an argument, use the 'stuck' form.  In\n>     other words, write `git foo -oArg` instead of `git foo -o Arg` for short\n>     options, and `git foo --long-opt=Arg` instead of `git foo --long-opt Arg`\n>     for long options.  An option that takes optional option-argument must be\n> -   written in the 'sticked' form.\n> +   written in the 'stuck' form.\n>  \n>   * when you give a revision parameter to a command, make sure the parameter is\n>     not ambiguous with a name of a file in the work tree.  E.g. do not write\n> @@ -165,7 +165,7 @@ $ git foo -o Arg\n>  ----------------------------\n>  \n>  However, this is *NOT* allowed for switches with an optional value, where the\n> -'sticked' form must be used:\n> +'stuck' form must be used:\n>  ----------------------------\n>  $ git describe --abbrev HEAD     # correct\n>  $ git describe --abbrev=10 HEAD  # correct\n> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\n> index 0be2b5159f1b..be50cf4de35c 100644\n> --- a/Documentation/technical/api-parse-options.txt\n> +++ b/Documentation/technical/api-parse-options.txt\n> @@ -29,9 +29,9 @@ that allow to change the behavior of a command.\n>  \n>  The parse-options API allows:\n>  \n> -* 'sticked' and 'separate form' of options with arguments.\n> -  `-oArg` is sticked, `-o Arg` is separate form.\n> -  `--option=Arg` is sticked, `--option Arg` is separate form.\n> +* 'stuck' and 'separate form' of options with arguments.\n> +  `-oArg` is stuck, `-o Arg` is separate form.\n> +  `--option=Arg` is stuck, `--option Arg` is separate form.\n>  \n>  * Long options may be 'abbreviated', as long as the abbreviation\n>    is unambiguous.\n> diff --git a/diff.c b/diff.c\n> index e34bf971207f..3950e0191067 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3394,7 +3394,7 @@ int parse_long_opt(const char *opt, const char **argv,\n>  \tif (prefixcmp(arg, opt))\n>  \t\treturn 0;\n>  \targ += strlen(opt);\n> -\tif (*arg == '=') { /* sticked form: --option=value */\n> +\tif (*arg == '=') { /* stuck form: --option=value */\n>  \t\t*optarg = arg + 1;\n>  \t\treturn 1;\n>  \t}\n> diff --git a/diff.h b/diff.h\n> index e34232501ee8..1c05b3a6bec6 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -244,7 +244,7 @@ extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *pat\n>  #define DIFF_SETUP_USE_SIZE_CACHE\t4\n>  \n>  /*\n> - * Poor man's alternative to parse-option, to allow both sticked form\n> + * Poor man's alternative to parse-option, to allow both stuck form\n>   * (--option=value) and separate form (--option value).\n>   */\n>  extern int parse_long_opt(const char *opt, const char **argv,\n"}]}