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

[Jakub Narębski]

W dniu 26.09.2016 o 00:52, Junio C Hamano pisze:
> Vasco Almeida  writes: 

>>  my $status_fmt = '%12s %12s %s';
>> -my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
>> +my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), 
>> __('path'));
> 
> Wouldn't it make sense to allow translators to tweak $status_fmt if
> you are allowing the earlier elements that are formatted with %12s,
> as their translation may not fit within that width, in which case
> they may want to make these columns wider?

Perl's printf, sprintf, and format think all codepoints take up 1 print
column; also, without "use utf8;" they all think that one byte is one
codepoint (as it is in latin1 encoding).

Many codepoints can take 0 print columns (zero-width joiners), or 2
columns (so called wide characters).

The proper way to justify Unicode output is described e.g. in
http://www.perl.com/pub/2012/05/perlunicook-unicode-column-width-for-printing.html

  use Unicode::GCString;

  my $gcs  = Unicode::GCString->new($str);  # grapheme cluster string
  my $cols = $gcs->columns;
  my $pad  = " " x (12 - $cols);

  $status_head .= $str . $pad . " ";

Though we would need to provide fallback if there is no perl-i18n,
no extended Unicode support in Perl (also, if we are not using
gettext).


So it is even more complicated.

>>  prompt_yesno(
>> -'Your edited hunk does not apply. Edit again '
>> -. '(saying "no" discards!) [y/n]? '
>> +# TRANSLATORS: do not translate [y/n]
>> +# The program will only accept that input
>> +# at this point.
>> +__('Your edited hunk does not apply. Edit again 
>> '
>> +   . '(saying "no" discards!) [y/n]? ')
> 
> Not just [y/n], but "no" in "saying no discards!" also needs to
> stay, no?  I wonder if it is a good idea to lose the TRANSLATORS
> comment by ejecting "[y/n]" outside the "__()" construct here.

Actually the message to translators should also mention that if
the translation of "no" doesn't begin with 'n', then one needs
to say something like '(saying "n" for "no" discards!)'.

Best,
-- 
Jakub Narębski

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

[Junio C Hamano]

Vasco Almeida  writes:

> As far as I understand, %12s means that the argument printed will have
> a minimum length of 12 columns. So if the translation of 'stage' is
> longer than 12 it will be printed fully no matter what. Though in that
> case, the header will not be align correctly anymore:

Exactly.  That was where my suggestion comes from.  In such a case
you may want to raise these numbers so that the fixed part
(i.e. header that you are letting the translators insert their
version of these words) would fit.

As Duy points out in his response to your message, that widening
further needs to take into account how many display columns each
translated words and phrases occupies, not just its byte length.

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

[Duy Nguyen]

On Wed, Sep 28, 2016 at 7:43 PM, Vasco Almeida  wrote:
> A Dom, 25-09-2016 às 15:52 -0700, Junio C Hamano escreveu:
>> > @@ -252,7 +253,7 @@ sub list_untracked {
>> >  }
>> >
>> >  my $status_fmt = '%12s %12s %s';
>> > -my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
>> > +my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), 
>> > __('path'));
>>
>> Wouldn't it make sense to allow translators to tweak $status_fmt if
>> you are allowing the earlier elements that are formatted with %12s,
>> as their translation may not fit within that width, in which case
>> they may want to make these columns wider?
>
> As far as I understand, %12s means that the argument printed will have
> a minimum length of 12 columns. So if the translation of 'stage' is
> longer than 12 it will be printed fully no matter what. Though in that
> case, the header will not be align correctly anymore:
> for other instances of this in the present patch series.

It's 12 bytes, not columns (unless perl understands input string's
encoding, which I doubt). Think about multi-byte encodings like utf-8,
where three letters (or "columns") do not necessary mean three bytes.
The result is most likely unaligned in that case.
-- 
Duy

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

[Vasco Almeida]

A Dom, 25-09-2016 às 15:52 -0700, Junio C Hamano escreveu:
> > @@ -252,7 +253,7 @@ sub list_untracked {
> >  }
> >  
> >  my $status_fmt = '%12s %12s %s';
> > -my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
> > +my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), 
> > __('path'));
> 
> Wouldn't it make sense to allow translators to tweak $status_fmt if
> you are allowing the earlier elements that are formatted with %12s,
> as their translation may not fit within that width, in which case
> they may want to make these columns wider?

