On Tue, Nov 15, 2011 at 09:04, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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/;
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.
>> 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.
>> 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.
>
>> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html