Re: [PATCH v2] i18n: config: unfold error messages marked for translation

[Jeff King <peff@peff.net>]

On Thu, Jul 28, 2016 at 01:14:03PM +0000, Vasco Almeida wrote:

>  static void die_bad_number(const char *name, const char *value)
>  {
> -	const char *reason = errno == ERANGE ?
> -			     "out of range" :
> -			     "invalid unit";
>  	if (!value)
>  		value = "";
>  
> -	if (cf && cf->origin_type && cf->name)
> -		die(_("bad numeric config value '%s' for '%s' in %s %s: %s"),
> -		    value, name, cf->origin_type, cf->name, reason);
> -	die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason);
> +	if (!(cf && cf->name))
> +		die(errno == ERANGE
> +		    ? _("bad numeric config value '%s' for '%s': out of range")
> +		    : _("bad numeric config value '%s' for '%s': invalid unit"),
> +		    value, name);
> +
> +	switch (cf->origin_type) {
> +	case CONFIG_ORIGIN_BLOB:
> +		die(errno == ERANGE
> +		    ? _("bad numeric config value '%s' for '%s' in blob %s: out of range")
> +		    : _("bad numeric config value '%s' for '%s' in blob %s: invalid unit"),
> +		    value, name, cf->name);

I get that the point of this patch is to make more complete sentences
for translation, rather than the lego-brick construction. And that makes
sense to me. But it seems like the ":" is a natural separator, and it
would be OK to make these:

  die(_("bad numeric config value '%s' for '%s' in blob %s: %s"),
	value, name, cf->name, reason);

instead of having two separate strings for the errno values.  After all,
that's what we do everywhere else where "reason" is supplied by
strerror(). It's just in this case that there is no errno value matching
"invalid unit", so we have to fill it in ourselves.

-Peff