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@xxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html