Ævar Arnfjörð Bjarmason [off-list ref] writes:
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?
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/;
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.
quoted hunk
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 hunk
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?
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
*.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.
Ævar Arnfjörð Bjarmason wrote:
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
This has $(prefix)/ (e.g., /usr/) at the start.
* 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
This does not.
* 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, makes sense. bin-wrappers/git has to get its locales from
somewhere.
On Wed, Nov 16, 2011 at 11:33, Jonathan Nieder [off-list ref] wrote:
Ævar Arnfjörð Bjarmason wrote:
quoted
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
This has $(prefix)/ (e.g., /usr/) at the start.
Yes I'm an idiot. I confused the installation part with stuff we
actually make in the git compilation directory.
The i18n code is indeed the onle thing that uses "share" in the
compilation directory.
Thanks for spotting that.
Ævar Arnfjörð Bjarmason wrote:
Yes I'm an idiot. I confused the installation part with stuff we
actually make in the git compilation directory.
The i18n code is indeed the onle thing that uses "share" in the
compilation directory.
Most of your message didn't seem idiotic. In particular, putting the
message catalogs somewhere less confusingly named than
$(srcdir)/share, like $(srcdir)/po/generated or
$(srcdir)/textdomaindir.built, makes a lot of sense to me.