{"thread":{"id":"38336","subject":"[PATCH v3] gettext.h: add parentheses around N_ expansion if supported","startedAt":"2015-01-11T20:09:22Z","lastAt":"2015-01-12T19:08:20Z","messageCount":2,"participants":["Kyle J. McKay","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"254550","messageId":"47914a9078cc9d272204ec00825d67f@74d39fa044aa309eaea14b9f57fe79c","threadId":"38336","inReplyTo":null,"subject":"[PATCH v3] gettext.h: add parentheses around N_ expansion if supported","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-11T20:09:22Z","receivedAt":"2015-01-11T20:09:22Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"The gettext N_ macro is used to mark strings for translation\nwithout actually translating them.  At runtime the string is\nexpected to 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 silently combine the strings into one and at runtime\nthe string passed to gettext will not match the strings that\nwere translated so no translation will actually occur.\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\nHowever, since these string literals are typically assigned to\nstatic variables and not all compilers support parenthesized\nstring literal assignments, allow this to be controlled by the\nMakefile with the default only enabled when the compiler is\nknown to support the syntax.\n\nFor now only __GNUC__ enables this by default which covers both\ngcc and clang which should result in early detection of any\nadjacent N_ macros.\n\nAlthough the necessary tests make the affected files a bit less\nelegant, the benefit of avoiding propagation of a translation-\nmarking error to all the translation teams thus creating extra\nwork for them when the error is eventually detected and fixed\nwould seem to outweigh the minor inelegance the additional\nconfiguration tests introduce.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Kyle J. McKay <mackyle@gmail.com>\n---\n\nThe \"yes\", \"no\", \"auto\" settings for the new Makefile configuration\nvariable \"USE_PARENS_AROUND_GETTEXT_N\" are based on the way the\nMakefile variable \"COMPUTE_HEADER_DEPENDENCIES\" works.  So I used\n\"yes\" and \"no\" for consistency instead of \"Yes\" and \"No\".\n\n Makefile          | 17 +++++++++++++++++\n gettext.h         | 24 ++++++++++++++++++++++++\n git-compat-util.h |  4 ++++\n 3 files changed, 45 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 06e5d243..0f3fbccf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -343,6 +343,15 @@ all::\n # return NULL when it receives a bogus time_t.\n #\n # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.\n+#\n+# Define USE_PARENS_AROUND_GETTEXT_N to \"yes\" if your compiler happily\n+# compiles the following initialization:\n+#\n+#   static const char s[] = (\"FOO\");\n+#\n+# and define it to \"no\" if you need to remove the parentheses () around the\n+# constant.  The default is \"auto\", which means to use parentheses if your\n+# compiler is detected to support it.\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -950,6 +959,14 @@ ifneq (,$(SOCKLEN_T))\n \tBASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T)\n endif\n \n+ifeq (yes,$(USE_PARENS_AROUND_GETTEXT_N))\n+\tBASIC_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=1\n+else\n+ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))\n+\tBASIC_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0\n+endif\n+endif\n+\n ifeq ($(uname_S),Darwin)\n \tifndef NO_FINK\n \t\tifeq ($(shell test -d /sw/lib && echo y),y)\ndiff --git a/gettext.h b/gettext.h\nindex 7671d09d..dc1722dd 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -63,6 +63,30 @@ 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+#if !USE_PARENS_AROUND_GETTEXT_N\n #define N_(msgid) msgid\n+#else\n+/*\n+ * Strictly speaking, this will lead to invalid C when\n+ * used this way:\n+ *\tstatic const char s[] = N_(\"FOO\");\n+ * which will expand to\n+ *\tstatic const char s[] = (\"FOO\");\n+ * and in valid C, the initializer on the right hand side must\n+ * be without the parentheses.  But many compilers do accept it\n+ * as a language extension and it will allow us to catch mistakes\n+ * like:\n+ *\tstatic const char *msgs[] = {\n+ *\t\tN_(\"one\")\n+ *\t\tN_(\"two\"),\n+ *\t\tN_(\"three\"),\n+ *\t\tNULL\n+ *\t};\n+ * (notice the missing comma on one of the lines) by forcing\n+ * a compilation error, because parenthesised (\"one\") (\"two\")\n+ * will not get silently turned into (\"onetwo\").\n+ */\n+#define N_(msgid) (msgid)\n+#endif\n \n #endif\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex dcecd857..107e68e1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -867,4 +867,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);\n #define gmtime_r git_gmtime_r\n #endif\n \n+#if !defined(USE_PARENS_AROUND_GETTEXT_N) && defined(__GNUC__)\n+#define USE_PARENS_AROUND_GETTEXT_N 1\n+#endif\n+\n #endif\n-- \n2.1.4\n"},{"id":"254555","messageId":"xmqqbnm395qz.fsf@gitster.dls.corp.google.com","threadId":"38336","inReplyTo":"47914a9078cc9d272204ec00825d67f@74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH v3] gettext.h: add parentheses around N_ expansion if supported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-12T19:08:20Z","receivedAt":"2015-01-12T19:08:20Z","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> The \"yes\", \"no\", \"auto\" settings for the new Makefile configuration\n> variable \"USE_PARENS_AROUND_GETTEXT_N\" are based on the way the\n> Makefile variable \"COMPUTE_HEADER_DEPENDENCIES\" works.  So I used\n> \"yes\" and \"no\" for consistency instead of \"Yes\" and \"No\".\n\nThanks for a careful execution of my clumsy illustration.\n\n> +# and define it to \"no\" if you need to remove the parentheses () around the\n> +# constant.  The default is \"auto\", which means to use parentheses if your\n> +# compiler is detected to support it.\n\nStrictly speaking, s/detected to/most likely to/, as we are only\nchecking \"__GNUC__\", but we can leave the phrasing as-is and wait\nuntil somebody complains.\n\nThanks.\n"}]}