{"thread":{"id":"40571","subject":"[PATCH] pull: add angle brackets to usage string","startedAt":"2015-10-16T02:22:13Z","lastAt":"2015-10-20T18:18:19Z","messageCount":10,"participants":["Alex Henrie","Junio C Hamano","Ralf Thielow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"271794","messageId":"1444962133-1266-1-git-send-email-alexhenrie24@gmail.com","threadId":"40571","inReplyTo":null,"subject":"[PATCH] pull: add angle brackets to usage string","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2015-10-16T02:22:13Z","receivedAt":"2015-10-16T02:22:13Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\n builtin/pull.c               | 2 +-\n contrib/examples/git-pull.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex a39bb0a..bf3fd3f 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset\n }\n \n static const char * const pull_usage[] = {\n-\tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n+\tN_(\"git pull [<options>] [<repository> [<refspec>...]]\"),\n \tNULL\n };\n \ndiff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh\nindex 6b3a03f..bcf362e 100755\n--- a/contrib/examples/git-pull.sh\n+++ b/contrib/examples/git-pull.sh\n@@ -8,7 +8,7 @@ SUBDIRECTORY_OK=Yes\n OPTIONS_KEEPDASHDASH=\n OPTIONS_STUCKLONG=Yes\n OPTIONS_SPEC=\"\\\n-git pull [options] [<repository> [<refspec>...]]\n+git pull [<options>] [<repository> [<refspec>...]]\n \n Fetch one or more remote refs and integrate it/them with the current HEAD.\n --\n-- \n2.6.1\n"},{"id":"271816","messageId":"xmqqwpumg480.fsf@gitster.mtv.corp.google.com","threadId":"40571","inReplyTo":"1444962133-1266-1-git-send-email-alexhenrie24@gmail.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T16:36:47Z","receivedAt":"2015-10-16T16:36:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n> ---\n\nMakes sense, as all the other <placeholders> in the usage string are\nbracketted.\n\nDoes it make sense to do this for contrib/examples, which is the\nhistorical record, though?  The first one I found with\n\n    $ less contrib/examples/*\n\nwas this:\n\n    #!/bin/sh\n\n    OPTIONS_KEEPDASHDASH=t\n    OPTIONS_SPEC=\"\\\n    git-checkout [options] [<branch>] [<paths>...]\n\nand the next one (clean) follows the same pattern.\n\nI'd discard the part of the patch for contrib/ and queue.\n\nThanks.\n\n>  builtin/pull.c               | 2 +-\n>  contrib/examples/git-pull.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index a39bb0a..bf3fd3f 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset\n>  }\n>  \n>  static const char * const pull_usage[] = {\n> -\tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n> +\tN_(\"git pull [<options>] [<repository> [<refspec>...]]\"),\n>  \tNULL\n>  };\n>  \n> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh\n> index 6b3a03f..bcf362e 100755\n> --- a/contrib/examples/git-pull.sh\n> +++ b/contrib/examples/git-pull.sh\n> @@ -8,7 +8,7 @@ SUBDIRECTORY_OK=Yes\n>  OPTIONS_KEEPDASHDASH=\n>  OPTIONS_STUCKLONG=Yes\n>  OPTIONS_SPEC=\"\\\n> -git pull [options] [<repository> [<refspec>...]]\n> +git pull [<options>] [<repository> [<refspec>...]]\n>  \n>  Fetch one or more remote refs and integrate it/them with the current HEAD.\n>  --\n"},{"id":"271818","messageId":"CAMMLpeQPngq-xM2vN4uX6+k5-uoOXyP3p0wgLO9LOkWxWtZBEA@mail.gmail.com","threadId":"40571","inReplyTo":"xmqqwpumg480.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2015-10-16T16:42:45Z","receivedAt":"2015-10-16T16:42:45Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"2015-10-16 10:36 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n> Makes sense, as all the other <placeholders> in the usage string are\n> bracketted.\n>\n> Does it make sense to do this for contrib/examples, which is the\n> historical record, though?\n\nI didn't know that contrib/examples was a historical record. The last\npatch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also\nmodified contrib/examples.\n\n-Alex\n"},{"id":"271822","messageId":"20151016172455.GA18751@ralf-ubuntu","threadId":"40571","inReplyTo":"1444962133-1266-1-git-send-email-alexhenrie24@gmail.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2015-10-16T17:24:55Z","receivedAt":"2015-10-16T17:24:55Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> wrote:\n> ---\n>  builtin/pull.c               | 2 +-\n>  contrib/examples/git-pull.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index a39bb0a..bf3fd3f 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset\n>  }\n>  \n>  static const char * const pull_usage[] = {\n> -\tN_(\"git pull [options] [<repository> [<refspec>...]]\"),\n> +\tN_(\"git pull [<options>] [<repository> [<refspec>...]]\"),\n>  \tNULL\n>  };\n\n\nThere seem to be three more places left missing these angle brances\nat the usage string.\nJunio, feel free to squash this or treat it as a separate patch\non top, if suitable.\n\n-- >8 --\nFrom: Ralf Thielow <ralf.thielow@gmail.com>\nDate: Fri, 16 Oct 2015 19:09:57 +0200\nSubject: [PATCH] am, credential-cache: add angle brackets to usage string\n\nSigned-off-by: Ralf Thielow <ralf.thielow@gmail.com>\n---\n builtin/am.c       | 4 ++--\n credential-cache.c | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 4f77e07..98992cd 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -2218,8 +2218,8 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \tint in_progress;\n \n \tconst char * const usage[] = {\n-\t\tN_(\"git am [options] [(<mbox>|<Maildir>)...]\"),\n-\t\tN_(\"git am [options] (--continue | --skip | --abort)\"),\n+\t\tN_(\"git am [<options>] [(<mbox>|<Maildir>)...]\"),\n+\t\tN_(\"git am [<options>] (--continue | --skip | --abort)\"),\n \t\tNULL\n \t};\n \ndiff --git a/credential-cache.c b/credential-cache.c\nindex 8689a15..f4afdc6 100644\n--- a/credential-cache.c\n+++ b/credential-cache.c\n@@ -88,7 +88,7 @@ int main(int argc, const char **argv)\n \tint timeout = 900;\n \tconst char *op;\n \tconst char * const usage[] = {\n-\t\t\"git credential-cache [options] <action>\",\n+\t\t\"git credential-cache [<options>] <action>\",\n \t\tNULL\n \t};\n \tstruct option options[] = {\n-- \n2.6.1.339.g81d1034\n"},{"id":"271826","messageId":"xmqq4mhqg15y.fsf@gitster.mtv.corp.google.com","threadId":"40571","inReplyTo":"CAMMLpeQPngq-xM2vN4uX6+k5-uoOXyP3p0wgLO9LOkWxWtZBEA@mail.gmail.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T17:42:49Z","receivedAt":"2015-10-16T17:42:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> 2015-10-16 10:36 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n>> Makes sense, as all the other <placeholders> in the usage string are\n>> bracketted.\n>>\n>> Does it make sense to do this for contrib/examples, which is the\n>> historical record, though?\n>\n> I didn't know that contrib/examples was a historical record. The last\n> patch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also\n> modified contrib/examples.\n\nYes, but that fixes historical \"mistake\", no?\n\nWith this, you are breaking historical practice by changing only one\ninstance to deviate from the then-current practice of saying\n'options' without brackets.  It is based on the point of view that\nconsiders anything inside <bracket> and a fixed string 'options' are\nmeant to be replaced by intelligent readers, which is as valid as\nthe more recent practice to consider only things inside <bracket>\nare placeholders.\n"},{"id":"271827","messageId":"xmqqziziemfx.fsf@gitster.mtv.corp.google.com","threadId":"40571","inReplyTo":"20151016172455.GA18751@ralf-ubuntu","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T17:46:10Z","receivedAt":"2015-10-16T17:46:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow <ralf.thielow@gmail.com> writes:\n\n> There seem to be three more places left missing these angle brances\n> at the usage string.\n> Junio, feel free to squash this or treat it as a separate patch\n> on top, if suitable.\n\nThanks. Will queue.\n\n>\n> -- >8 --\n> From: Ralf Thielow <ralf.thielow@gmail.com>\n> Date: Fri, 16 Oct 2015 19:09:57 +0200\n> Subject: [PATCH] am, credential-cache: add angle brackets to usage string\n>\n> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>\n> ---\n>  builtin/am.c       | 4 ++--\n>  credential-cache.c | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/am.c b/builtin/am.c\n> index 4f77e07..98992cd 100644\n> --- a/builtin/am.c\n> +++ b/builtin/am.c\n> @@ -2218,8 +2218,8 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n>  \tint in_progress;\n>  \n>  \tconst char * const usage[] = {\n> -\t\tN_(\"git am [options] [(<mbox>|<Maildir>)...]\"),\n> -\t\tN_(\"git am [options] (--continue | --skip | --abort)\"),\n> +\t\tN_(\"git am [<options>] [(<mbox>|<Maildir>)...]\"),\n> +\t\tN_(\"git am [<options>] (--continue | --skip | --abort)\"),\n>  \t\tNULL\n>  \t};\n>  \n> diff --git a/credential-cache.c b/credential-cache.c\n> index 8689a15..f4afdc6 100644\n> --- a/credential-cache.c\n> +++ b/credential-cache.c\n> @@ -88,7 +88,7 @@ int main(int argc, const char **argv)\n>  \tint timeout = 900;\n>  \tconst char *op;\n>  \tconst char * const usage[] = {\n> -\t\t\"git credential-cache [options] <action>\",\n> +\t\t\"git credential-cache [<options>] <action>\",\n>  \t\tNULL\n>  \t};\n>  \tstruct option options[] = {\n"},{"id":"271937","messageId":"CAMMLpeTf93NnqPM+SaxfhsSdMg1HUiVyo5FFdiuGF9o-KQbGog@mail.gmail.com","threadId":"40571","inReplyTo":"xmqq4mhqg15y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2015-10-19T17:27:37Z","receivedAt":"2015-10-19T17:27:37Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"2015-10-16 11:42 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n> Alex Henrie <alexhenrie24@gmail.com> writes:\n>\n>> 2015-10-16 10:36 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n>>> Makes sense, as all the other <placeholders> in the usage string are\n>>> bracketted.\n>>>\n>>> Does it make sense to do this for contrib/examples, which is the\n>>> historical record, though?\n>>\n>> I didn't know that contrib/examples was a historical record. The last\n>> patch I submitted, b7447679e84ed973430ab19fce87f56857b83068, also\n>> modified contrib/examples.\n>\n> Yes, but that fixes historical \"mistake\", no?\n>\n> With this, you are breaking historical practice by changing only one\n> instance to deviate from the then-current practice of saying\n> 'options' without brackets.  It is based on the point of view that\n> considers anything inside <bracket> and a fixed string 'options' are\n> meant to be replaced by intelligent readers, which is as valid as\n> the more recent practice to consider only things inside <bracket>\n> are placeholders.\n\nOK, I see. You're saying that it's OK to fix typos and grammatical\nerrors in contrib/examples, but it's not okay to modernize the\nscripts' designs. That's fine; standardizing placeholder syntax is\nprimarily for the benefit of translators, and contrib/ is not\ntranslated, so it's not causing a problem.\n\n-Alex\n"},{"id":"271959","messageId":"xmqqbnbu9kzr.fsf@gitster.mtv.corp.google.com","threadId":"40571","inReplyTo":"CAMMLpeTf93NnqPM+SaxfhsSdMg1HUiVyo5FFdiuGF9o-KQbGog@mail.gmail.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-20T05:17:44Z","receivedAt":"2015-10-20T05:17:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> 2015-10-16 11:42 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n>>\n>> Yes, but that fixes historical \"mistake\", no?\n>>\n>> With this, you are breaking historical practice by changing only one\n>> instance to deviate from the then-current practice of saying\n>> 'options' without brackets.  It is based on the point of view that\n>> considers anything inside <bracket> and a fixed string 'options' are\n>> meant to be replaced by intelligent readers, which is as valid as\n>> the more recent practice to consider only things inside <bracket>\n>> are placeholders.\n>\n> OK, I see. You're saying that it's OK to fix typos and grammatical\n> errors in contrib/examples, but it's not okay to modernize the\n> scripts' designs.\n\nPlease read it again, look at contrib/examples and realize that that\nis not what I said at all.\n\nThis is not about modern vs old-school.  The reason why the part of\nthe patch to contrib/ under discussion is wrong is because of\n(in)consistency.\n\nLook at the output from \"git grep option contrib/examples/\" and\nnotice that in the old days, these scripted Porcelains consistently\nsaid \"[options]\" without \"<bracket>\".\n\nIt would have been a different matter if the patch _were_ to update\nall \"[options]\" to \"[<options>]\" in contrib/examples/ consistently,\nand such a patch might have even been an improvement, especially if\nthe modern style were clearly superiour than the old-school style\n(which is not, by they way [*1*]).\n\nBut that is not what the patch did.  It turned only one of them into\n\"[<options>]\", making the single instance inconsistent from all the\nothers around it.  That is why it was wrong.\n\n\n[Footnote]\n\n*1* The \"modern\" style is not necessarily an improvement, by the\n    way.  The way we specify that a \"thing\" in the help text is a\n    placeholder and that there may be more instances of the same\n    \"thing\" is to say \"[<thing>...]\", but in your \"modernized\" form,\n    unlike all the other usual \"things\", possibly multiple options\n    are spelled \"[<options>]\" without having ellipses at the end,\n    which is an oddball.  If we are to treat options specially like\n    that anyway, intelligent readers can read an \"old-school\"\n    description \"[options]\" and understand that that token stands\n    for possibly multiple options just fine, and all we have gained\n    by going to the \"modernized\" form is to waste two characters for\n    <brackets>.\n\n    I am not saying that we should not apply the other half of the\n    patch that makes builtin/pull.c say \"[<options>]\".  These days,\n    many other commands nearby (i.e. the \"modern\" ones) do use that\n    form consistently, so it is an improvement.\n"},{"id":"271980","messageId":"CAMMLpeStjJY2sqGvC0wV_OqFSA3OuzmcDitvr53d=z8+y2dBYA@mail.gmail.com","threadId":"40571","inReplyTo":"xmqqbnbu9kzr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2015-10-20T16:54:20Z","receivedAt":"2015-10-20T16:54:20Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"2015-10-19 23:17 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n> Alex Henrie <alexhenrie24@gmail.com> writes:\n>\n>> 2015-10-16 11:42 GMT-06:00 Junio C Hamano <gitster@pobox.com>:\n>>>\n>>> Yes, but that fixes historical \"mistake\", no?\n>>>\n>>> With this, you are breaking historical practice by changing only one\n>>> instance to deviate from the then-current practice of saying\n>>> 'options' without brackets.  It is based on the point of view that\n>>> considers anything inside <bracket> and a fixed string 'options' are\n>>> meant to be replaced by intelligent readers, which is as valid as\n>>> the more recent practice to consider only things inside <bracket>\n>>> are placeholders.\n>>\n>> OK, I see. You're saying that it's OK to fix typos and grammatical\n>> errors in contrib/examples, but it's not okay to modernize the\n>> scripts' designs.\n>\n> Please read it again, look at contrib/examples and realize that that\n> is not what I said at all.\n>\n> This is not about modern vs old-school.  The reason why the part of\n> the patch to contrib/ under discussion is wrong is because of\n> (in)consistency.\n>\n> Look at the output from \"git grep option contrib/examples/\" and\n> notice that in the old days, these scripted Porcelains consistently\n> said \"[options]\" without \"<bracket>\".\n>\n> It would have been a different matter if the patch _were_ to update\n> all \"[options]\" to \"[<options>]\" in contrib/examples/ consistently,\n> and such a patch might have even been an improvement, especially if\n> the modern style were clearly superiour than the old-school style\n> (which is not, by they way [*1*]).\n>\n> But that is not what the patch did.  It turned only one of them into\n> \"[<options>]\", making the single instance inconsistent from all the\n> others around it.  That is why it was wrong.\n\nI understand now, thanks. I really appreciate your commitment to being\nconsistent.\n\n> [Footnote]\n>\n> *1* The \"modern\" style is not necessarily an improvement, by the\n>     way.  The way we specify that a \"thing\" in the help text is a\n>     placeholder and that there may be more instances of the same\n>     \"thing\" is to say \"[<thing>...]\", but in your \"modernized\" form,\n>     unlike all the other usual \"things\", possibly multiple options\n>     are spelled \"[<options>]\" without having ellipses at the end,\n>     which is an oddball.  If we are to treat options specially like\n>     that anyway, intelligent readers can read an \"old-school\"\n>     description \"[options]\" and understand that that token stands\n>     for possibly multiple options just fine, and all we have gained\n>     by going to the \"modernized\" form is to waste two characters for\n>     <brackets>.\n>\n>     I am not saying that we should not apply the other half of the\n>     patch that makes builtin/pull.c say \"[<options>]\".  These days,\n>     many other commands nearby (i.e. the \"modern\" ones) do use that\n>     form consistently, so it is an improvement.\n\nI pushed to change [options] to [<options>] because even if the angle\nbrackets don't help new users or translators in this particular case,\nthe angle brackets encourage Git authors to use angle brackets when\nwriting commands that are not so easy to understand. If you think that\n[<option>...] is better because it is even more consistent, I would be\nhappy to send a patch to make that change.\n\nAnyway, thanks again for your attention to detail.\n\n-Alex\n"},{"id":"271985","messageId":"xmqqeggp8kus.fsf@gitster.mtv.corp.google.com","threadId":"40571","inReplyTo":"CAMMLpeStjJY2sqGvC0wV_OqFSA3OuzmcDitvr53d=z8+y2dBYA@mail.gmail.com","subject":"Re: [PATCH] pull: add angle brackets to usage string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-20T18:18:19Z","receivedAt":"2015-10-20T18:18:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> I pushed to change [options] to [<options>] because even if the angle\n> brackets don't help new users or translators in this particular case,\n> the angle brackets encourage Git authors to use angle brackets when\n> writing commands that are not so easy to understand. If you think that\n> [<option>...] is better because it is even more consistent, I would be\n> happy to send a patch to make that change.\n\nI do agree that [<option>...] would make things consistent between\noptions and non-option arguments.  But I also would expect that\nreasonably intelligent readers know that options are special, and\nthey would understand that [options] and [<options>] mean the same\nthing as [<option>...] anyway, so I do not feel too strongly either\nway myself (meaning: I wouldn't be motivated to prepare patches for\nit myself, I wouldn't jump up and down saying they are wrong and\nrevert them if such patches were applied by an interim maintainer\nwhile I am on vacation, and I would apply such patches myself only\nif they do not overly interfere with topics in flight while\nmerging).\n\nThanks.\n"}]}