[RFC/PATCH] i18n: do not define gettext/ngettext in NO_GETTEXT case

[Jonathan Nieder <jrnieder@gmail.com>]

gettext.h is responsible for defining the _ and  Q_ helpers which are
either simple stubs (in the NO_GETTEXT case) or synonyms for the
libintl functions used to translated git output.  As an implementation
artifact, ever since the !NO_GETTEXT case was implemented
(v1.7.9-rc0~42^2, 2011-11-18), they have also defined gettext and
ngettext macros in the NO_GETTEXT case:

	#ifdef gettext
	# undef gettext
	#endif
	#define gettext(x) (x)

New readers looking at this header might be tempted to use gettext and
ngettext directly, since they are defined in all cases, instead of
using the _ and Q_ wrappers which look like shortcuts.

However gettext and ngettext bypass the GETTEXT_POISON feature.  So
any code using them directly will produce perfectly sensible English
output when run in the test suite with GETTEXT_POISON set, instead of
the poison markers that normally would make it easy to catch output
intended for machines that has been marked for translation by mistake.

Avoid the temptation by _not_ providing fallback definitions for
gettext and ngettext ourselves.  This way, the header is less
misleading and code that uses gettext et al directly will fail to
compile when NO_GETTEXT is set so we can catch it early.

We also take the opportunity to avoid a little ifdef-ery by splitting
the NO_GETTEXT and !NO_GETTEXT implementations into different headers.
Unfortunately this involves some duplication of code.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:

> This will be a problem with GETTEXT_POISON (except your patch bypasses
> poison so it got missed --- will send a relevant fix as a reply).

Here's a rough patch to make that kind of thing easier to catch.  It
might make more sense to do

	#ifdef NO_GETTEXT
	#define git_gettext_(x) (x)
	#define git_ngettext_(msgid, plu, n) ...
	#else
	#define git_gettext_(x) gettext(x)
	#define git_ngettext_(msgid, plu, n) ngettext(msgid, plu, n)
	#endif

	static inline const char *_(const char *msgid)
	{
		return poisoning() ? "poison!" : git_gettext_(msgid);
	}
	...

but I didn't think of it until I had already written this patch.
Anyway, this way you get both approaches for easy comparison. ;-)

 Makefile          |    3 +++
 gettext-libintl.h |   26 ++++++++++++++++++++++++++
 gettext-noop.h    |   30 ++++++++++++++++++++++++++++++
 gettext.h         |   53 +++--------------------------------------------------
 git-compat-util.h |    6 ++++++
 5 files changed, 68 insertions(+), 50 deletions(-)
 create mode 100644 gettext-libintl.h
 create mode 100644 gettext-noop.h

diff --git a/Makefile b/Makefile
index c457c34f..83e8c0f1 100644
--- a/Makefile
+++ b/Makefile
@@ -1525,6 +1525,9 @@ endif
 ifdef NO_GETTEXT
 	BASIC_CFLAGS += -DNO_GETTEXT
 	USE_GETTEXT_SCHEME ?= fallthrough
+	LIB_H += gettext-noop.h
+else
+	LIB_H += gettext-libintl.h
 endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
diff --git a/gettext-libintl.h b/gettext-libintl.h
new file mode 100644
index 00000000..c9199703
--- /dev/null
+++ b/gettext-libintl.h
@@ -0,0 +1,26 @@
+#ifndef GETTEXT_LIBINTL_H
+#define GETTEXT_LIBINTL_H
+
+#include <libintl.h>
+
+#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
+
+extern void git_setup_gettext(void);
+
+static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
+{
+	return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
+}
+
+static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
+const char *Q_(const char *msgid, const char *plu, unsigned long n)
+{
+	if (use_gettext_poison())
+		return "# GETTEXT POISON #";
+	return ngettext(msgid, plu, n);
+}
+
+/* Mark msgid for translation but do not translate it. */
+#define N_(msgid) msgid
+
+#endif
diff --git a/gettext-noop.h b/gettext-noop.h
new file mode 100644
index 00000000..28843a61
--- /dev/null
+++ b/gettext-noop.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2010-2011 Ævar Arnfjörð Bjarmason
+ *
+ * This is a skeleton no-op implementation of gettext for Git.
+ */
+
+#ifndef GETTEXT_NOOP_H
+#define GETTEXT_NOOP_H
+
+#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
+
+static inline void git_setup_gettext(void) { }
+
+static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
+{
+	return use_gettext_poison() ? "# GETTEXT POISON #" : msgid;
+}
+
+static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
+const char *Q_(const char *msgid, const char *plu, unsigned long n)
+{
+	if (use_gettext_poison())
+		return "# GETTEXT POISON #";
+	return n == 1 ? msgid : plu;
+}
+
+/* Mark msgid for translation but do not translate it. */
+#define N_(msgid) msgid
+
+#endif
diff --git a/gettext.h b/gettext.h
index 57ba8bb0..9bc67243 100644
--- a/gettext.h
+++ b/gettext.h
@@ -1,11 +1,3 @@
-/*
- * Copyright (c) 2010-2011 Ævar Arnfjörð Bjarmason
- *
- * This is a skeleton no-op implementation of gettext for Git.
- * You can replace it with something that uses libintl.h and wraps
- * gettext() to try out the translations.
- */
-
 #ifndef GETTEXT_H
 #define GETTEXT_H
 
@@ -13,49 +5,10 @@
 #error "namespace conflict: '_' or 'Q_' is pre-defined?"
 #endif
 
-#ifndef NO_GETTEXT
-#	include <libintl.h>
+#ifdef NO_GETTEXT
+#include "gettext-noop.h"
 #else
-#	ifdef gettext
-#		undef gettext
-#	endif
-#	define gettext(s) (s)
-#	ifdef ngettext
-#		undef ngettext
-#	endif
-#	define ngettext(s, p, n) ((n == 1) ? (s) : (p))
+#include "gettext-libintl.h"
 #endif
 
-#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
-
-#ifndef NO_GETTEXT
-extern void git_setup_gettext(void);
-#else
-static inline void git_setup_gettext(void)
-{
-}
-#endif
-
-#ifdef GETTEXT_POISON
-extern int use_gettext_poison(void);
-#else
-#define use_gettext_poison() 0
-#endif
-
-static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
-{
-	return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
-}
-
-static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
-const char *Q_(const char *msgid, const char *plu, unsigned long n)
-{
-	if (use_gettext_poison())
-		return "# GETTEXT POISON #";
-	return ngettext(msgid, plu, n);
-}
-
-/* Mark msgid for translation but do not translate it. */
-#define N_(msgid) msgid
-
 #endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 8f3972cd..ca4a4f19 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -236,6 +236,12 @@ extern char *gitbasename(char *);
 #endif
 #endif
 
+#ifdef GETTEXT_POISON
+extern int use_gettext_poison(void);
+#else
+#define use_gettext_poison() 0
+#endif
+
 #include "compat/bswap.h"
 
 /* General helper functions */
-- 
1.7.9