From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 09 Jan 2015 22:22:46 GMT
Subject: Re: [PATCH v2] gettext.h: add parentheses around N_ expansion if supported
Message-ID: <xmqqwq4vbnm1.fsf@gitster.dls.corp.google.com>
In-Reply-To: <61CC69D2-A0F2-4BE2-B37A-F1D8FE19C45A@gmail.com>

"Kyle J. McKay" <mackyle@gmail.com> writes:

> I'm not super attached to this change, it's just that it seems to me
> that translation support for Git is a scarce resource.  I'm guessing
> that when considering the 7 complete translations (bg, ca, de, fr, sv,
> vi and zh_CN) the average number of translators per language is in the
> low single digits.  So I hate to see unnecessary translation churn,
> not when it can be so easily prevented.

Yes, I share the same feeling and I agree that it is a worthy goal.

I just did not want an unconditional "#ifdef __GNUC__" that nobody
can override without editing the source, when __GNUC__ is a rough
approximation whether a specific language extension exists and is
enabled (we do not know which -W<option> or options like --pedantic
that will be added in the future would interfere with us).

What I had in mind instead was something along this line (but with a
better make variable name).  In an unconfigured build, it decides
between ("msg") and "msg" using the same __GNUC__ heuristic, but
that can be overridden from the $(MAKE) command line in a pinch.

I do not do autoconf, but it would be trivial to set CAN_USE_* by
try-compiling a small program.

 Makefile          | 17 ++++++++++++++++-
 gettext.h         | 24 ++++++++++++++++++++++++
 git-compat-util.h |  7 +++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 06e5d24..48f4ce2 100644
--- a/Makefile
+++ b/Makefile
@@ -343,6 +343,12 @@ all::
 # return NULL when it receives a bogus time_t.
 #
 # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
+#
+# Define CAN_USE_CONSTANT_STRING_IN_PARENTHESES to Yes if your compiler
+# happily compiles the following initialization:
+#     static const char s[] = ("FOO");
+# and define it to No if you need to remove the parentheses () around the
+# constant.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -950,6 +956,16 @@ ifneq (,$(SOCKLEN_T))
 	BASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T)
 endif
 
+ifeq (Yes,$(CAN_USE_CONSTANT_STRING_IN_PARENTHESES))
+	BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=1
+else
+ifneq (,$(CAN_USE_CONSTANT_STRING_IN_PARENTHESES))
+	BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=0
+else
+	BASIC_CFLAGS += -DUSE_PARENS_AROUND_N=-1
+endif
+endif
+
 ifeq ($(uname_S),Darwin)
 	ifndef NO_FINK
 		ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -2486,4 +2502,3 @@ cover_db: coverage-report
 
 cover_db_html: cover_db
 	cover -report html -outputdir cover_db_html cover_db
-
diff --git a/gettext.h b/gettext.h
index 7671d09..9b54ded 100644
--- a/gettext.h
+++ b/gettext.h
@@ -63,6 +63,30 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)
 }
 
 /* Mark msgid for translation but do not translate it. */
+#if !USE_PARENS_AROUND_N
 #define N_(msgid) msgid
+#else
+/*
+ * Strictly speaking, this will lead to invalid C when
+ * used this way:
+ *     static const char s[] = N_("FOO");
+ * which will expand to
+ *     static const char s[] = ("FOO");
+ * and in valid C, the initializer on the right hand side must
+ * be without the parentheses.  But many compilers do accept it
+ * as a language extension and it will allow us to catch mistakes
+ * like:
+ *     static const char **msgs = {
+ *                N_("one")
+ *                N_("two"),
+ *                N_("three"),
+ *                NULL
+ *     }
+ * (notice the missing comma on one of the lines) by forcing
+ * an compilation error, because parenthesised ("one") ("two")
+ * will not get silently turned into ("onetwo").
+ */
+#define N_(msgid) (msgid)
+#endif
 
 #endif
diff --git a/git-compat-util.h b/git-compat-util.h
index dcecd85..8640163 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -867,4 +867,11 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
 #define gmtime_r git_gmtime_r
 #endif
 
+#if USE_PARENS_AROUND_N == -1
+#  ifdef __GNUC__
+#    undef USE_PARENS_AROUND_N
+#    define USE_PARENS_AROUND_N 1
+#  endif
+#endif
+
 #endif