{"thread":{"id":"44706","subject":"[PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","startedAt":"2016-12-14T15:10:44Z","lastAt":"2016-12-19T15:03:53Z","messageCount":8,"participants":["Jeff King","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"307781","messageId":"20161214151009.4wdzjb44f6aki5ug@sigill.intra.peff.net","threadId":"44706","inReplyTo":null,"subject":"[PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-14T15:10:10Z","receivedAt":"2016-12-14T15:10:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Programs may use usage_msg_opt() to print a brief message\nfollowed by the program usage, and then exit. The message\nisn't prefixed at all, though, so it doesn't match our usual\nerror output and is easy to overlook:\n\n    $ git clone 1 2 3\n    Too many arguments.\n\n    usage: git clone [<options>] [--] <repo> [<dir>]\n\n    -v, --verbose         be more verbose\n    -q, --quiet           be more quiet\n    --progress            force progress reporting\n    -n, --no-checkout     don't create a checkout\n    --bare                create a bare repository\n    [...and so on for another 31 lines...]\n\nIt looks especially bad when the message starts with an\noption, like:\n\n    $ git replace -e\n    -e needs exactly one argument\n\n    usage: git replace [-f] <object> <replacement>\n       or: git replace [-f] --edit <object>\n    [...etc...]\n\nLet's put our usual \"fatal:\" prefix in front of it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSome of the message in git-clone could stand to be rewritten to match\nour usual style, too (no capitals, no trailing period), but that's\nobviously out of scope for this patch. I don't think this change makes\nthem look any worse.\n\n parse-options.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 312a85dbd..4fbe924a5 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,\n \t\t   const char * const *usagestr,\n \t\t   const struct option *options)\n {\n-\tfprintf(stderr, \"%s\\n\\n\", msg);\n+\tfprintf(stderr, \"fatal: %s\\n\\n\", msg);\n \tusage_with_options(usagestr, options);\n }\n \n-- \n2.11.0.341.g202cd3142\n"},{"id":"307793","messageId":"xmqqr35axujg.fsf@gitster.mtv.corp.google.com","threadId":"44706","inReplyTo":"20161214151009.4wdzjb44f6aki5ug@sigill.intra.peff.net","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-14T17:57:55Z","receivedAt":"2016-12-14T17:58:53Z","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> Programs may use usage_msg_opt() to print a brief message\n> followed by the program usage, and then exit. The message\n> isn't prefixed at all, though, so it doesn't match our usual\n> error output and is easy to overlook:\n>\n>     $ git clone 1 2 3\n>     Too many arguments.\n>\n>     usage: git clone [<options>] [--] <repo> [<dir>]\n>\n>     -v, --verbose         be more verbose\n>     -q, --quiet           be more quiet\n>     --progress            force progress reporting\n>     -n, --no-checkout     don't create a checkout\n>     --bare                create a bare repository\n>     [...and so on for another 31 lines...]\n>\n> It looks especially bad when the message starts with an\n> option, like:\n>\n>     $ git replace -e\n>     -e needs exactly one argument\n>\n>     usage: git replace [-f] <object> <replacement>\n>        or: git replace [-f] --edit <object>\n>     [...etc...]\n>\n> Let's put our usual \"fatal:\" prefix in front of it.\n\nI briefly wondered if any caller uses this in a non-fatal situation,\nbut usage_with_options() always dies, so this looks like the right\nthing to do.  Thanks.\n\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> Some of the message in git-clone could stand to be rewritten to match\n> our usual style, too (no capitals, no trailing period), but that's\n> obviously out of scope for this patch. I don't think this change makes\n> them look any worse.\n>\n>  parse-options.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/parse-options.c b/parse-options.c\n> index 312a85dbd..4fbe924a5 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,\n>  \t\t   const char * const *usagestr,\n>  \t\t   const struct option *options)\n>  {\n> -\tfprintf(stderr, \"%s\\n\\n\", msg);\n> +\tfprintf(stderr, \"fatal: %s\\n\\n\", msg);\n>  \tusage_with_options(usagestr, options);\n>  }\n"},{"id":"308019","messageId":"20161219120719.GF24125@ash","threadId":"44706","inReplyTo":"20161214151009.4wdzjb44f6aki5ug@sigill.intra.peff.net","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-12-19T12:07:19Z","receivedAt":"2016-12-19T12:08:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 14, 2016 at 10:10:10AM -0500, Jeff King wrote:\n> diff --git a/parse-options.c b/parse-options.c\n> index 312a85dbd..4fbe924a5 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,\n>  \t\t   const char * const *usagestr,\n>  \t\t   const struct option *options)\n>  {\n> -\tfprintf(stderr, \"%s\\n\\n\", msg);\n> +\tfprintf(stderr, \"fatal: %s\\n\\n\", msg);\n\nYour commit message does not make clear if you want this \"fatal\" to be\ngrep-able (by scripts) or not. If not, please _() the string.  Maybe\nthis to reduce work for translators\n\n\t/* TRANSLATORS: this is the prefix before usage error */\n\tfprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n\n>  \tusage_with_options(usagestr, options);\n>  }\n>  \n> -- \n> 2.11.0.341.g202cd3142\n"},{"id":"308026","messageId":"20161219134148.vxa4fz3jw2i23lfm@sigill.intra.peff.net","threadId":"44706","inReplyTo":"20161219120719.GF24125@ash","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-19T13:41:48Z","receivedAt":"2016-12-19T13:42:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 19, 2016 at 07:07:19PM +0700, Duy Nguyen wrote:\n\n> On Wed, Dec 14, 2016 at 10:10:10AM -0500, Jeff King wrote:\n> > diff --git a/parse-options.c b/parse-options.c\n> > index 312a85dbd..4fbe924a5 100644\n> > --- a/parse-options.c\n> > +++ b/parse-options.c\n> > @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,\n> >  \t\t   const char * const *usagestr,\n> >  \t\t   const struct option *options)\n> >  {\n> > -\tfprintf(stderr, \"%s\\n\\n\", msg);\n> > +\tfprintf(stderr, \"fatal: %s\\n\\n\", msg);\n> \n> Your commit message does not make clear if you want this \"fatal\" to be\n> grep-able (by scripts) or not. If not, please _() the string.  Maybe\n> this to reduce work for translators\n> \n> \t/* TRANSLATORS: this is the prefix before usage error */\n> \tfprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n\nI don't think we translate any of our \"fatal:\", \"error:\", etc, do we?\nIt certainly doesn't look like it in usage.c.\n\n-Peff\n"},{"id":"308029","messageId":"CACsJy8BgfyDTUmNh_PvoVcz2q92eNTRZy5myegTdi8mu6imVUQ@mail.gmail.com","threadId":"44706","inReplyTo":"20161219134148.vxa4fz3jw2i23lfm@sigill.intra.peff.net","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-12-19T13:53:30Z","receivedAt":"2016-12-19T13:54:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 19, 2016 at 8:41 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Dec 19, 2016 at 07:07:19PM +0700, Duy Nguyen wrote:\n>\n>> On Wed, Dec 14, 2016 at 10:10:10AM -0500, Jeff King wrote:\n>> > diff --git a/parse-options.c b/parse-options.c\n>> > index 312a85dbd..4fbe924a5 100644\n>> > --- a/parse-options.c\n>> > +++ b/parse-options.c\n>> > @@ -661,7 +661,7 @@ void NORETURN usage_msg_opt(const char *msg,\n>> >                const char * const *usagestr,\n>> >                const struct option *options)\n>> >  {\n>> > -   fprintf(stderr, \"%s\\n\\n\", msg);\n>> > +   fprintf(stderr, \"fatal: %s\\n\\n\", msg);\n>>\n>> Your commit message does not make clear if you want this \"fatal\" to be\n>> grep-able (by scripts) or not. If not, please _() the string.  Maybe\n>> this to reduce work for translators\n>>\n>>       /* TRANSLATORS: this is the prefix before usage error */\n>>       fprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n>\n> I don't think we translate any of our \"fatal:\", \"error:\", etc, do we?\n> It certainly doesn't look like it in usage.c.\n\nI know. But those existed before the l10n starts, some of those belong\nto plumbing messages. This one is new.\n-- \nDuy\n"},{"id":"308030","messageId":"20161219140535.ehrbosgn32nl27ki@sigill.intra.peff.net","threadId":"44706","inReplyTo":"CACsJy8BgfyDTUmNh_PvoVcz2q92eNTRZy5myegTdi8mu6imVUQ@mail.gmail.com","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-19T14:05:35Z","receivedAt":"2016-12-19T14:06:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:\n\n> >> Your commit message does not make clear if you want this \"fatal\" to be\n> >> grep-able (by scripts) or not. If not, please _() the string.  Maybe\n> >> this to reduce work for translators\n> >>\n> >>       /* TRANSLATORS: this is the prefix before usage error */\n> >>       fprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n> >\n> > I don't think we translate any of our \"fatal:\", \"error:\", etc, do we?\n> > It certainly doesn't look like it in usage.c.\n> \n> I know. But those existed before the l10n starts, some of those belong\n> to plumbing messages. This one is new.\n\nWe add lots of new messages which are themselves translated, and they\nstill get untranslated prefixes. It seems like consistency is more\nimportant than translating this one spot. But then, I do not use a\ntranslated git myself, so I would not see the difference either way.\n\n-Peff\n"},{"id":"308033","messageId":"20161219143231.nh54e4cebxttvtav@sigill.intra.peff.net","threadId":"44706","inReplyTo":"CACsJy8AswJb1wzV+mcQ3HXrib3HRtVje7PPd9AWPE9Bi32_2sw@mail.gmail.com","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-19T14:32:32Z","receivedAt":"2016-12-19T14:34:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 19, 2016 at 09:30:09PM +0700, Duy Nguyen wrote:\n\n> On Mon, Dec 19, 2016 at 9:05 PM, Jeff King <peff@peff.net> wrote:\n> > On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:\n> >\n> >> >> Your commit message does not make clear if you want this \"fatal\" to be\n> >> >> grep-able (by scripts) or not. If not, please _() the string.  Maybe\n> >> >> this to reduce work for translators\n> >> >>\n> >> >>       /* TRANSLATORS: this is the prefix before usage error */\n> >> >>       fprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n> >> >\n> >> > I don't think we translate any of our \"fatal:\", \"error:\", etc, do we?\n> >> > It certainly doesn't look like it in usage.c.\n> >>\n> >> I know. But those existed before the l10n starts, some of those belong\n> >> to plumbing messages. This one is new.\n> >\n> > We add lots of new messages which are themselves translated, and they\n> > still get untranslated prefixes. It seems like consistency is more\n> > important than translating this one spot. But then, I do not use a\n> > translated git myself, so I would not see the difference either way.\n> \n> I'm kinda used to the half English half Vietnamese messages after so\n> many years (not just git). I do like the prefix translated as well.\n> But I guess we could leave this a is for now. At least we know the\n> scope of this message and will have easier time i18n-izing it when we\n> do the same for die(), warning() and friends.\n\nYeah. It's a big enough change that at the very least it should go into\nits own patch. I don't have a strong opinion myself, except that if we\nwant to leave plumbing messages completely grep-able, we probably need\nto distinguish between different calls to die(), etc. Which sounds kind\nof nasty.\n\nI dunno. I _thought_ nobody was supposed to be grepping stderr, even for\nplumbing commands. But maybe that does not match reality.\n\n-Peff\n"},{"id":"308034","messageId":"CACsJy8AswJb1wzV+mcQ3HXrib3HRtVje7PPd9AWPE9Bi32_2sw@mail.gmail.com","threadId":"44706","inReplyTo":"20161219140535.ehrbosgn32nl27ki@sigill.intra.peff.net","subject":"Re: [PATCH] parse-options: print \"fatal:\" before usage_msg_opt()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-12-19T14:30:09Z","receivedAt":"2016-12-19T15:03:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 19, 2016 at 9:05 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Dec 19, 2016 at 08:53:30PM +0700, Duy Nguyen wrote:\n>\n>> >> Your commit message does not make clear if you want this \"fatal\" to be\n>> >> grep-able (by scripts) or not. If not, please _() the string.  Maybe\n>> >> this to reduce work for translators\n>> >>\n>> >>       /* TRANSLATORS: this is the prefix before usage error */\n>> >>       fprintf(stderr, \"%s %s\\n\\n\", _(\"fatal:\"), msg);\n>> >\n>> > I don't think we translate any of our \"fatal:\", \"error:\", etc, do we?\n>> > It certainly doesn't look like it in usage.c.\n>>\n>> I know. But those existed before the l10n starts, some of those belong\n>> to plumbing messages. This one is new.\n>\n> We add lots of new messages which are themselves translated, and they\n> still get untranslated prefixes. It seems like consistency is more\n> important than translating this one spot. But then, I do not use a\n> translated git myself, so I would not see the difference either way.\n\nI'm kinda used to the half English half Vietnamese messages after so\nmany years (not just git). I do like the prefix translated as well.\nBut I guess we could leave this a is for now. At least we know the\nscope of this message and will have easier time i18n-izing it when we\ndo the same for die(), warning() and friends.\n-- \nDuy\n"}]}