As far as I understand, %12s means that the argument printed will have
a minimum length of 12 columns. So if the translation of 'stage' is
longer than 12 it will be printed fully no matter what. Though in that
case, the header will not be align correctly anymore:

my $status_fmt = '%12s %12s %s';

     123456789abcdefghijkl unstaged caminho
  1:unchanged+1/-0 git-add--interactive.perl
  2:unchanged  +4226/-3152 po/git.pot
  3:unchanged +11542/-10426 po/pt_PT.po


my $status_fmt = '%12s %8s %s';

     123456789abcdefghijkl unstaged caminho
  1:unchanged+2/-1 git-add--interactive.perl
  2:unchanged +4226/-3152 po/git.pot
  3:unchanged +11542/-10426 po/pt_PT.po


For reference in C locale (my $status_fmt = '%12s %12s %s';)

           staged unstaged path
  1:+4/-4  nothing git-add--interactive.perl
  2:unchanged  +4232/-3150 po/git.pot
  3:unchanged +11572/-10448 po/pt_PT.po

I did not contemplate this issue before, but I think allowing a
translator to tweak $status_fmt would not be enough to properly align
the header if the translation is longer than 12 columns.

Maybe a lazy solution would be to add a TRANSLATOR: comment asking to
fit the translation of those words in 12 columns, but that would be
unpractical because 'stage' and 'unstage' can occur alone, like they do
here, in other place in the future, without having that length
restriction.

I think the real fix would be to find the longer column and use that
length for the remaining rows. I will try to do that if I can.


I also forgot to mark strings 'unchanged' and 'nothing' that are
displayed on that status. I will mark then in the next re-roll.

> >   prompt_yesno(
> > - 'Your edited hunk does not apply. Edit again '
> > - . '(saying "no" discards!) [y/n]? '
> > + # TRANSLATORS: do not translate [y/n]
> > + # The program will only accept that input
> > + # at this point.
> > + __('Your edited hunk does not apply. Edit 
> > again '
> > +    . '(saying "no" discards!) [y/n]? ')
> 
> Not just [y/n], but "no" in "saying no discards!" also needs to
> stay, no?  I wonder if it is a good idea to lose the TRANSLATORS
> comment by ejecting "[y/n]" outside the "__()" construct here.

I fear that ejecting "[y/n]" would not be good for right-to-left
languages since "[y/n]" would be the first thing a user of those
languages would read followed by the actual question. I feel the same
for other instances of this in the present patch series.

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

[Junio C Hamano]

Vasco Almeida  writes:

> Mark simple strings (without interpolation) for translation.
>
> Brackets around first parameter of ternary operator is necessary because
> otherwise xgettext fails to extract strings marked for translation from
> the rest of the file.
>
> Signed-off-by: Vasco Almeida 
> ---
>  git-add--interactive.perl | 68 
> +--
>  1 file changed, 36 insertions(+), 32 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 822f857..fb8e5de 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -4,6 +4,7 @@ use 5.008;
>  use strict;
>  use warnings;
>  use Git;
> +use Git::I18N;
>  
>  binmode(STDOUT, ":raw");
>  
> @@ -252,7 +253,7 @@ sub list_untracked {
>  }
>  
>  my $status_fmt = '%12s %12s %s';
> -my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
> +my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), 
> __('path'));

Wouldn't it make sense to allow translators to tweak $status_fmt if
you are allowing the earlier elements that are formatted with %12s,
as their translation may not fit within that width, in which case
they may want to make these columns wider?

>   prompt_yesno(
> - 'Your edited hunk does not apply. Edit again '
> - . '(saying "no" discards!) [y/n]? '
> + # TRANSLATORS: do not translate [y/n]
> + # The program will only accept that input
> + # at this point.
> + __('Your edited hunk does not apply. Edit again 
> '
> +. '(saying "no" discards!) [y/n]? ')

Not just [y/n], but "no" in "saying no discards!" also needs to
stay, no?  I wonder if it is a good idea to lose the TRANSLATORS
comment by ejecting "[y/n]" outside the "__()" construct here.