git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Feb 2, 2012, 00:20 UTC
Message-ID
<20120202002053.GD29599@burratino>
In-Reply-To
<20120201233515.GC29599@burratino>

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
Previous: Jonathan NiederNext: Jonathan Nieder
Message 18 of 32 in “Correct singular form in diff summary line for human interaction”
  1. Correct singular form in diff summary line for human interactionNguyễn Thái Ngọc Duy, Jan 31, 2012
  2. Jonathan NiederJan 31, 2012
  3. Junio C HamanoJan 31, 2012
  4. Nguyen Thai Ngoc DuyFeb 1, 2012
  5. Thomas DickeyFeb 1, 2012
  6. Thomas DickeyFeb 1, 2012
  7. Nguyen Thai Ngoc DuyFeb 1, 2012
  8. Junio C HamanoFeb 1, 2012
  9. Thomas DickeyFeb 1, 2012
  10. Use correct grammar in diffstat summary lineNguyễn Thái Ngọc Duy, Feb 1, 2012
  11. Junio C HamanoFeb 1, 2012
  12. Nguyen Thai Ngoc DuyFeb 2, 2012
  13. Junio C HamanoFeb 2, 2012
  14. Nguyen Thai Ngoc DuyFeb 3, 2012
  15. Junio C HamanoFeb 3, 2012
  16. Jeff KingFeb 3, 2012
  17. Jonathan NiederFeb 1, 2012
  18. i18n: do not define gettext/ngettext in NO_GETTEXT caseJonathan Nieder, Feb 2, 2012
  19. Jonathan NiederFeb 2, 2012
  20. Jonathan NiederFeb 2, 2012
  21. 0/7 tests: diffstat summary line varies by locale (Re: [PATCH v2] Use correct grammar in diffstat summary line)Jonathan Nieder, Mar 13, 2012
  22. 1/7 test: use test_i18ncmp when checking --stat outputJonathan Nieder, Mar 13, 2012
  23. Junio C HamanoMar 13, 2012
  24. Junio C HamanoMar 13, 2012
  25. Jonathan NiederMar 13, 2012
  26. Jonathan NiederMar 13, 2012
  27. 2/7 test: use numstat instead of diffstat in funny-names testJonathan Nieder, Mar 13, 2012
  28. 3/7 test: modernize funny-names test styleJonathan Nieder, Mar 13, 2012
  29. 4/7 test: test cherry-pick functionality and output separatelyJonathan Nieder, Mar 13, 2012
  30. 5/7 test: use --numstat instead of --stat in "git stash show" testsJonathan Nieder, Mar 13, 2012
  31. 6/7 test: use numstat instead of diffstat in binary-diff testJonathan Nieder, Mar 13, 2012
  32. 7/7 diffstat summary line varies by locale: miscellanyJonathan Nieder, Mar 13, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.