Re: [PATCH] i18n: add infrastructure for translating Git with gettext

On Tue, Nov 15, 2011 at 09:04, Junio C Hamano [off-list ref] wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

Change the skeleton implementation of i18n in Git to one that can show
localized strings to users for our C, Shell and Perl programs using
either GNU libint or the Solaris gettext implementation.

Happy ;-).

Isn't it libintl, though?

quoted

This new internationalization support is enabled by default. If
gettext isn't available, or if Git is compiled with
NO_GETTEXT=YesPlease, Git fall back on its current behavior of showing

s/fall/falls/;

I'll fix both of these speling errurs in a new version of the
patch.

But I won't send it for at least a few days. I don't think there's
much point as we're in no rush to apply this, and it might as well sit
on-list for a few days to accumulate more review.

quoted

This change is somewhat large because as well as adding a C, Shell and
Perl i18n interface we're adding a lot of tests for them, and for
those tests to work we need a skeleton PO file to actually test
translations.

You _could_ split it up this way, I think, if you really wanted to:

 * The mechanism, i.e. integration with libintl and gettext, without any
  translated strings but with the .pot file, with tests to make sure in a
  locale that is missing *.mo files for it, we get the default output;

 * Sample translation for is_IS locale, with tests to make sure that we
  get translated output in a locale that has *.mo files for it.

But I am not sure if that is better.

I think it's better to keep it in one large patch. The changes to the
core code are pretty minimal, most of what it's adding is new stuff
that's only ever used if i18n itself is used.

quoted

diff --git a/.gitignore b/.gitignore
index 8572c8c..c47f3a8 100644
--- a/.gitignore
+++ b/.gitignore

@@ -224,3 +224,4 @@

 *.pdb
 /Debug/
 /Release/
+/share/

I somehow find it distasteful that this is overly wide; same with the
change in Makefile to "rm -rf share/". I suspect it wouldn't be the case
if it were limited to share/locale/ or something.

But perhaps we would never ship anything in share/ and things in there
will always come from elsewhere.

quoted

diff --git a/Makefile b/Makefile
index ee34eab..896f5fd 100644
--- a/Makefile
+++ b/Makefile

...

@@ -2435,6 +2507,7 @@ clean:

      $(RM) $(TEST_PROGRAMS)
      $(RM) -r bin-wrappers
      $(RM) -r $(dep_dirs)
+     $(RM) -r po/git.pot share/

It seems "distclean" tells us to clean po/git.pot, which hints at me that
normal "clean" shouldn't?

I'm not quite sure what we should do with it.

Here's what we do with the sharedir currently:

 * It's used as the gitwebdir already, and is used also as the
   localedir with this patch:

    sharedir = $(prefix)/share
    gitwebdir = $(sharedir)/gitweb
    localedir = $(sharedir)/locale

 * Even then I can't find the option that makes gitweb put something
   in it, after a "make" with this patch:

    $ find share -type f
    share/locale/is/LC_MESSAGES/git.mo

 * It *is* important that we have this under a
   $SOMETHING/is/LC_MESSAGES/git.mo structure, since we set
   GIT_TEXTDOMAINDIR to $SOMETHING, and the gettext library will only
   understand something in this layout.

   But that $SOMETHING doesn't have to be $ROOT/share, it could be
   e.g. $ROOT/po/generated, which would make it not conflict with the
   gitwebdir.

 * Yeah, we shouldn't remove po/git.pot in "clean" as well as
   "distclean". I missed that it had been added in 92a684b916 while
   applying this patch.