Re: [PATCH] parseopt: do not translate empty help string
Junio C Hamano writes: > Junio C Hamano writes: > >>> Oh, I forgot that we actually had a wrapper instead of the usual _. >>> Yes, I think that would be the better solution to guard against this. >> >> OK, then let's replace the patch text of your commit ;-). > > He, we need to update the log message a bit, too. Thanks! But now it's your patch :-) > -- >8 -- > From: Thomas Rast > Subject: [PATCH] gettext: do not translate empty string > > The gettext .po files have a header, but it looks like the translation > specification for an empty string. This results in _("") actually > returning that header. > > Prevent us from passing empty strings to gettext. In some places, ^^ ourselves? I'm not a native speaker though. > we run _(opts->help) where opts->help may be an empty string. > > Signed-off-by: Thomas Rast > Signed-off-by: Junio C Hamano > --- > gettext.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/gettext.h b/gettext.h > index 57ba8bb..376297b 100644 > --- a/gettext.h > +++ b/gettext.h > @@ -44,6 +44,8 @@ extern int use_gettext_poison(void); > > static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) > { > + if (!*msgid) > + return ""; > return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); > } -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] parseopt: do not translate empty help string
Junio C Hamano writes: >> Oh, I forgot that we actually had a wrapper instead of the usual _. >> Yes, I think that would be the better solution to guard against this. > > OK, then let's replace the patch text of your commit ;-). He, we need to update the log message a bit, too. -- >8 -- From: Thomas Rast Subject: [PATCH] gettext: do not translate empty string The gettext .po files have a header, but it looks like the translation specification for an empty string. This results in _("") actually returning that header. Prevent us from passing empty strings to gettext. In some places, we run _(opts->help) where opts->help may be an empty string. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- gettext.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gettext.h b/gettext.h index 57ba8bb..376297b 100644 --- a/gettext.h +++ b/gettext.h @@ -44,6 +44,8 @@ extern int use_gettext_poison(void); static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) { + if (!*msgid) + return ""; return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); } -- 1.7.12.124.ga484625 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] parseopt: do not translate empty help string
Thomas Rast writes: > Junio C Hamano writes: > >> Thomas Rast writes: >> >>> The gettext .po files have a header, but it looks like the translation >>> specification for an empty string. This results in _("") actually >>> returning that header. >> >> Thanks; this is a tricky bit to catch and makes me wonder where else >> we have a similar breakage. >> >> Perhaps we would want to do this instead? I dunno. >> >> gettext.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git i/gettext.h w/gettext.h >> index 57ba8bb..376297b 100644 >> --- i/gettext.h >> +++ w/gettext.h >> @@ -44,6 +44,8 @@ extern int use_gettext_poison(void); >> >> static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) >> { >> +if (!*msgid) >> +return ""; >> return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); >> } > > Oh, I forgot that we actually had a wrapper instead of the usual _. > Yes, I think that would be the better solution to guard against this. OK, then let's replace the patch text of your commit ;-). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] parseopt: do not translate empty help string
Junio C Hamano writes: > Thomas Rast writes: > >> The gettext .po files have a header, but it looks like the translation >> specification for an empty string. This results in _("") actually >> returning that header. > > Thanks; this is a tricky bit to catch and makes me wonder where else > we have a similar breakage. > > Perhaps we would want to do this instead? I dunno. > > gettext.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git i/gettext.h w/gettext.h > index 57ba8bb..376297b 100644 > --- i/gettext.h > +++ w/gettext.h > @@ -44,6 +44,8 @@ extern int use_gettext_poison(void); > > static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) > { > + if (!*msgid) > + return ""; > return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); > } Oh, I forgot that we actually had a wrapper instead of the usual _. Yes, I think that would be the better solution to guard against this. -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] parseopt: do not translate empty help string
Thomas Rast writes: > The gettext .po files have a header, but it looks like the translation > specification for an empty string. This results in _("") actually > returning that header. > > Prevent parseopt from passing empty strings to gettext when it > displays help about commands. In some instances it already did this, > but git-grep's --or etc. caught another case. > > Signed-off-by: Thomas Rast > --- > parse-options.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/parse-options.c b/parse-options.c > index c1c66bd..f95bbb2 100644 > --- a/parse-options.c > +++ b/parse-options.c > @@ -562,7 +562,8 @@ static int usage_with_options_internal(struct > parse_opt_ctx_t *ctx, > fputc('\n', outfile); > pad = USAGE_OPTS_WIDTH; > } > - fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help)); > + fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", > + *opts->help ? _(opts->help) : ""); > } > fputc('\n', outfile); Thanks; this is a tricky bit to catch and makes me wonder where else we have a similar breakage. Perhaps we would want to do this instead? I dunno. gettext.h | 2 ++ 1 file changed, 2 insertions(+) diff --git i/gettext.h w/gettext.h index 57ba8bb..376297b 100644 --- i/gettext.h +++ w/gettext.h @@ -44,6 +44,8 @@ extern int use_gettext_poison(void); static inline FORMAT_PRESERVING(1) const char *_(const char *msgid) { + if (!*msgid) + return ""; return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid); } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] parseopt: do not translate empty help string
The gettext .po files have a header, but it looks like the translation specification for an empty string. This results in _("") actually returning that header. Prevent parseopt from passing empty strings to gettext when it displays help about commands. In some instances it already did this, but git-grep's --or etc. caught another case. Signed-off-by: Thomas Rast --- parse-options.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/parse-options.c b/parse-options.c index c1c66bd..f95bbb2 100644 --- a/parse-options.c +++ b/parse-options.c @@ -562,7 +562,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx, fputc('\n', outfile); pad = USAGE_OPTS_WIDTH; } - fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help)); + fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", + *opts->help ? _(opts->help) : ""); } fputc('\n', outfile); -- 1.7.12.rc1.203.gc3c8071 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html