git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC v2 5/6] gettext: Add a Gettext interface for Perl

From
Jakub Narebski <jnareb@gmail.com>
Date
Jun 1, 2010, 17:00 UTC
Message-ID
<m3iq62r8jn.fsf@localhost.localdomain>
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);
Show 21 quoted lines
> 
> 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.

Show 12 quoted lines
>  
>  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.

Show 9 quoted lines
>  
>  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).
Show 10 quoted lines
>  	$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);
Show 5 quoted lines
> +
> +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');
Show 7 quoted lines
> +
> +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 '...'?
Show 14 quoted lines
> +
> +	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.
Show 8 quoted lines
> +
> +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.
Show 5 quoted lines
> +
> +1;
> +
> +__END__
> +
It's nice that you have provided documentation.
Show 9 quoted lines
> +=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>.

Show 7 quoted lines
> +
> +=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.

Show 5 quoted lines
> +
> +=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?

Show 11 quoted lines
> +
> +=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+?

Show 21 quoted lines
> +
> +=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
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 20 of 56 in “Add internationalization support to Git”
  1. 0/5 Add internationalization support to GitÆvar Arnfjörð Bjarmason, May 29, 2010
  2. 1/5 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, May 29, 2010
  3. 2/5 gitignore: Ignore files generated by gettextÆvar Arnfjörð Bjarmason, May 29, 2010
  4. 3/5 Makefile: Remove Gettext files on make cleanÆvar Arnfjörð Bjarmason, May 29, 2010
  5. 4/5 gettext: Add a skeleton po/is.poÆvar Arnfjörð Bjarmason, May 29, 2010
  6. 5/5 Add infrastructure to make shellscripts translatableÆvar Arnfjörð Bjarmason, May 29, 2010
  7. Jonathan NiederMay 30, 2010
  8. Ævar Arnfjörð BjarmasonMay 30, 2010
  9. Jonathan NiederMay 30, 2010
  10. Ævar Arnfjörð BjarmasonMay 31, 2010
  11. 0/6 Add internationalization support to GitÆvar Arnfjörð Bjarmason, May 30, 2010
  12. 1/6 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, May 30, 2010
  13. Jakub NarebskiJun 1, 2010
  14. autoconf: Check if <libintl.h> exists and set NO_GETTEXTÆvar Arnfjörð Bjarmason, Jun 1, 2010
  15. Jakub NarebskiJun 1, 2010
  16. 2/6 gitignore: Ignore files generated by gettextÆvar Arnfjörð Bjarmason, May 30, 2010
  17. 3/6 Makefile: Remove Gettext files on make cleanÆvar Arnfjörð Bjarmason, May 30, 2010
  18. 4/6 gettext: Add a Gettext interface for shell scriptsÆvar Arnfjörð Bjarmason, May 30, 2010
  19. 5/6 gettext: Add a Gettext interface for PerlÆvar Arnfjörð Bjarmason, May 30, 2010
  20. Jakub NarebskiJun 1, 2010
  21. Ævar Arnfjörð BjarmasonJun 1, 2010
  22. Jakub NarebskiJun 2, 2010
  23. 6/6 gettext: Add a skeleton po/is.poÆvar Arnfjörð Bjarmason, May 30, 2010
  24. Jonathan NiederMay 30, 2010
  25. Jonathan NiederMay 30, 2010
  26. Ævar Arnfjörð BjarmasonMay 31, 2010
  27. 0/7 Add internationalization support to GitÆvar Arnfjörð Bjarmason, Jun 1, 2010
  28. Ævar Arnfjörð BjarmasonJun 2, 2010
  29. 0/7 Add internationalization support to GitÆvar Arnfjörð Bjarmason, Jun 2, 2010
  30. 1/7 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, Jun 2, 2010
  31. Peter KreftingJun 2, 2010
  32. Ævar Arnfjörð BjarmasonJun 2, 2010
  33. Peter KreftingJun 2, 2010
  34. Ævar Arnfjörð BjarmasonJun 2, 2010
  35. Peter KreftingJun 2, 2010
  36. 2/7 gettext: Add a Gettext interface for shell scriptsÆvar Arnfjörð Bjarmason, Jun 2, 2010
  37. 3/7 gettext: Add a Gettext interface for PerlÆvar Arnfjörð Bjarmason, Jun 2, 2010
  38. 4/7 Makefile: Don't install Gettext .mo files if NO_GETTEXTÆvar Arnfjörð Bjarmason, Jun 2, 2010
  39. 5/7 Makefile: Override --keyword= for all languagesÆvar Arnfjörð Bjarmason, Jun 2, 2010
  40. 6/7 gettext: Sanity tests for Git's Gettext supportÆvar Arnfjörð Bjarmason, Jun 2, 2010
  41. 7/7 gettext: Add a skeleton po/is.poÆvar Arnfjörð Bjarmason, Jun 2, 2010
  42. Johannes SixtJun 2, 2010
  43. 0/2 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, Jun 2, 2010
  44. 1/2 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, Jun 2, 2010
  45. 2/2 Add initial C, Shell and Perl gettext translationsÆvar Arnfjörð Bjarmason, Jun 2, 2010
  46. 1/7 Add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason, Jun 1, 2010
  47. 2/7 gettext: Add a Gettext interface for shell scriptsÆvar Arnfjörð Bjarmason, Jun 1, 2010
  48. Johannes SixtJun 2, 2010
  49. Ævar Arnfjörð BjarmasonJun 2, 2010
  50. Johannes SixtJun 2, 2010
  51. 3/7 gettext: Add a Gettext interface for PerlÆvar Arnfjörð Bjarmason, Jun 1, 2010
  52. 4/7 Makefile: Don't install Gettext .mo files if NO_GETTEXTÆvar Arnfjörð Bjarmason, Jun 1, 2010
  53. 5/7 Makefile: Override --keyword= for all languagesÆvar Arnfjörð Bjarmason, Jun 1, 2010
  54. 6/7 gettext: Basic sanity tests for Git's Gettext supportÆvar Arnfjörð Bjarmason, Jun 1, 2010
  55. Johannes SixtJun 2, 2010
  56. 7/7 gettext: Add a skeleton po/is.poÆvar Arnfjörð Bjarmason, Jun 1, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.