From: Jakub Narebski <jnareb@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Jeff Epler" <jepler@unpythonic.net>,
"Jakub Narębski" <jnareb@gmail.com>
Subject: Re: [PATCH/RFC v2 5/6] gettext: Add a Gettext interface for Perl
Date: Tue, 01 Jun 2010 10:00:18 -0700 (PDT) [thread overview]
Message-ID: <m3iq62r8jn.fsf@localhost.localdomain> (raw)
In-Reply-To: <1275252857-21593-6-git-send-email-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Make Git's gettext messages available to Perl programs through
> Locale::Messages. Gracefully fall back to English on systems that
> don't contain the module.
This is I think a very good idea, both providing wrapper with fallback
to untranslated messages (and encapsulating translation), and using the
Locale::Messages module from libintl-perl library. The "On the state of
i18n in Perl" (http://rassie.org/archives/247) blog post from 26 April
2009 provides nice counterpoint to Locale::Maketext::TPJ13 / The Perl
Journal #13 article[1] from 1999... especially because using gettext is
natural for translating git command output and GUI, because git uses it
already for Tcl/Tk (gitk and git-gui), and it is natural solution for
code in C, which slightly less than half of git code.
Well, we could use Locale::Maketext::Gettext, but it is not in Perl
core either, and as http://rassie.org/archives/247 says its '*.po'
files are less natural. The gettext documentation (gettext.info) also
recommends libintl-perl, or to be more exact Locale::TextDomain from
it.
[1] http://search.cpan.org/perldoc?Locale::Maketext::TPJ13
http://interglacial.com/~sburke/tpj/as_html/tpj13.html
The question is why not use Locale::TextDomain, the high-level Perl-y
framework, wrapper around Locale::Messages from the same libintl-perl
library? The gettext documentation (in gettext.info, chapter "13.5.18
Perl") says:
Prerequisite
`use POSIX;'
`use Locale::TextDomain;' (included in the package libintl-perl
which is available on the Comprehensive Perl Archive Network CPAN,
http://www.cpan.org/).
This would change
- print "Emails will be sent from: ", $sender, "\n";
+ printf gettext("Emails will be sent from: %s\n"), $sender;
to either
+ print __"Emails will be sent from: ", $sender, "\n";
or
+ printf __("Emails will be sent from: %s\n"), $sender;
or
+ print __x("Emails will be sent from: {sender}\n",
+ sender => $sender);
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> Makefile | 4 ++-
> git-send-email.perl | 3 +-
> perl/Git/Gettext.pm | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
> perl/Makefile.PL | 5 ++-
> 4 files changed, 92 insertions(+), 3 deletions(-)
> create mode 100644 perl/Git/Gettext.pm
>
> diff --git a/Makefile b/Makefile
> index dce2faa..2101713 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1884,7 +1884,9 @@ cscope:
> $(FIND) . -name '*.[hcS]' -print | xargs cscope -b
>
> pot:
> - $(XGETTEXT) -k_ -o po/git.pot $(C_OBJ:o=c) $(SCRIPT_SH)
> + $(XGETTEXT) --keyword=_ --output=po/git.pot --language=C $(C_OBJ:o=c)
> + $(XGETTEXT) --join-existing --output=po/git.pot --language=Shell $(SCRIPT_SH)
Shouldn't this line be in earlier patch, i.e. in "gettext: Add a
Gettext interface for shell scripts"?
> + $(XGETTEXT) --join-existing --output=po/git.pot --language=Perl $(SCRIPT_PERL)
From gettext documentation (in gettext.info, chapter "13.5.18 Perl"):
Extractor
`xgettext -k__ -k\$__ -k%__ -k__x -k__n:1,2 -k__nx:1,2 -k__xn:1,2
-kN__ -k'
Is it equivalent to specifying 'xgettext --language=Perl'?
Of course the above assumes that you are using Locale::TextDomain, or
at least use the same conventions.
>
> POFILES := $(wildcard po/*.po)
> MOFILES := $(patsubst po/%.po,share/locale/%/LC_MESSAGES/git.mo,$(POFILES))
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 111c981..a36718e 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -26,6 +26,7 @@ use Term::ANSIColor;
> use File::Temp qw/ tempdir tempfile /;
> use Error qw(:try);
> use Git;
> +use Git::Gettext qw< :all >;
Please follow the existing convention, i.e.
+use Git::Gettext qw(:all);
like you see in context line for 'use Error'.
Well, not that git-send-email.perl is very consistent about this
issue, see 'use File::Temp qw/ tempdir tempfile /;' versus
'use Error qw(:try);' but I'd reather you didn't introduce yet
another form.
>
> Getopt::Long::Configure qw/ pass_through /;
>
> @@ -674,7 +675,7 @@ if (!defined $sender) {
> $sender = $repoauthor || $repocommitter || '';
> $sender = ask("Who should the emails appear to be from? [$sender] ",
> default => $sender);
> - print "Emails will be sent from: ", $sender, "\n";
> + printf gettext("Emails will be sent from: %s\n"), $sender;
As I wrote, this IMHO should be either
+ printf __("Emails will be sent from: %s\n"), $sender;
or
+ print __x("Emails will be sent from: {sender}\n", sender => $sender);
(note that parantheses differ in those two examples).
> $prompting++;
> }
>
> diff --git a/perl/Git/Gettext.pm b/perl/Git/Gettext.pm
> new file mode 100644
> index 0000000..f434783
> --- /dev/null
> +++ b/perl/Git/Gettext.pm
> @@ -0,0 +1,83 @@
> +package Git::Gettext;
Should this package be named Git::Gettext, or other name would be
better, perhaps Git::I18N (like e.g. Games::Risk have
Games::Risk::I18N), or Git::Locale, or even Git::Translator?
Not very important.
> +use strict;
> +use warnings;
> +use Exporter;
> +use base 'Exporter';
O.K.
The alternative would be to use
+use Exporter qw(import);
> +
> +our $VERSION = '0.01';
> +
> +our @EXPORT;
> +our @EXPORT_OK = qw< gettext >;
Same comment as above: more common way is to use qw(gettext) not
qw< gettext >; it is also the notation used in Exporter documentation.
> +our %EXPORT_TAGS;
> +@{ $EXPORT_TAGS{'all'} } = @EXPORT_OK;
Why not simply
+our %EXPORT_TAGS = ('all' => [ @EXPORT_OK ]);
or the reverse
+our %EXPORT_TAGS = ('all' => [qw(gettext)]);
+our @EXPORT_OK = @{$EXPORT_TAGS{'all'}};
or the reverse using tag utility functions
+our %EXPORT_TAGS = ('all' => [qw(gettext)]);
+Exporter::export_ok_tags('all');
> +
> +sub __bootstrap_locale_messages {
> + our $TEXTDOMAIN = 'git';
> +
> + # TODO: How do I make the sed replacements in the top level
> + # Makefile reach me here?
> + #our $TEXTDOMAINDIR = q|@@LOCALEDIR@@|;
In Perl (well, in gitweb/gitweb.perl) we use '++VAR++' and not
'@@VAR@@' for placeholders, because '@' is sigil in Perl. This is not
important in above example, because it is not interpolated string.
Make invoked on perl/Makefile, when invoked from main Makefile by
'$(MAKE) -C perl' (via QUIET_SUBDIR0) passes 'localedir' to submake;
perl/Makefile should probably have something like
localedir ?= $(sharedir)/locale
That is assuming that 'localedir' is added to list of exported
variables.
But I am not sure how such substitution should be performed.
> + our $TEXTDOMAINDIR = q</usr/local/share/locale>;
Why q<...> and not simply '...'?
> +
> + require POSIX;
> + POSIX->import(qw< setlocale >);
> + # Non-core prerequisite module
> + require Locale::Messages;
> + Locale::Messages->import(qw< :locale_h :libintl_h >);
> +
> + setlocale(LC_MESSAGES(), '');
> + setlocale(LC_CTYPE(), '');
> + textdomain($TEXTDOMAIN);
> + bindtextdomain($TEXTDOMAIN => $TEXTDOMAINDIR);
> +
> + return;
> +}
This would probably be a bit simpler with Locale::TextDomain.
> +
> +BEGIN
> +{
> + local ($@, $!);
> + eval { __bootstrap_locale_messages() };
> + if ($@) {
> + # Oh noes, no Locale::Messages here
> + *gettext = sub ($) { $_[0] };
Do you intended to use subroutine protytype here? Ah, I see that
Locale::Messages::gettext has the same prototype...
> + }
> +}
Does it need to be in BEGIN block? Probably yes.
> +
> +1;
> +
> +__END__
> +
It's nice that you have provided documentation.
> +=head1 NAME
> +
> +Git::Gettext - Perl interface to Git's Gettext localizations
> +
> +=head1 DESCRIPTION
> +
> +Git's internal interface to Gettext via L<Locale::Messages>. If
> +L<Locale::Messages> can't be loaded (it's not a core module) we
> +provide stub passthrough fallbacks.
Very good.
It would probably be better though to use L<Locale::TextDomain>
instead of low(er)-level L<Locale::Messages>.
> +
> +=head1 FUNCTIONS
> +
> +=head2 gettext($)
> +
> +L<Locale::Messages>'s gettext function if all goes well, otherwise our
> +passthrough fallback function.
Other packages use _T() function for that, or like Locale::TextDomain
__() function.
> +
> +=head1 EXPORTS
> +
> +Exports are done via L<Exporter>. Invididual functions can be
> +exporter, or all of them via the C<:all> export tag.
Shouldn't this be described in less technical way in SYNOPSIS and
DESCRIPTION sections instead?
> +
> +=head1 AUTHOR
> +
> +E<AElig>var ArnfjE<ouml>rE<eth> Bjarmason <avarab@gmail.com>
> +
> +=head1 LICENSE AND COPYRIGHT
> +
> +Copyright 2010 E<AElig>var ArnfjE<ouml>rE<eth> Bjarmason <avarab@gmail.com>
> +
> +This program is free software, you can redistribute it and/or modify
> +it under the same terms as Perl itself.
Which is dual licensed: GPL + Perl artistic.
Was it intended to use 'same terms as Perl itself' rather than GPLv2
or GPLv2+?
> +
> +=cut
> diff --git a/perl/Makefile.PL b/perl/Makefile.PL
> index 0b9deca..702ec7c 100644
> --- a/perl/Makefile.PL
> +++ b/perl/Makefile.PL
> @@ -16,7 +16,10 @@ endif
> MAKE_FRAG
> }
>
> -my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');
> +my %pm = (
> + 'Git.pm' => '$(INST_LIBDIR)/Git.pm',
> + 'Git/Gettext.pm' => '$(INST_LIBDIR)/Git/Gettext.pm',
> +);
>
> # We come with our own bundled Error.pm. It's not in the set of default
> # Perl modules so install it if it's not available on the system yet.
> --
> 1.7.1.248.gcd6d1
>
--
Jakub Narebski
Poland
ShadeHawk on #git
next prev parent reply other threads:[~2010-06-01 17:23 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-29 22:45 [PATCH/RFC 0/5] Add internationalization support to Git Ævar Arnfjörð Bjarmason
2010-05-29 22:45 ` [PATCH 1/5] Add infrastructure for translating Git with gettext Ævar Arnfjörð Bjarmason
2010-05-29 22:45 ` [PATCH 2/5] gitignore: Ignore files generated by gettext Ævar Arnfjörð Bjarmason
2010-05-29 22:45 ` [PATCH 3/5] Makefile: Remove Gettext files on make clean Ævar Arnfjörð Bjarmason
2010-05-29 22:45 ` [PATCH 4/5] gettext: Add a skeleton po/is.po Ævar Arnfjörð Bjarmason
2010-05-29 22:45 ` [PATCH 5/5] Add infrastructure to make shellscripts translatable Ævar Arnfjörð Bjarmason
2010-05-30 1:46 ` [PATCH/RFC 0/5] Add internationalization support to Git Jonathan Nieder
2010-05-30 16:04 ` Ævar Arnfjörð Bjarmason
2010-05-30 22:23 ` Jonathan Nieder
2010-05-31 12:17 ` Ævar Arnfjörð Bjarmason
2010-05-30 20:54 ` [PATCH/RFC v2 0/6] " Ævar Arnfjörð Bjarmason
2010-05-30 20:54 ` [PATCH/RFC v2 1/6] Add infrastructure for translating Git with gettext Ævar Arnfjörð Bjarmason
2010-06-01 17:01 ` Jakub Narebski
2010-06-01 18:11 ` [PATCH/RCF] autoconf: Check if <libintl.h> exists and set NO_GETTEXT Ævar Arnfjörð Bjarmason
2010-06-01 21:22 ` Jakub Narebski
2010-05-30 20:54 ` [PATCH/RFC v2 2/6] gitignore: Ignore files generated by gettext Ævar Arnfjörð Bjarmason
2010-05-30 20:54 ` [PATCH/RFC v2 3/6] Makefile: Remove Gettext files on make clean Ævar Arnfjörð Bjarmason
2010-05-30 20:54 ` [PATCH/RFC v2 4/6] gettext: Add a Gettext interface for shell scripts Ævar Arnfjörð Bjarmason
2010-05-30 20:54 ` [PATCH/RFC v2 5/6] gettext: Add a Gettext interface for Perl Ævar Arnfjörð Bjarmason
2010-06-01 17:00 ` Jakub Narebski [this message]
2010-06-01 19:06 ` Ævar Arnfjörð Bjarmason
2010-06-02 11:47 ` Jakub Narebski
2010-05-30 20:54 ` [PATCH/RFC v2 6/6] gettext: Add a skeleton po/is.po Ævar Arnfjörð Bjarmason
2010-05-30 21:29 ` Jonathan Nieder
2010-05-30 21:39 ` Jonathan Nieder
2010-05-31 14:17 ` Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 0/7] Add internationalization support to Git Ævar Arnfjörð Bjarmason
2010-06-02 0:11 ` Ævar Arnfjörð Bjarmason
2010-06-02 1:05 ` [PATCH/RFC v4 " Ævar Arnfjörð Bjarmason
2010-06-02 1:05 ` [PATCH/RFC v4 1/7] Add infrastructure for translating Git with gettext Ævar Arnfjörð Bjarmason
2010-06-02 9:12 ` Peter Krefting
2010-06-02 9:29 ` Ævar Arnfjörð Bjarmason
2010-06-02 10:11 ` Peter Krefting
2010-06-02 10:56 ` Ævar Arnfjörð Bjarmason
2010-06-02 11:31 ` Peter Krefting
2010-06-02 1:05 ` [PATCH/RFC v4 2/7] gettext: Add a Gettext interface for shell scripts Ævar Arnfjörð Bjarmason
2010-06-02 1:06 ` [PATCH/RFC v4 3/7] gettext: Add a Gettext interface for Perl Ævar Arnfjörð Bjarmason
2010-06-02 1:06 ` [PATCH/RFC v4 4/7] Makefile: Don't install Gettext .mo files if NO_GETTEXT Ævar Arnfjörð Bjarmason
2010-06-02 1:06 ` [PATCH/RFC v4 5/7] Makefile: Override --keyword= for all languages Ævar Arnfjörð Bjarmason
2010-06-02 1:06 ` [PATCH/RFC v4 6/7] gettext: Sanity tests for Git's Gettext support Ævar Arnfjörð Bjarmason
2010-06-02 1:06 ` [PATCH/RFC v4 7/7] gettext: Add a skeleton po/is.po Ævar Arnfjörð Bjarmason
2010-06-02 6:32 ` [PATCH/RFC v3 0/7] Add internationalization support to Git Johannes Sixt
2010-06-02 22:33 ` [PATCH/RFC v5 0/2] Add infrastructure for translating Git with gettext Ævar Arnfjörð Bjarmason
2010-06-02 22:33 ` [PATCH/RFC v5 1/2] " Ævar Arnfjörð Bjarmason
2010-06-02 22:33 ` [PATCH/RFC v5 2/2] Add initial C, Shell and Perl gettext translations Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 1/7] Add infrastructure for translating Git with gettext Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 2/7] gettext: Add a Gettext interface for shell scripts Ævar Arnfjörð Bjarmason
2010-06-02 6:32 ` Johannes Sixt
2010-06-02 8:53 ` Ævar Arnfjörð Bjarmason
2010-06-02 9:38 ` Johannes Sixt
2010-06-01 23:39 ` [PATCH/RFC v3 3/7] gettext: Add a Gettext interface for Perl Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 4/7] Makefile: Don't install Gettext .mo files if NO_GETTEXT Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 5/7] Makefile: Override --keyword= for all languages Ævar Arnfjörð Bjarmason
2010-06-01 23:39 ` [PATCH/RFC v3 6/7] gettext: Basic sanity tests for Git's Gettext support Ævar Arnfjörð Bjarmason
2010-06-02 6:32 ` Johannes Sixt
2010-06-01 23:39 ` [PATCH/RFC v3 7/7] gettext: Add a skeleton po/is.po Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3iq62r8jn.fsf@localhost.localdomain \
--to=jnareb@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=jepler@unpythonic.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).