Re: [PATCH] Internationalization of git-gui

From: Christian Stimming <stimming@tuhh.de>
Date: Jul 21, 2007, 07:50 UTC
Message-ID: <200707210951.00210.stimming@tuhh.de>
In-Reply-To: <20070721021717.GS32566@spearce.org>

Thanks for the detailed feedback.

Am Samstag, 21. Juli 2007 04:17 schrieb Shawn O. Pearce:

Show 7 quoted lines

> Christian Stimming <stimming@tuhh.de> wrote:
> > I used (and prefer) "_" because that's the standard function name for
> > i18n'd strings when using gettext (talking about a "standard" way).
>
> but I don't think its worth the CPU cycles in
> Tcl to eval mc via _ every time we need a string when it only is
> saving us one keystroke on a function name,

The actual lookup of the string in the translation catalog far outweighs the discussion of one extra function evaluation, but if that's what you prefer -

> *and* we are breaking  tradition with Tcl.
>
> So when in Rome, wear a toga.  Or in this case, use [mc ...].

Ok.

Show 7 quoted lines

> > Do you want to have patches
> > submitted after some further changes (which ones?)
>
> Yes.  Here's a few to get started with and that are really obvious.
> Some I'm just asking for more information on.
>
>  - Import msgcat::mc and use [mc] instead of [_].

Will do.

>  - Please combine the second and third patches into a single change.
>  There is no reason to switch to [mc {}] only to switch to [mc ""].

Will do.

>  - Please use mc's formatting support, rather than [format].
>  Its shorter code.

Didn't know about that (you know, C gettext doesn't have that), and I'll have to check whether this might confuse xgettext on string extraction, but will probably be done.

Show 5 quoted lines

>  - Don't bother trying to translate the strings "Tools" (for the
>  Tools menu) or "Migrate" (for its only menu option).  This block
>  of code doesn't even belong in git-gui.  Its for my day-job and
>  is a custom hack that I need to strip out and carry as a local
>  patch there, rather than in the public distribution.

Err... from looking at the code, it's not quite clear to me whether any code parts like these are not publicly shown to the user. For that reason I rather translated everything that was currently available in the file. But I'll happily remove this from the patch.

>  - In our Makefile we do the looping in GNU make using its
>  $(foreach) operator, rather than using the shell's for builtin.
>  In other words, can we have the catalog target look more like the
>  install target?

Sure. I did it this way because I'm more used to the shell syntax, but I'll change that.

>  - Can ALL_LINGUAS be automatically built from the directory
>  contents of the po/ directory?

Yes. I used this one because this variable always appears in gettext's autoconf infrastructure, but it's not required here. Will remove it.

>  - Can we define a dist rule for the maintainer to build the catalog
>  files, so the maintainer can convert the .po -> .msg for Tcl and
>  the user doesn't need the GNU tools installed to build git-gui?

Yes, I'll try to add that, but I'd need further feedback and testing whether it actually works.

Show 5 quoted lines

> > and/or in different
> > formats?
>
> Please send one patch per email message, inline and not attached.
> This way they are easy to review, respond to and comment on.

I'll try to do that, but at the workplace where I work on this issue I'm forced to use a webmailer and I have to check whether this leaves the patches intact.

Show 6 quoted lines

> > Do you prefer to have all changes in a smaller number of
> > commit rather than split the way I did before?
>
> No, this series looks reasonably fine to me structurally.
>
> Did you base the patches on git.git's git-gui/ subdirectory,

Yes.

> or 
> did you base them on the git-gui.git repository?  Technically all
> patches for git-gui should be against the git-gui repository on
> repo.or.cz, as git-gui is its own project.

Will do.

Show 7 quoted lines

> Dscho recently created a fork of git-gui.git here:
>
>   http://repo.or.cz/w/git-gui/git-gui-i18n.git
>
> and added your patch series into it.  But I'd like to see some
> cleanups before it merges in, and I want to hold off on actually
> applying it into git-gui 0.8.0 is released,

Was this meant to say "hold off until git-gui 0.8.0 is released"? Sure, no problem.

I'll submit an updated set of patches by beginning of next week. Thank you very much for the feedback.

Christian