Re: [PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation

[Vasco Almeida <vascomalmeida@sapo.pt>]

> W dniu 31.08.2016 o 14:31, Vasco Almeida pisze:
> > Use of sprintf following die or error_msg is necessary for
> > placeholder
> > substitution take place.
> 
> No, it is not.  Though I don't think that we have in out Git::I18N
> the support for Perl i18n placeholder substitution.

I will try to change the commit message to better reflect the reality.

> From gettext manual:
> https://www.gnu.org/software/gettext/manual/gettext.html#perl_002dfor
> mat
> 
>   15.3.16 Perl Format Strings
> 
>   There are two kinds format strings in Perl: those acceptable to the
> Perl
>   built-in function printf, labelled as ‘perl-format’, and those
> acceptable
>   to the libintl-perl function __x, labelled as ‘perl-brace-format’.
> 
>   Perl printf format strings are described in the sprintf section of
>   ‘man perlfunc’.
> 
>   Perl brace format strings are described in the
> Locale::TextDomain(3pm)
>   manual page of the CPAN package libintl-perl. In brief, Perl format
> uses
>   placeholders put between braces (‘{’ and ‘}’). The placeholder must
> have
>   the syntax of simple identifiers.
>  
> Git doesn't use Locale::TextDomain, from what I understand, to
> provide
> fallback in no-gettext case.  Also, Locale::TextDomain is not in
> core.

Yes that can be a reason not to use Locale::TextDomain. When Ævar
Arnfjörð Bjarmason added gettext support and i18n stuff, he chose no to
use TextDomain because it did more than he wanted it to do, and that
could introduce bugs and unnecessary work.

5e9637c ("i18n: add infrastructure for translating Git with gettext",
2011-11-18)

https://public-inbox.org/git/AANLkTilYD_NyIZMyj9dHtVk-ylVBfvyxpCC7982LW
nVd@mail.gmail.com/


> > diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> > index e11a33d..4e1e857 100755
> > --- a/git-add--interactive.perl
> > +++ b/git-add--interactive.perl
> > @@ -612,12 +612,12 @@ sub list_and_choose {
> >                       else {
> >                               $bottom = $top = find_unique($choice, @stuff);
> >                               if (!defined $bottom) {
> > -                                     error_msg "Huh ($choice)?\n";
> > +                                     error_msg sprintf(__("Huh (%s)?\n"), $choice);
> 
> So this would be, self explained without need of comment
> for translators:
> 
>   +                                     error_msg __x ("Huh ({choice})?\n"), choice => $choice);
> 
> 
> >                                       next TOPLOOP;
> >                               }
> 
> Though this is probably more work that you wanted to do.
> The __x might be defined like this (borrowing from Locale::TextDomain),
> which needs to be put into perl/Git/I18N.pm
> 
>   sub __ ($);
>   sub __expand ($%);
> 
>   # With interpolation.
>   sub __x ($@)
>   {
>         my ($msgid, %vars) = @_;
> 
>         return __expand (__($msgid), %vars);
>   }
>   
>   sub __expand ($%)
>   {
>         my ($translation, %args) = @_;
>     
>         my $re = join '|', map { quotemeta $_ } keys %args;
>         $translation =~ s/\{($re)\}/defined $args{$1} ? $args{$1} : "{$1}"/ge;
> 
>         return $translation;
>   }

I wonder if it is worth the trouble to add and use these functions,
when there is already a way that works and for me looks simpler. One
reason, if valid, would be that translators already translate strings
with %d and %s from C source which is where the majority of the English
text comes from. Thus it would make little difference for them.
If we use in perl string like in C there is a chance that there will be
a match of some string and would lead to just one msgid instead of two
in the git.pot template for translation. Actually this happens for the
string with "Huh (%s)?" in clean.c.

Unfortunately, I do not know if I would add these changes because I
know little about perl and hence I am not comfortable to do so.

Maybe if you see it is indeed worth adding these to Git I18N.pm, you
could send a follow-up patch or a replacement for this one.