{"thread":{"id":"38290","subject":"[PATCH 1/2] log.c: fix translation markings","startedAt":"2015-01-06T10:34:18Z","lastAt":"2015-01-06T19:29:02Z","messageCount":6,"participants":["Kyle J. McKay","Ramsay Jones","Andreas Schwab","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"254323","messageId":"ecb18f9d6ac56da0a61c3b98f8f2236@74d39fa044aa309eaea14b9f57fe79c","threadId":"38290","inReplyTo":null,"subject":"[PATCH 1/2] log.c: fix translation markings","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T10:34:18Z","receivedAt":"2015-01-06T10:34:18Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"The parse_options API expects an array of alternative usage lines\nto which it automatically ads the language-appropriate \"or\" when\ndisplaying.  Each of these options is marked for translation with N_\nand then later translated when gettext is called on each element\nof the array.\n\nSince the N_ macro just expands to its argument, if two N_-marked\nstrings appear next to each other without being separated by anything\nelse such as a comma, the preprocessor will join them into one string.\n\nIn that case two separate strings get marked for translation, but at\nruntime they have been joined into a single string passed to gettext\nwhich then fails to get translated because the combined string was\nnever marked for translation.\n\nFix this by properly separating the two N_ marked strings with\na comma and removing the embedded \"\\n\" and \"   or:\" that are\nproperly supplied by the parse_options API.\n\nSigned-off-by: Kyle J. McKay <mackyle@gmail.com>\n---\n builtin/log.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f2a9f015..923ffe72 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -38,8 +38,8 @@ static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n static const char * const builtin_log_usage[] = {\n-\tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\\n\")\n-\tN_(\"   or: git show [options] <object>...\"),\n+\tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\"),\n+\tN_(\"git show [options] <object>...\"),\n \tNULL\n };\n \n-- \n2.1.4\n"},{"id":"254324","messageId":"f67db70c63f8dcbfc58d7ffda86ff10@74d39fa044aa309eaea14b9f57fe79c","threadId":"38290","inReplyTo":"ecb18f9d6ac56da0a61c3b98f8f2236@74d39fa044aa309eaea14b9f57fe79c","subject":"[PATCH 2/2] gettext.h: add parentheses around N_ expansion","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T10:34:19Z","receivedAt":"2015-01-06T10:34:19Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"The N_ macro is used to mark strings for translation without\nactually translating them.  At runtime the string is expected\nto be passed to the gettext API for translation.\n\nIf two N_ macro invocations appear next to each other with only\nwhitespace (or nothing at all) between them, the two separate\nstrings will be marked for translation, but the preprocessor\nwill then combine the strings into one and at runtime the\nstring passed to gettext will not match the strings that were\ntranslated.\n\nAvoid this by adding parentheses around the expansion of the\nN_ macro so that instead of ending up with two adjacent strings\nthat are then combined by the preprocessor, two adjacent strings\nsurrounded by parentheses result instead which causes a compile\nerror so the mistake can be quickly found and corrected.\n\nSigned-off-by: Kyle J. McKay <mackyle@gmail.com>\n---\nThis patch is optional, but prevents the problem fixed by 1/2\nfrom recurring.\n\n gettext.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gettext.h b/gettext.h\nindex 7671d09d..d11a4139 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -63,6 +63,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)\n }\n \n /* Mark msgid for translation but do not translate it. */\n-#define N_(msgid) msgid\n+#define N_(msgid) (msgid)\n \n #endif\n-- \n2.1.4\n"},{"id":"254333","messageId":"54ABE205.70801@ramsay1.demon.co.uk","threadId":"38290","inReplyTo":"f67db70c63f8dcbfc58d7ffda86ff10@74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH 2/2] gettext.h: add parentheses around N_ expansion","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2015-01-06T13:24:21Z","receivedAt":"2015-01-06T13:24:21Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"On 06/01/15 10:34, Kyle J. McKay wrote:\n> The N_ macro is used to mark strings for translation without\n> actually translating them.  At runtime the string is expected\n> to be passed to the gettext API for translation.\n> \n> If two N_ macro invocations appear next to each other with only\n> whitespace (or nothing at all) between them, the two separate\n> strings will be marked for translation, but the preprocessor\n> will then combine the strings into one and at runtime the\n> string passed to gettext will not match the strings that were\n> translated.\n> \n> Avoid this by adding parentheses around the expansion of the\n> N_ macro so that instead of ending up with two adjacent strings\n> that are then combined by the preprocessor, two adjacent strings\n> surrounded by parentheses result instead which causes a compile\n> error so the mistake can be quickly found and corrected.\n> \n> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>\n> ---\n> This patch is optional, but prevents the problem fixed by 1/2\n> from recurring.\n> \n>  gettext.h | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/gettext.h b/gettext.h\n> index 7671d09d..d11a4139 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -63,6 +63,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)\n>  }\n>  \n>  /* Mark msgid for translation but do not translate it. */\n> -#define N_(msgid) msgid\n> +#define N_(msgid) (msgid)\n>  \n>  #endif\n> \n\nHmm, see commit 642f85faa (\"i18n: avoid parenthesized string as\narray initializer\", 07-04-2011), for a counter-point. :-P\n\nATB,\nRamsay Jones\n"},{"id":"254337","messageId":"694DC462-40E3-4081-8616-64483FCD0D77@gmail.com","threadId":"38290","inReplyTo":"54ABE205.70801@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/2] gettext.h: add parentheses around N_ expansion","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T14:38:58Z","receivedAt":"2015-01-06T14:38:58Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 05:24, Ramsay Jones wrote:\n\n> On 06/01/15 10:34, Kyle J. McKay wrote:\n>> Avoid this by adding parentheses around the expansion of the\n>> N_ macro so that instead of ending up with two adjacent strings\n>> that are then combined by the preprocessor, two adjacent strings\n>> surrounded by parentheses result instead which causes a compile\n>> error so the mistake can be quickly found and corrected.\n>>\n>> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>\n>> ---\n>> This patch is optional, but prevents the problem fixed by 1/2\n>> from recurring.\n>>\n>> gettext.h | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/gettext.h b/gettext.h\n>> index 7671d09d..d11a4139 100644\n>> --- a/gettext.h\n>> +++ b/gettext.h\n>> @@ -63,6 +63,6 @@ const char *Q_(const char *msgid, const char  \n>> *plu, unsigned long n)\n>> }\n>>\n>> /* Mark msgid for translation but do not translate it. */\n>> -#define N_(msgid) msgid\n>> +#define N_(msgid) (msgid)\n>>\n>> #endif\n>>\n>\n> Hmm, see commit 642f85faa (\"i18n: avoid parenthesized string as\n> array initializer\", 07-04-2011), for a counter-point. :-P\n\nOh interesting.  So the bug bug fixed by 1/2 must have crept in after  \nthat change then.\n\nEven clang -ansi -pedantic doesn't seem to complain about this.  And  \n(\"a\") is just as much a constant expression as \"a\".  Are you sure it's  \nnot just a tcc bug?\n\nIn any case, 642f85faa is talking about this:\n\n   static const char ignore_error[] = (\"something\");\n\nwhich is assigning a const char * to a const char [].\n\nBut builtin/log.c uses this:\n\n   static const char * const builtin_log_usage[] = {(\"something\",  \n\"else\"};\n\nWhich is assigning a const char * to a const char * const.\n\nAnyhow, if it breaks some existing compiler that works now, it  \nshouldn't be picked up.  But it would sure be nice to have something  \nto prevent a recurrence of the bug 1/2 fixes.  Perhaps the (msgid)  \nversion should be enabled when __GNUC__ is defined -- that should be  \ncompatible and enough folks use that it would catch such a problem  \nearly.\n\n-Kyle\n"},{"id":"254339","messageId":"87tx04rlbw.fsf@igel.home","threadId":"38290","inReplyTo":"694DC462-40E3-4081-8616-64483FCD0D77@gmail.com","subject":"Re: [PATCH 2/2] gettext.h: add parentheses around N_ expansion","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2015-01-06T15:16:19Z","receivedAt":"2015-01-06T15:16:19Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"\"Kyle J. McKay\" <mackyle@gmail.com> writes:\n\n> Even clang -ansi -pedantic doesn't seem to complain about this.  And (\"a\")\n> is just as much a constant expression as \"a\".  Are you sure it's not just\n> a tcc bug?\n\nThe C standard asks for a string literal as the initializer, not an\nexpression.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"254361","messageId":"xmqqzj9voght.fsf@gitster.dls.corp.google.com","threadId":"38290","inReplyTo":"ecb18f9d6ac56da0a61c3b98f8f2236@74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH 1/2] log.c: fix translation markings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T19:29:02Z","receivedAt":"2015-01-06T19:29:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" <mackyle@gmail.com> writes:\n\n> Fix this by properly separating the two N_ marked strings with\n> a comma ...\n\nSensible.\n\n> ... and removing the embedded \"\\n\" and \"   or:\" that are\n> properly supplied by the parse_options API.\n\nWow.  I learn new things or reminded of old things every day.\n\nThanks.\n\n>\n> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>\n> ---\n>  builtin/log.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index f2a9f015..923ffe72 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -38,8 +38,8 @@ static const char *fmt_patch_subject_prefix = \"PATCH\";\n>  static const char *fmt_pretty;\n>  \n>  static const char * const builtin_log_usage[] = {\n> -\tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\\n\")\n> -\tN_(\"   or: git show [options] <object>...\"),\n> +\tN_(\"git log [<options>] [<revision range>] [[--] <path>...]\"),\n> +\tN_(\"git show [options] <object>...\"),\n>  \tNULL\n>  };\n"}]}