threads / patch / 31296

patchparseopt: do not translate empty help string

Subject: [PATCH] parseopt: do not translate empty help string

## tl;dr

6 messages between Aug 20, 2012 and Aug 20, 2012. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Thomas Rast· Aug 20, 2012, 18:24 UTC · lore

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 <trast@student.ethz.ch>
---
 parse-options.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to parse-options.c +2 −1
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
Junio C Hamano· Aug 20, 2012, 19:21 UTC · re: Thomas Rast · lore

Re: [PATCH] parseopt: do not translate empty help string

Thomas Rast <trast@student.ethz.ch> writes:
Show 26 quoted lines
> 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 <trast@student.ethz.ch>
> ---
>  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(+)
Show changes to diff +2 −0
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);
 }
 
Thomas Rast· Aug 20, 2012, 20:10 UTC · re: Junio C Hamano · lore

Re: [PATCH] parseopt: do not translate empty help string

Junio C Hamano <gitster@pobox.com> writes:
Show 26 quoted lines
> Thomas Rast <trast@student.ethz.ch> 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
Junio C Hamano· Aug 20, 2012, 22:04 UTC · re: Thomas Rast · lore

Re: [PATCH] parseopt: do not translate empty help string

Thomas Rast <trast@student.ethz.ch> writes:
Show 31 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Rast <trast@student.ethz.ch> 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 ;-).
Junio C Hamano· Aug 20, 2012, 22:08 UTC · re: Junio C Hamano · lore

Re: [PATCH] parseopt: do not translate empty help string

Junio C Hamano <gitster@pobox.com> 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 <trast@student.ethz.ch>
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 <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 gettext.h | 2 ++
 1 file changed, 2 insertions(+)
Show changes to gettext.h +2 −0
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
Thomas Rast· Aug 20, 2012, 23:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] parseopt: do not translate empty help string

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Junio C Hamano <gitster@pobox.com> 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 :-)
Show 9 quoted lines
> -- >8 --
> From: Thomas Rast <trast@student.ethz.ch>
> 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.
Show 20 quoted lines
> we run _(opts->help) where opts->help may be an empty string.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  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

← back to recent threads