Re: [PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
> 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
n...@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.
Re: [PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
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.
>From gettext manual:
https://www.gnu.org/software/gettext/manual/gettext.html#perl_002dformat
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.
The syntax, with the help of shorthand helper function, looks like this:
http://search.cpan.org/dist/libintl-perl/lib/Locale/TextDomain.pm#EXPORTED_FUNCTIONS
https://metacpan.org/pod/Locale::TextDomain#EXPORTED-FUNCTIONS
__x MSGID, ID1 => VAL1, ID2 => VAL2, ...
One of the nicest features in Perl is its capability to interpolate
variables into strings:
print "This is the $color $thing.\n";
This nice feature might con you into thinking that you could now write
print __"This is the $color $thing.\n";
[But this doesn't work...]
[...] The Perl backend to GNU gettext has defined an alternative format
[to using printf / sprintf] for interpolatable strings:
"This is the {color} {thing}.\n";
Instead of Perl variables you use place-holders (legal Perl variables
are also legal place-holders) in curly braces, and then you call
print __x ("This is the {color} {thing}.\n",
thing => $thang,
color => $color);
> Signed-off-by: Vasco Almeida
> ---
> git-add--interactive.perl | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> 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;
}
Best regards,
--
Jakub Narębski
Re: [PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
Vasco Almeida writes:
> @@ -1048,7 +1048,7 @@ sub edit_hunk_manually {
> my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
> my $fh;
> open $fh, '>', $hunkfile
> - or die "failed to open hunk edit file for writing: " . $!;
> + or die sprintf(__("failed to open hunk edit file for writing:
> %s"), $!);
OK, $! presumably is given in the user's language, so we let
translators prepare the error-specific text and interpolate $! into
it.
Makes sense.
[PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
Use of sprintf following die or error_msg is necessary for placeholder
substitution take place.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
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);
next TOPLOOP;
}
}
if ($opts->{SINGLETON} && $bottom != $top) {
- error_msg "Huh ($choice)?\n";
+ error_msg sprintf(__("Huh (%s)?\n"), $choice);
next TOPLOOP;
}
for ($i = $bottom-1; $i <= $top-1; $i++) {
@@ -1048,7 +1048,7 @@ sub edit_hunk_manually {
my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
my $fh;
open $fh, '>', $hunkfile
- or die "failed to open hunk edit file for writing: " . $!;
+ or die sprintf(__("failed to open hunk edit file for writing:
%s"), $!);
print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
print $fh @$oldtext;
my $participle = $patch_mode_flavour{PARTICIPLE};
@@ -1075,7 +1075,7 @@ EOF
}
open $fh, '<', $hunkfile
- or die "failed to open hunk edit file for reading: " . $!;
+ or die sprintf(__("failed to open hunk edit file for reading:
%s"), $!);
my @newtext = grep { !/^#/ } <$fh>;
close $fh;
unlink $hunkfile;
@@ -1225,7 +1225,7 @@ sub apply_patch_for_checkout_commit {
sub patch_update_cmd {
my @all_mods = list_modified($patch_mode_flavour{FILTER});
- error_msg "ignoring unmerged: $_->{VALUE}\n"
+ error_msg sprintf(__("ignoring unmerged: %s\n"), $_->{VALUE})
for grep { $_->{UNMERGED} } @all_mods;
@all_mods = grep { !$_->{UNMERGED} } @all_mods;
@@ -1407,11 +1407,13 @@ sub patch_update_file {
chomp $response;
}
if ($response !~ /^\s*\d+\s*$/) {
- error_msg "Invalid number:
'$response'\n";
+ error_msg sprintf(__("Invalid number:
'%s'\n"),
+$response);
} elsif (0 < $response && $response <= $num) {
$ix = $response - 1;
} else {
- error_msg "Sorry, only $num hunks
available.\n";
+ error_msg sprintf(__("Sorry, only %s
hunks available.\n"),
+$num);
}
next;
}
@@ -1449,7 +1451,7 @@ sub patch_update_file {
if ($@) {
my ($err,$exp) = ($@, $1);
$err =~ s/ at .*git-add--interactive
line \d+, line \d+.*$//;
- error_msg "Malformed search regexp
$exp: $err\n";
+ error_msg sprintf(__("Malformed search
regexp %s: %s\n"), $exp, $err);
next;
}
my $iy = $ix;
@@ -1612,18 +1614,18 @@ sub process_args {
$patch_mode = $1;
$arg = shift @ARGV or die __("missing --");
} else {
- die "unknown --patch mode: $1";
+ die sprintf(__("unknown --patch mode: %s"), $1);
}
} else {
$patch_mode = 'stage';
$arg = shift @ARGV or die __("missing --");
}
- die "invalid argument $arg, expecting --"
- unless $arg eq "--";
+ die sprintf(__("invalid argument %s, expecting --"),
+ $arg) unless $arg eq "--";
%patch_mode_flavour = %{$patch_modes{$patch_mode}};
}
elsif ($arg ne "--") {
- die "invalid argument $arg, expecting --";
+ die sprintf(__("invalid argument %s, expe
[PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
Use of sprintf following die or error_msg is necessary for placeholder
substitution take place.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
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);
next TOPLOOP;
}
}
if ($opts->{SINGLETON} && $bottom != $top) {
- error_msg "Huh ($choice)?\n";
+ error_msg sprintf(__("Huh (%s)?\n"), $choice);
next TOPLOOP;
}
for ($i = $bottom-1; $i <= $top-1; $i++) {
@@ -1048,7 +1048,7 @@ sub edit_hunk_manually {
my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
my $fh;
open $fh, '>', $hunkfile
- or die "failed to open hunk edit file for writing: " . $!;
+ or die sprintf(__("failed to open hunk edit file for writing:
%s"), $!);
print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
print $fh @$oldtext;
my $participle = $patch_mode_flavour{PARTICIPLE};
@@ -1075,7 +1075,7 @@ EOF
}
open $fh, '<', $hunkfile
- or die "failed to open hunk edit file for reading: " . $!;
+ or die sprintf(__("failed to open hunk edit file for reading:
%s"), $!);
my @newtext = grep { !/^#/ } <$fh>;
close $fh;
unlink $hunkfile;
@@ -1225,7 +1225,7 @@ sub apply_patch_for_checkout_commit {
sub patch_update_cmd {
my @all_mods = list_modified($patch_mode_flavour{FILTER});
- error_msg "ignoring unmerged: $_->{VALUE}\n"
+ error_msg sprintf(__("ignoring unmerged: %s\n"), $_->{VALUE})
for grep { $_->{UNMERGED} } @all_mods;
@all_mods = grep { !$_->{UNMERGED} } @all_mods;
@@ -1407,11 +1407,13 @@ sub patch_update_file {
chomp $response;
}
if ($response !~ /^\s*\d+\s*$/) {
- error_msg "Invalid number:
'$response'\n";
+ error_msg sprintf(__("Invalid number:
'%s'\n"),
+$response);
} elsif (0 < $response && $response <= $num) {
$ix = $response - 1;
} else {
- error_msg "Sorry, only $num hunks
available.\n";
+ error_msg sprintf(__("Sorry, only %s
hunks available.\n"),
+$num);
}
next;
}
@@ -1449,7 +1451,7 @@ sub patch_update_file {
if ($@) {
my ($err,$exp) = ($@, $1);
$err =~ s/ at .*git-add--interactive
line \d+, line \d+.*$//;
- error_msg "Malformed search regexp
$exp: $err\n";
+ error_msg sprintf(__("Malformed search
regexp %s: %s\n"), $exp, $err);
next;
}
my $iy = $ix;
@@ -1612,18 +1614,18 @@ sub process_args {
$patch_mode = $1;
$arg = shift @ARGV or die __("missing --");
} else {
- die "unknown --patch mode: $1";
+ die sprintf(__("unknown --patch mode: %s"), $1);
}
} else {
$patch_mode = 'stage';
$arg = shift @ARGV or die __("missing --");
}
- die "invalid argument $arg, expecting --"
- unless $arg eq "--";
+ die sprintf(__("invalid argument %s, expecting --"),
+ $arg) unless $arg eq "--";
%patch_mode_flavour = %{$patch_modes{$patch_mode}};
}
elsif ($arg ne "--") {
- die "invalid argument $arg, expecting --";
+ die sprintf(__("invalid argument %s, expe
