Re: [PATCH/RFC 00/17] Begin gettextizing Git

On Tue, Aug 31, 2010 at 18:08, Jonathan Nieder [off-list ref] wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

Or: git://github.com/avar/git.git gettextize-git-mainporcelain-v2

Is that against "next"?  For an RFC that's okay, but for a series
that will be part of a release it can be tedious to separate out
(not all topics in "next" necessarily are released, and "next"
traditionally gets rewound after each release).

Yes it's against next since it had the gettext series merged and next
is about to be released isn't it? I could base it on something else if
that's appropriate.

Comments on the patches:

 Makefile: use variables and shorter lines for xgettext

The -o<whatever> passed to cc isn't usually included in CFLAGS,
and a part of me is similarly uncomfortable with including it in
XGETTEXT_OPTIONS.  Isn't that parameter something that should be
possible to change in the build system independently from the
user's XGETTEXT_OPTIONS preferences?

Maybe, but it'll always be --output=po/git.pot so I saw no reason to
seperate it. Should it be? The -o for the C compiler changes, but the
"make pot" target will always write to po/git.pot.

 gettext.c: work around us not using setlocale(LC_CTYPE, "")

The perror() problem shows up with strerror(), too, of course.
(perror just made for an easier demo.)

Yeah, and everything external like that, unfortunately.

 gettext: Make NO_GETTEXT=YesPlease the default in releases

Copy-edits for the notes in INSTALL:

quoted

+       - Git includes EXPERIMENTAL support for localization with gettext
+         which is currently disabled by default in official Git
+         releases.

s/EXPERIMENTAL/experimental/?  No need to shout.

OKEY THEN!

I'd also s/currently // since this will not be current after a while.

ok.

quoted

+         If you really want to build it you have to specify NO_GETTEXT=
+         as a Makefile argument. If you're a downstream distributor
+         please don't do so without consulting with the Git Mailing List
+         first about the stability of this feature.

Similarly I'd s/really //.  If we want to dissuade people from trying
it out, we should probably do that with more explicit statements.

Thanks.

quoted

+         It's only being included in releases so that source messages can
+         be marked for translation without resulting in painful and
+         inevitable merge conflicts between Git's pu branch and the
+         rest. END WARNING.

Not sure what this means.  Maybe:

 The infrastructure is only included in this release to avoid
 complications in building other work on top of it.  If you turn
 it on, expect breakage.

Yes, that's better. I've worked all these into a
gettextize-git-mainporcelain-v3 branch at the same repository:

    http://github.com/avar/git/compare/gettextize-git-mainporcelain-v2...gettextize-git-mainporcelain-v3

Or: git://github.com/avar/git.git gettextize-git-mainporcelain-v3