Re: [PATCH v2 04/11] i18n: add--interactive: mark plural strings

> W dniu 26.09.2016 o 20:15, Vasco Almeida pisze:
> > 
> > A Qua, 31-08-2016 às 12:31 +0000, Vasco Almeida escreveu:
> > > 
> > > 
> > > Mark plural strings for translation.  Unfold each action case in
> > > one
> > > entire sentence.
> > > 
> > > Pass new keyword for xgettext to extract.
> > > 
> > > Update test to include new subrotine Q__() for plural strings
> > > handling.
> 
> Why use Q__() as the name of the subroutine? [looks further]. Oh, I
> see
> that you are following the example of C shortcut functions (_, Q_ and
> N_).
> 
> But this is Perl, not C.  The standard shortcut functions are those
> defined in Locale::TextDomain, even if we can't use this module
> directly.
> Those that deal with plural strings handling are __n and __nx / __xn.
> 
> The Perl equivalent of Q_ shorthand function in C, C++, etc. is __n.
> There is also a function __nx for combining handling plural strings
> together with variable interpolation.

> > > diff --git a/git-add--interactive.perl b/git-add
> > > --interactive.perl
> > > index 4e1e857..08badfa 100755
> > > --- a/git-add--interactive.perl
> > > +++ b/git-add--interactive.perl
> > > @@ -666,12 +666,18 @@ sub status_cmd {
> > >  sub say_n_paths {
> > >  	my $did = shift @_;
> > >  	my $cnt = scalar @_;
> > > -	print "$did ";
> > > -	if (1 < $cnt) {
> > > -		print "$cnt paths\n";
> > > -	}
> > > -	else {
> > > -		print "one path\n";
> > > +	if ($did eq 'added') {
> > > +		printf(Q__("added one path\n", "added %d
> > > paths\n",
> > > +			   $cnt), $cnt);
> > > +	} elsif ($did eq 'updated') {
> > > +		printf(Q__("updated one path\n", "updated %d
> > > paths\n",
> > > +			   $cnt), $cnt);
> > > +	} elsif ($did eq 'reverted') {
> > > +		printf(Q__("reverted one path\n", "reverted %d
> > > paths\n",
> > > +			   $cnt), $cnt);
> > > +	} else {
> > > +		printf(Q__("touched one path\n", "touched %d
> > > paths\n",
> > > +			   $cnt), $cnt);
> > >  	}
> > >  }
> 
> One one hand side, it is recommended to avoid lego-like construction
> of sentences.
> 
>   Translatable strings should be entire sentences. It is often not
>   possible to translate single verbs or adjectives in a substitutable
>   way.
> 
> I think however that the action part ($did in original non-i18n code)
> is whole part in any language, so something like the following would
> be enough:
> 
>   	# this hash is as much for validation, as for translation
>    	my %actions = map { $_ => 1 } (N__"added", N__"updated",
> N__"reverted");
>    	if (exists $actions{$did}) {
>    		print __nx("{did} one path\n", "{did} {count}
> paths\n", $cnt,
>    			   did => __($did), count => $cnt);
>   	} else {
>    		print __nx("touched one path\n", "touched {count}
> paths\n", $cnt,
>    			   count => $cnt);
>    	}
> 
> Please correct me if I am wrong, and you know language where
> "added %d paths", "updated %d paths", "reverted %d paths" etc. must
> have
> different word order.

> > When $cnt is 1 I get the following warning:
> > Redundant argument in printf at .../libexec/git-core/git-add
> > --interactive line 680.
> 
> I wonder what is the case of C code - is similar warning here, or is
> gettext smarter in that case...

> > The singular form does not have a %d to consume $cnt argument to
> > printf(). Either we find a way to suppress that warning or we
> > change
> > the singular form to contain %d.
> 
> Anyway, with __nx there should be no such problem.
> 
> > 
> > 
> > > 
> > > @@ -1508,8 +1514,10 @@ sub patch_update_file {
> > > ...
> > > -					print colored
> > > $header_color, "Split into ",
> > > -					scalar(@split), "
> > > hunks.\n";
> > > +					print colored
> > > $header_color, sprintf(
> > > +						Q__("Split into
> > > %d hunk.\n",
> > > +						    "Split into
> > > %d hunks.\n",
> > > +						    scalar(@spli
> > > t)), scalar(@split));
> > 
> > Like we do with this.
> 
> Note that it is a bit of change in behavior: previously Git would
> say "Split into 0 hunks.\n".
> 
> 
> Though this is probably more work that you wanted to do.  The __n
> would
> take place of Q__, while the __nx function might be defined like this
> (borrowing from Locale::TextDomain), which needs to be put into
> Git::I18N:
> 
> # Plural with interpolation.
> sub __nx ($$$@)
> {
>     my ($msgid, $msgid_plural, $count, %args) = @_;
>     
>     return __expand ((__n $msgid, $msgid_plural, $count),
> 		     %args);
> }