All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] WIP: begin to translate git with gettext
@ 2010-05-17 16:05 Jeff Epler
  2010-05-17 23:29 ` Robert Buck
  2010-05-18  7:40 ` Michael J Gruber
  0 siblings, 2 replies; 12+ messages in thread
From: Jeff Epler @ 2010-05-17 16:05 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Git Mailing List, Jakub Narebski, Dmitry Potapov, Jan Hudec,
	Thomas Rast, Marc Weber

Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
[resent with Cc to list and thread participants]

While I'm certain that there are a lot of things to object to in this
patch, it shows 90% of what is needed to use gettext to translate
the portions of git written in c, without involving undesired gnu
infrastructure such as automake.

Makefile adds necessary rules for generating git.pot and for building
and installing compiled message catalogs (.mo) from text message
catalogs (.po).  It also adds a gettext support header and source file.

Minimal changes are made to git to use the requested LC_CTYPE and
LC_MESSAGES, and some messages for 'git status' are marked for
translation.

When I provided a gibberish translation of a message:
#: wt-status.c:87
msgid "# Changed but not updated:"
msgstr "# Changes not blah blah blah"

running 'git status' used the translation:
$ git status
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
#
# Changes not blah blah blah
...

I ran with 'make install' and prefix set in config.mak.  It didn't seem
to work when running from the source directory, and it may or may not
work with runtime prefix.


 Makefile    |   26 +++++++++++++
 gettext.c   |   17 +++++++++
 gettext.h   |   15 ++++++++
 git.c       |    3 ++
 wt-status.c |  117 ++++++++++++++++++++++++++++++-----------------------------
 5 files changed, 120 insertions(+), 58 deletions(-)
 create mode 100644 gettext.c
 create mode 100644 gettext.h

diff --git a/Makefile b/Makefile
index 4f7224a..c02ca18 100644
--- a/Makefile
+++ b/Makefile
@@ -294,6 +294,8 @@ RPMBUILD = rpmbuild
 TCL_PATH = tclsh
 TCLTK_PATH = wish
 PTHREAD_LIBS = -lpthread
+XGETTEXT = xgettext
+MSGFMT = msgfmt
 
 export TCL_PATH TCLTK_PATH
 
@@ -518,6 +520,7 @@ LIB_H += userdiff.h
 LIB_H += utf8.h
 LIB_H += xdiff-interface.h
 LIB_H += xdiff/xdiff.h
+LIB_H += gettext.h
 
 LIB_OBJS += abspath.o
 LIB_OBJS += advice.o
@@ -559,6 +562,7 @@ LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
 LIB_OBJS += fsck.o
+LIB_OBJS += gettext.o
 LIB_OBJS += graph.o
 LIB_OBJS += grep.o
 LIB_OBJS += hash.o
@@ -1371,6 +1375,12 @@ ifdef USE_NED_ALLOCATOR
        COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
 endif
 
+ifdef NO_GETTEXT
+	COMPAT_CFLAGS += -DNO_GETTEXT
+else
+	LIBINTL = -lintl
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
@@ -1400,6 +1410,7 @@ ifndef V
 	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
 	QUIET_GEN      = @echo '   ' GEN $@;
 	QUIET_LNCP     = @echo '   ' LN/CP $@;
+	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
 	QUIET_SUBDIR0  = +@subdir=
 	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
@@ -1427,6 +1438,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_SQ = $(subst ','\'',$(htmldir))
 prefix_SQ = $(subst ','\'',$(prefix))
+sharedir_SQ = $(subst ','\'',$(sharedir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
@@ -1858,6 +1870,17 @@ cscope:
 	$(RM) cscope*
 	$(FIND) . -name '*.[hcS]' -print | xargs cscope -b
 
+pot:
+	$(XGETTEXT) -k_ -o po/git.pot $(C_OBJ:o=c)
+
+POFILES := $(wildcard po/*.po)
+MOFILES := $(patsubst po/%.po,share/locale/%/LC_MESSAGES/git.mo,$(POFILES))
+MODIRS := $(patsubst po/%.po,share/locale/%/LC_MESSAGES/,$(POFILES))
+all:: $(MOFILES)
+share/locale/%/LC_MESSAGES/git.mo: po/%.po
+	@mkdir -p $(dir $@)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
+
 ### Detect prefix changes
 TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
@@ -1970,6 +1993,9 @@ install: all
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(sharedir_SQ)/locale'
+	(cd share && tar cf - locale) | \
+		(cd '$(DESTDIR_SQ)$(sharedir_SQ)' && umask 022 && tar xof -)
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
diff --git a/gettext.c b/gettext.c
new file mode 100644
index 0000000..aadce19
--- /dev/null
+++ b/gettext.c
@@ -0,0 +1,17 @@
+#ifdef NO_GETTEXT
+void git_setup_gettext() {}
+#else
+#include "exec_cmd.h"
+#include <libintl.h>
+#include <stdlib.h>
+
+void git_setup_gettext() {
+    const char *podir = system_path("share/locale");
+    if(!podir) return;
+    char *ret = bindtextdomain("git", podir);
+    free((void*)podir);
+    ret = setlocale(LC_MESSAGES, "");
+    ret = setlocale(LC_CTYPE, "");
+    ret = textdomain("git");
+}
+#endif
diff --git a/gettext.h b/gettext.h
new file mode 100644
index 0000000..8b221b4
--- /dev/null
+++ b/gettext.h
@@ -0,0 +1,15 @@
+#ifndef GETTEXT_H
+#define GETTEXT_H
+
+void git_setup_gettext();
+
+#ifdef NO_GETTEXT
+#define _(s) (s)
+#define N_(s) (s)
+#else
+#include <libintl.h>
+#define _(s) gettext(s)
+#define N_(s) (s)
+#endif
+
+#endif
diff --git a/git.c b/git.c
index 6bae305..5e7aedd 100644
--- a/git.c
+++ b/git.c
@@ -3,6 +3,7 @@
 #include "cache.h"
 #include "quote.h"
 #include "run-command.h"
+#include "gettext.h"
 
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path]\n"
@@ -481,6 +482,8 @@ int main(int argc, const char **argv)
 	if (!cmd)
 		cmd = "git-help";
 
+	git_setup_gettext();
+
 	/*
 	 * "git-xxxx" is the same as "git xxxx", but we obviously:
 	 *
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..a31cbc6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "run-command.h"
 #include "remote.h"
+#include "gettext.h"
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
@@ -48,33 +49,33 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 
-	color_fprintf_ln(s->fp, c, "# Unmerged paths:");
+	color_fprintf_ln(s->fp, c, _("# Unmerged paths:"));
 	if (!advice_status_hints)
 		return;
 	if (s->in_merge)
 		;
 	else if (!s->is_initial)
-		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
+		color_fprintf_ln(s->fp, c, _("#   (use \"git reset %s <file>...\" to unstage)"), s->reference);
 	else
-		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
-	color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" as appropriate to mark resolution)");
-	color_fprintf_ln(s->fp, c, "#");
+		color_fprintf_ln(s->fp, c, _("#   (use \"git rm --cached <file>...\" to unstage)"));
+	color_fprintf_ln(s->fp, c, _("#   (use \"git add/rm <file>...\" as appropriate to mark resolution)"));
+	color_fprintf_ln(s->fp, c, _("#"));
 }
 
 static void wt_status_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 
-	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
+	color_fprintf_ln(s->fp, c, _("# Changes to be committed:"));
 	if (!advice_status_hints)
 		return;
 	if (s->in_merge)
 		; /* NEEDSWORK: use "git reset --unresolve"??? */
 	else if (!s->is_initial)
-		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
+		color_fprintf_ln(s->fp, c, _("#   (use \"git reset %s <file>...\" to unstage)"), s->reference);
 	else
-		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
-	color_fprintf_ln(s->fp, c, "#");
+		color_fprintf_ln(s->fp, c, _("#   (use \"git rm --cached <file>...\" to unstage)"));
+	color_fprintf_ln(s->fp, c, _("#"));
 }
 
 static void wt_status_print_dirty_header(struct wt_status *s,
@@ -83,32 +84,32 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 
-	color_fprintf_ln(s->fp, c, "# Changed but not updated:");
+	color_fprintf_ln(s->fp, c, _("# Changed but not updated:"));
 	if (!advice_status_hints)
 		return;
 	if (!has_deleted)
-		color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to update what will be committed)");
+		color_fprintf_ln(s->fp, c, _("#   (use \"git add <file>...\" to update what will be committed)"));
 	else
-		color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" to update what will be committed)");
-	color_fprintf_ln(s->fp, c, "#   (use \"git checkout -- <file>...\" to discard changes in working directory)");
+		color_fprintf_ln(s->fp, c, _("#   (use \"git add/rm <file>...\" to update what will be committed)"));
+	color_fprintf_ln(s->fp, c, _("#   (use \"git checkout -- <file>...\" to discard changes in working directory)"));
 	if (has_dirty_submodules)
-		color_fprintf_ln(s->fp, c, "#   (commit or discard the untracked or modified content in submodules)");
-	color_fprintf_ln(s->fp, c, "#");
+		color_fprintf_ln(s->fp, c, _("#   (commit or discard the untracked or modified content in submodules)"));
+	color_fprintf_ln(s->fp, c, _("#"));
 }
 
 static void wt_status_print_untracked_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
-	color_fprintf_ln(s->fp, c, "# Untracked files:");
+	color_fprintf_ln(s->fp, c, _("# Untracked files:"));
 	if (!advice_status_hints)
 		return;
-	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to include in what will be committed)");
-	color_fprintf_ln(s->fp, c, "#");
+	color_fprintf_ln(s->fp, c, _("#   (use \"git add <file>...\" to include in what will be committed)"));
+	color_fprintf_ln(s->fp, c, _("#"));
 }
 
 static void wt_status_print_trailer(struct wt_status *s)
 {
-	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
+	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
 }
 
 #define quote_path quote_path_relative
@@ -119,20 +120,20 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
 	const char *c = color(WT_STATUS_UNMERGED, s);
 	struct wt_status_change_data *d = it->util;
 	struct strbuf onebuf = STRBUF_INIT;
-	const char *one, *how = "bug";
+	const char *one, *how = _("bug");
 
 	one = quote_path(it->string, -1, &onebuf, s->prefix);
-	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
+	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));
 	switch (d->stagemask) {
-	case 1: how = "both deleted:"; break;
-	case 2: how = "added by us:"; break;
-	case 3: how = "deleted by them:"; break;
-	case 4: how = "added by them:"; break;
-	case 5: how = "deleted by us:"; break;
-	case 6: how = "both added:"; break;
-	case 7: how = "both modified:"; break;
+	case 1: how = _("both deleted:"); break;
+	case 2: how = _("added by us:"); break;
+	case 3: how = _("deleted by them:"); break;
+	case 4: how = _("added by them:"); break;
+	case 5: how = _("deleted by us:"); break;
+	case 6: how = _("both added:"); break;
+	case 7: how = _("both modified:"); break;
 	}
-	color_fprintf(s->fp, c, "%-20s%s\n", how, one);
+	color_fprintf(s->fp, c, _("%-20s%s\n"), how, one);
 	strbuf_release(&onebuf);
 }
 
@@ -158,13 +159,13 @@ static void wt_status_print_change_data(struct wt_status *s,
 		break;
 	case WT_STATUS_CHANGED:
 		if (d->new_submodule_commits || d->dirty_submodule) {
-			strbuf_addstr(&extra, " (");
+			strbuf_addstr(&extra, _(" ("));
 			if (d->new_submodule_commits)
-				strbuf_addf(&extra, "new commits, ");
+				strbuf_addf(&extra, _("new commits, "));
 			if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
-				strbuf_addf(&extra, "modified content, ");
+				strbuf_addf(&extra, _("modified content, "));
 			if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-				strbuf_addf(&extra, "untracked content, ");
+				strbuf_addf(&extra, _("untracked content, "));
 			strbuf_setlen(&extra, extra.len - 2);
 			strbuf_addch(&extra, ')');
 		}
@@ -175,40 +176,40 @@ static void wt_status_print_change_data(struct wt_status *s,
 	one = quote_path(one_name, -1, &onebuf, s->prefix);
 	two = quote_path(two_name, -1, &twobuf, s->prefix);
 
-	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
+	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));
 	switch (status) {
 	case DIFF_STATUS_ADDED:
-		color_fprintf(s->fp, c, "new file:   %s", one);
+		color_fprintf(s->fp, c, _("new file:   %s"), one);
 		break;
 	case DIFF_STATUS_COPIED:
-		color_fprintf(s->fp, c, "copied:     %s -> %s", one, two);
+		color_fprintf(s->fp, c, _("copied:     %s -> %s"), one, two);
 		break;
 	case DIFF_STATUS_DELETED:
-		color_fprintf(s->fp, c, "deleted:    %s", one);
+		color_fprintf(s->fp, c, _("deleted:    %s"), one);
 		break;
 	case DIFF_STATUS_MODIFIED:
-		color_fprintf(s->fp, c, "modified:   %s", one);
+		color_fprintf(s->fp, c, _("modified:   %s"), one);
 		break;
 	case DIFF_STATUS_RENAMED:
-		color_fprintf(s->fp, c, "renamed:    %s -> %s", one, two);
+		color_fprintf(s->fp, c, _("renamed:    %s -> %s"), one, two);
 		break;
 	case DIFF_STATUS_TYPE_CHANGED:
-		color_fprintf(s->fp, c, "typechange: %s", one);
+		color_fprintf(s->fp, c, _("typechange: %s"), one);
 		break;
 	case DIFF_STATUS_UNKNOWN:
-		color_fprintf(s->fp, c, "unknown:    %s", one);
+		color_fprintf(s->fp, c, _("unknown:    %s"), one);
 		break;
 	case DIFF_STATUS_UNMERGED:
-		color_fprintf(s->fp, c, "unmerged:   %s", one);
+		color_fprintf(s->fp, c, _("unmerged:   %s"), one);
 		break;
 	default:
-		die("bug: unhandled diff status %c", status);
+		die(_("bug: unhandled diff status %c"), status);
 	}
 	if (extra.len) {
-		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "%s", extra.buf);
+		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("%s"), extra.buf);
 		strbuf_release(&extra);
 	}
-	fprintf(s->fp, "\n");
+	fprintf(s->fp, _("\n"));
 	strbuf_release(&onebuf);
 	strbuf_release(&twobuf);
 }
@@ -535,7 +536,7 @@ static void wt_status_print_untracked(struct wt_status *s)
 	for (i = 0; i < s->untracked.nr; i++) {
 		struct string_list_item *it;
 		it = &(s->untracked.items[i]);
-		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
+		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));
 		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED, s), "%s",
 				 quote_path(it->string, strlen(it->string),
 					    &buf, s->prefix));
@@ -594,14 +595,14 @@ void wt_status_print(struct wt_status *s)
 	const char *branch_color = color(WT_STATUS_HEADER, s);
 
 	if (s->branch) {
-		const char *on_what = "On branch ";
+		const char *on_what = _("On branch ");
 		const char *branch_name = s->branch;
 		if (!prefixcmp(branch_name, "refs/heads/"))
 			branch_name += 11;
 		else if (!strcmp(branch_name, "HEAD")) {
 			branch_name = "";
 			branch_color = color(WT_STATUS_NOBRANCH, s);
-			on_what = "Not currently on any branch.";
+			on_what = _("Not currently on any branch.");
 		}
 		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "# ");
 		color_fprintf_ln(s->fp, branch_color, "%s%s", on_what, branch_name);
@@ -610,9 +611,9 @@ void wt_status_print(struct wt_status *s)
 	}
 
 	if (s->is_initial) {
-		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
-		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "# Initial commit");
-		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("# Initial commit"));
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
 	}
 
 	wt_status_print_updated(s);
@@ -625,25 +626,25 @@ void wt_status_print(struct wt_status *s)
 	if (s->show_untracked_files)
 		wt_status_print_untracked(s);
 	else if (s->commitable)
-		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
+		 fprintf(s->fp, _("# Untracked files not listed (use -u option to show untracked files)\n"));
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
 	if (!s->commitable) {
 		if (s->amend)
-			fprintf(s->fp, "# No changes\n");
+			fprintf(s->fp, _("# No changes\n"));
 		else if (s->nowarn)
 			; /* nothing */
 		else if (s->workdir_dirty)
-			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
+			printf(_("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n"));
 		else if (s->untracked.nr)
-			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
+			printf(_("nothing added to commit but untracked files present (use \"git add\" to track)\n"));
 		else if (s->is_initial)
-			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
+			printf(_("nothing to commit (create/copy files and use \"git add\" to track)\n"));
 		else if (!s->show_untracked_files)
-			printf("nothing to commit (use -u to show untracked files)\n");
+			printf(_("nothing to commit (use -u to show untracked files)\n"));
 		else
-			printf("nothing to commit (working directory clean)\n");
+			printf(_("nothing to commit (working directory clean)\n"));
 	}
 }
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-17 16:05 [PATCH] WIP: begin to translate git with gettext Jeff Epler
@ 2010-05-17 23:29 ` Robert Buck
  2010-05-18  4:23   ` Ævar Arnfjörð Bjarmason
  2010-05-18  7:40 ` Michael J Gruber
  1 sibling, 1 reply; 12+ messages in thread
From: Robert Buck @ 2010-05-17 23:29 UTC (permalink / raw)
  To: Jeff Epler
  Cc: Ævar Arnfjörð,
	Git Mailing List, Jakub Narebski, Dmitry Potapov, Jan Hudec,
	Thomas Rast, Marc Weber

Is gettext portable? Or is it only POSIX? If it's not portable, have
you considered using ICU instead as it is the best of class solution
for I18N/L10N?

That's my 0.02.

-Bob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-17 23:29 ` Robert Buck
@ 2010-05-18  4:23   ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-18  4:23 UTC (permalink / raw)
  To: Robert Buck
  Cc: Jeff Epler, Git Mailing List, Jakub Narebski, Dmitry Potapov,
	Jan Hudec, Thomas Rast, Marc Weber

On Mon, May 17, 2010 at 23:29, Robert Buck <buck.robert.j@gmail.com> wrote:
> Is gettext portable? Or is it only POSIX? If it's not portable, have
> you considered using ICU instead as it is the best of class solution
> for I18N/L10N?

Yes it's portable. libintl or libraries that provide compatible
interfaces are available on every platform Git is.

ICU is not a gettext replacement, it's a Unicode processing library, I
don't think it has any gettext-like features. In any case it would be
a huge dependency for the service it would provide.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-17 16:05 [PATCH] WIP: begin to translate git with gettext Jeff Epler
  2010-05-17 23:29 ` Robert Buck
@ 2010-05-18  7:40 ` Michael J Gruber
  2010-05-18  8:11   ` Ævar Arnfjörð Bjarmason
  2010-05-18 16:40   ` Jeff Epler
  1 sibling, 2 replies; 12+ messages in thread
From: Michael J Gruber @ 2010-05-18  7:40 UTC (permalink / raw)
  To: Jeff Epler
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Jakub Narebski, Dmitry Potapov, Jan Hudec, Thomas Rast,
	Marc Weber

Jeff Epler venit, vidit, dixit 17.05.2010 18:05:
> Signed-off-by: Jeff Epler <jepler@unpythonic.net>
> ---
> [resent with Cc to list and thread participants]
> 
> While I'm certain that there are a lot of things to object to in this
> patch, it shows 90% of what is needed to use gettext to translate
> the portions of git written in c, without involving undesired gnu
> infrastructure such as automake.
> 
> Makefile adds necessary rules for generating git.pot and for building
> and installing compiled message catalogs (.mo) from text message
> catalogs (.po).  It also adds a gettext support header and source file.
> 
> Minimal changes are made to git to use the requested LC_CTYPE and
> LC_MESSAGES, and some messages for 'git status' are marked for
> translation.
> 
> When I provided a gibberish translation of a message:
> #: wt-status.c:87
> msgid "# Changed but not updated:"
> msgstr "# Changes not blah blah blah"
> 
> running 'git status' used the translation:
> $ git status
> # On branch master
> # Your branch is ahead of 'origin/master' by 1 commit.
> #
> # Changes not blah blah blah
> ...
> 
> I ran with 'make install' and prefix set in config.mak.  It didn't seem
> to work when running from the source directory, and it may or may not
> work with runtime prefix.
> 
> 
>  Makefile    |   26 +++++++++++++
>  gettext.c   |   17 +++++++++
>  gettext.h   |   15 ++++++++
>  git.c       |    3 ++
>  wt-status.c |  117 ++++++++++++++++++++++++++++++-----------------------------
>  5 files changed, 120 insertions(+), 58 deletions(-)
>  create mode 100644 gettext.c
>  create mode 100644 gettext.h
> 
> diff --git a/Makefile b/Makefile
> index 4f7224a..c02ca18 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -294,6 +294,8 @@ RPMBUILD = rpmbuild
>  TCL_PATH = tclsh
>  TCLTK_PATH = wish
>  PTHREAD_LIBS = -lpthread
> +XGETTEXT = xgettext
> +MSGFMT = msgfmt
>  
>  export TCL_PATH TCLTK_PATH
>  
> @@ -518,6 +520,7 @@ LIB_H += userdiff.h
>  LIB_H += utf8.h
>  LIB_H += xdiff-interface.h
>  LIB_H += xdiff/xdiff.h
> +LIB_H += gettext.h
>  
>  LIB_OBJS += abspath.o
>  LIB_OBJS += advice.o
> @@ -559,6 +562,7 @@ LIB_OBJS += entry.o
>  LIB_OBJS += environment.o
>  LIB_OBJS += exec_cmd.o
>  LIB_OBJS += fsck.o
> +LIB_OBJS += gettext.o
>  LIB_OBJS += graph.o
>  LIB_OBJS += grep.o
>  LIB_OBJS += hash.o
> @@ -1371,6 +1375,12 @@ ifdef USE_NED_ALLOCATOR
>         COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
>  endif
>  
> +ifdef NO_GETTEXT
> +	COMPAT_CFLAGS += -DNO_GETTEXT
> +else
> +	LIBINTL = -lintl
> +endif
> +
>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK=NoThanks
>  endif
> @@ -1400,6 +1410,7 @@ ifndef V
>  	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
>  	QUIET_GEN      = @echo '   ' GEN $@;
>  	QUIET_LNCP     = @echo '   ' LN/CP $@;
> +	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
>  	QUIET_SUBDIR0  = +@subdir=
>  	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
>  			 $(MAKE) $(PRINT_DIR) -C $$subdir
> @@ -1427,6 +1438,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
>  template_dir_SQ = $(subst ','\'',$(template_dir))
>  htmldir_SQ = $(subst ','\'',$(htmldir))
>  prefix_SQ = $(subst ','\'',$(prefix))
> +sharedir_SQ = $(subst ','\'',$(sharedir))
>  
>  SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
>  PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
> @@ -1858,6 +1870,17 @@ cscope:
>  	$(RM) cscope*
>  	$(FIND) . -name '*.[hcS]' -print | xargs cscope -b
>  
> +pot:
> +	$(XGETTEXT) -k_ -o po/git.pot $(C_OBJ:o=c)
> +
> +POFILES := $(wildcard po/*.po)
> +MOFILES := $(patsubst po/%.po,share/locale/%/LC_MESSAGES/git.mo,$(POFILES))
> +MODIRS := $(patsubst po/%.po,share/locale/%/LC_MESSAGES/,$(POFILES))
> +all:: $(MOFILES)
> +share/locale/%/LC_MESSAGES/git.mo: po/%.po
> +	@mkdir -p $(dir $@)
> +	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
> +
>  ### Detect prefix changes
>  TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
>               $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
> @@ -1970,6 +1993,9 @@ install: all
>  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
> +	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(sharedir_SQ)/locale'
> +	(cd share && tar cf - locale) | \
> +		(cd '$(DESTDIR_SQ)$(sharedir_SQ)' && umask 022 && tar xof -)
>  	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
>  ifndef NO_PERL
>  	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
> diff --git a/gettext.c b/gettext.c
> new file mode 100644
> index 0000000..aadce19
> --- /dev/null
> +++ b/gettext.c
> @@ -0,0 +1,17 @@
> +#ifdef NO_GETTEXT
> +void git_setup_gettext() {}
> +#else
> +#include "exec_cmd.h"
> +#include <libintl.h>
> +#include <stdlib.h>
> +
> +void git_setup_gettext() {
> +    const char *podir = system_path("share/locale");
> +    if(!podir) return;
> +    char *ret = bindtextdomain("git", podir);
> +    free((void*)podir);
> +    ret = setlocale(LC_MESSAGES, "");
> +    ret = setlocale(LC_CTYPE, "");
> +    ret = textdomain("git");
> +}
> +#endif
> diff --git a/gettext.h b/gettext.h
> new file mode 100644
> index 0000000..8b221b4
> --- /dev/null
> +++ b/gettext.h
> @@ -0,0 +1,15 @@
> +#ifndef GETTEXT_H
> +#define GETTEXT_H
> +
> +void git_setup_gettext();
> +
> +#ifdef NO_GETTEXT
> +#define _(s) (s)
> +#define N_(s) (s)
> +#else
> +#include <libintl.h>
> +#define _(s) gettext(s)
> +#define N_(s) (s)
> +#endif
> +
> +#endif
> diff --git a/git.c b/git.c
> index 6bae305..5e7aedd 100644
> --- a/git.c
> +++ b/git.c
> @@ -3,6 +3,7 @@
>  #include "cache.h"
>  #include "quote.h"
>  #include "run-command.h"
> +#include "gettext.h"
>  
>  const char git_usage_string[] =
>  	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path]\n"
> @@ -481,6 +482,8 @@ int main(int argc, const char **argv)
>  	if (!cmd)
>  		cmd = "git-help";
>  
> +	git_setup_gettext();
> +
>  	/*
>  	 * "git-xxxx" is the same as "git xxxx", but we obviously:
>  	 *
> diff --git a/wt-status.c b/wt-status.c
> index 8ca59a2..a31cbc6 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -9,6 +9,7 @@
>  #include "quote.h"
>  #include "run-command.h"
>  #include "remote.h"
> +#include "gettext.h"
>  
>  static char default_wt_status_colors[][COLOR_MAXLEN] = {
>  	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
> @@ -48,33 +49,33 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
>  {
>  	const char *c = color(WT_STATUS_HEADER, s);
>  
> -	color_fprintf_ln(s->fp, c, "# Unmerged paths:");
> +	color_fprintf_ln(s->fp, c, _("# Unmerged paths:"));
>  	if (!advice_status_hints)
>  		return;
>  	if (s->in_merge)
>  		;
>  	else if (!s->is_initial)
> -		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git reset %s <file>...\" to unstage)"), s->reference);
>  	else
> -		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
> -	color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" as appropriate to mark resolution)");
> -	color_fprintf_ln(s->fp, c, "#");
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git rm --cached <file>...\" to unstage)"));
> +	color_fprintf_ln(s->fp, c, _("#   (use \"git add/rm <file>...\" as appropriate to mark resolution)"));
> +	color_fprintf_ln(s->fp, c, _("#"));
>  }
>  
>  static void wt_status_print_cached_header(struct wt_status *s)
>  {
>  	const char *c = color(WT_STATUS_HEADER, s);
>  
> -	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
> +	color_fprintf_ln(s->fp, c, _("# Changes to be committed:"));
>  	if (!advice_status_hints)
>  		return;
>  	if (s->in_merge)
>  		; /* NEEDSWORK: use "git reset --unresolve"??? */
>  	else if (!s->is_initial)
> -		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git reset %s <file>...\" to unstage)"), s->reference);
>  	else
> -		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
> -	color_fprintf_ln(s->fp, c, "#");
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git rm --cached <file>...\" to unstage)"));
> +	color_fprintf_ln(s->fp, c, _("#"));
>  }
>  
>  static void wt_status_print_dirty_header(struct wt_status *s,
> @@ -83,32 +84,32 @@ static void wt_status_print_dirty_header(struct wt_status *s,
>  {
>  	const char *c = color(WT_STATUS_HEADER, s);
>  
> -	color_fprintf_ln(s->fp, c, "# Changed but not updated:");
> +	color_fprintf_ln(s->fp, c, _("# Changed but not updated:"));
>  	if (!advice_status_hints)
>  		return;
>  	if (!has_deleted)
> -		color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to update what will be committed)");
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git add <file>...\" to update what will be committed)"));
>  	else
> -		color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" to update what will be committed)");
> -	color_fprintf_ln(s->fp, c, "#   (use \"git checkout -- <file>...\" to discard changes in working directory)");
> +		color_fprintf_ln(s->fp, c, _("#   (use \"git add/rm <file>...\" to update what will be committed)"));
> +	color_fprintf_ln(s->fp, c, _("#   (use \"git checkout -- <file>...\" to discard changes in working directory)"));
>  	if (has_dirty_submodules)
> -		color_fprintf_ln(s->fp, c, "#   (commit or discard the untracked or modified content in submodules)");
> -	color_fprintf_ln(s->fp, c, "#");
> +		color_fprintf_ln(s->fp, c, _("#   (commit or discard the untracked or modified content in submodules)"));
> +	color_fprintf_ln(s->fp, c, _("#"));
>  }
>  
>  static void wt_status_print_untracked_header(struct wt_status *s)
>  {
>  	const char *c = color(WT_STATUS_HEADER, s);
> -	color_fprintf_ln(s->fp, c, "# Untracked files:");
> +	color_fprintf_ln(s->fp, c, _("# Untracked files:"));
>  	if (!advice_status_hints)
>  		return;
> -	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to include in what will be committed)");
> -	color_fprintf_ln(s->fp, c, "#");
> +	color_fprintf_ln(s->fp, c, _("#   (use \"git add <file>...\" to include in what will be committed)"));
> +	color_fprintf_ln(s->fp, c, _("#"));
>  }
>  
>  static void wt_status_print_trailer(struct wt_status *s)
>  {
> -	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
> +	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
>  }
>  
>  #define quote_path quote_path_relative
> @@ -119,20 +120,20 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
>  	const char *c = color(WT_STATUS_UNMERGED, s);
>  	struct wt_status_change_data *d = it->util;
>  	struct strbuf onebuf = STRBUF_INIT;
> -	const char *one, *how = "bug";
> +	const char *one, *how = _("bug");
>  
>  	one = quote_path(it->string, -1, &onebuf, s->prefix);
> -	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
> +	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));
>  	switch (d->stagemask) {
> -	case 1: how = "both deleted:"; break;
> -	case 2: how = "added by us:"; break;
> -	case 3: how = "deleted by them:"; break;
> -	case 4: how = "added by them:"; break;
> -	case 5: how = "deleted by us:"; break;
> -	case 6: how = "both added:"; break;
> -	case 7: how = "both modified:"; break;
> +	case 1: how = _("both deleted:"); break;
> +	case 2: how = _("added by us:"); break;
> +	case 3: how = _("deleted by them:"); break;
> +	case 4: how = _("added by them:"); break;
> +	case 5: how = _("deleted by us:"); break;
> +	case 6: how = _("both added:"); break;
> +	case 7: how = _("both modified:"); break;
>  	}
> -	color_fprintf(s->fp, c, "%-20s%s\n", how, one);
> +	color_fprintf(s->fp, c, _("%-20s%s\n"), how, one);
>  	strbuf_release(&onebuf);
>  }
>  
> @@ -158,13 +159,13 @@ static void wt_status_print_change_data(struct wt_status *s,
>  		break;
>  	case WT_STATUS_CHANGED:
>  		if (d->new_submodule_commits || d->dirty_submodule) {
> -			strbuf_addstr(&extra, " (");
> +			strbuf_addstr(&extra, _(" ("));
>  			if (d->new_submodule_commits)
> -				strbuf_addf(&extra, "new commits, ");
> +				strbuf_addf(&extra, _("new commits, "));
>  			if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
> -				strbuf_addf(&extra, "modified content, ");
> +				strbuf_addf(&extra, _("modified content, "));
>  			if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
> -				strbuf_addf(&extra, "untracked content, ");
> +				strbuf_addf(&extra, _("untracked content, "));
>  			strbuf_setlen(&extra, extra.len - 2);
>  			strbuf_addch(&extra, ')');
>  		}
> @@ -175,40 +176,40 @@ static void wt_status_print_change_data(struct wt_status *s,
>  	one = quote_path(one_name, -1, &onebuf, s->prefix);
>  	two = quote_path(two_name, -1, &twobuf, s->prefix);
>  
> -	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
> +	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));
>  	switch (status) {
>  	case DIFF_STATUS_ADDED:
> -		color_fprintf(s->fp, c, "new file:   %s", one);
> +		color_fprintf(s->fp, c, _("new file:   %s"), one);
>  		break;
>  	case DIFF_STATUS_COPIED:
> -		color_fprintf(s->fp, c, "copied:     %s -> %s", one, two);
> +		color_fprintf(s->fp, c, _("copied:     %s -> %s"), one, two);
>  		break;
>  	case DIFF_STATUS_DELETED:
> -		color_fprintf(s->fp, c, "deleted:    %s", one);
> +		color_fprintf(s->fp, c, _("deleted:    %s"), one);
>  		break;
>  	case DIFF_STATUS_MODIFIED:
> -		color_fprintf(s->fp, c, "modified:   %s", one);
> +		color_fprintf(s->fp, c, _("modified:   %s"), one);
>  		break;
>  	case DIFF_STATUS_RENAMED:
> -		color_fprintf(s->fp, c, "renamed:    %s -> %s", one, two);
> +		color_fprintf(s->fp, c, _("renamed:    %s -> %s"), one, two);
>  		break;
>  	case DIFF_STATUS_TYPE_CHANGED:
> -		color_fprintf(s->fp, c, "typechange: %s", one);
> +		color_fprintf(s->fp, c, _("typechange: %s"), one);
>  		break;
>  	case DIFF_STATUS_UNKNOWN:
> -		color_fprintf(s->fp, c, "unknown:    %s", one);
> +		color_fprintf(s->fp, c, _("unknown:    %s"), one);
>  		break;
>  	case DIFF_STATUS_UNMERGED:
> -		color_fprintf(s->fp, c, "unmerged:   %s", one);
> +		color_fprintf(s->fp, c, _("unmerged:   %s"), one);

I have no experience whatsover with gettext, but it looks quite
dangerous to me to have printf format specifiers as part of the
localized text. It means that our programs can crash depending on the
LANG setting at run time if localisers mess up. We'll never catch this
unless we run all tests in all languages!

Also, the basic structure of the output should probably be independent
of the language, preferring consistent structure across languages over
linguistically consistent structure  within a language.

That means we'll have to do a lot of strcat's (the _() things are not
compile time constants, are they?) rather than those mechanical
replacements above. Are you prepared to do that?

>  		break;
>  	default:
> -		die("bug: unhandled diff status %c", status);
> +		die(_("bug: unhandled diff status %c"), status);
>  	}
>  	if (extra.len) {
> -		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "%s", extra.buf);
> +		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("%s"), extra.buf);

Seriously?

>  		strbuf_release(&extra);
>  	}
> -	fprintf(s->fp, "\n");
> +	fprintf(s->fp, _("\n"));
>  	strbuf_release(&onebuf);
>  	strbuf_release(&twobuf);
>  }
> @@ -535,7 +536,7 @@ static void wt_status_print_untracked(struct wt_status *s)
>  	for (i = 0; i < s->untracked.nr; i++) {
>  		struct string_list_item *it;
>  		it = &(s->untracked.items[i]);
> -		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
> +		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("#\t"));

?

>  		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED, s), "%s",
>  				 quote_path(it->string, strlen(it->string),
>  					    &buf, s->prefix));
> @@ -594,14 +595,14 @@ void wt_status_print(struct wt_status *s)
>  	const char *branch_color = color(WT_STATUS_HEADER, s);
>  
>  	if (s->branch) {
> -		const char *on_what = "On branch ";
> +		const char *on_what = _("On branch ");
>  		const char *branch_name = s->branch;
>  		if (!prefixcmp(branch_name, "refs/heads/"))
>  			branch_name += 11;
>  		else if (!strcmp(branch_name, "HEAD")) {
>  			branch_name = "";
>  			branch_color = color(WT_STATUS_NOBRANCH, s);
> -			on_what = "Not currently on any branch.";
> +			on_what = _("Not currently on any branch.");
>  		}
>  		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "# ");
>  		color_fprintf_ln(s->fp, branch_color, "%s%s", on_what, branch_name);
> @@ -610,9 +611,9 @@ void wt_status_print(struct wt_status *s)
>  	}
>  
>  	if (s->is_initial) {
> -		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
> -		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "# Initial commit");
> -		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
> +		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
> +		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("# Initial commit"));
> +		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), _("#"));
>  	}
>  
>  	wt_status_print_updated(s);
> @@ -625,25 +626,25 @@ void wt_status_print(struct wt_status *s)
>  	if (s->show_untracked_files)
>  		wt_status_print_untracked(s);
>  	else if (s->commitable)
> -		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
> +		 fprintf(s->fp, _("# Untracked files not listed (use -u option to show untracked files)\n"));
>  
>  	if (s->verbose)
>  		wt_status_print_verbose(s);
>  	if (!s->commitable) {
>  		if (s->amend)
> -			fprintf(s->fp, "# No changes\n");
> +			fprintf(s->fp, _("# No changes\n"));
>  		else if (s->nowarn)
>  			; /* nothing */
>  		else if (s->workdir_dirty)
> -			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
> +			printf(_("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n"));
>  		else if (s->untracked.nr)
> -			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
> +			printf(_("nothing added to commit but untracked files present (use \"git add\" to track)\n"));
>  		else if (s->is_initial)
> -			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
> +			printf(_("nothing to commit (create/copy files and use \"git add\" to track)\n"));
>  		else if (!s->show_untracked_files)
> -			printf("nothing to commit (use -u to show untracked files)\n");
> +			printf(_("nothing to commit (use -u to show untracked files)\n"));
>  		else
> -			printf("nothing to commit (working directory clean)\n");
> +			printf(_("nothing to commit (working directory clean)\n"));
>  	}
>  }
>  

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18  7:40 ` Michael J Gruber
@ 2010-05-18  8:11   ` Ævar Arnfjörð Bjarmason
  2010-05-18 16:40   ` Jeff Epler
  1 sibling, 0 replies; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-18  8:11 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Jeff Epler, Git Mailing List, Jakub Narebski, Dmitry Potapov,
	Jan Hudec, Thomas Rast, Marc Weber

On Tue, May 18, 2010 at 07:40, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> I have no experience whatsover with gettext, but it looks quite
> dangerous to me to have printf format specifiers as part of the
> localized text. It means that our programs can crash depending on the
> LANG setting at run time if localisers mess up. We'll never catch this
> unless we run all tests in all languages!

I don't have much experience with gettext either (except through
Launchpad), maybe it has some internal facilities to avoid errors in
these cases.

You can test if the translated messages contain the same format
specifiers as the originals, and in any case much larger projects than
Git manage dozens of translations with gettext while avoiding
disaster.

> Also, the basic structure of the output should probably be independent
> of the language, preferring consistent structure across languages over
> linguistically consistent structure  within a language.
>
> That means we'll have to do a lot of strcat's (the _() things are not
> compile time constants, are they?) rather than those mechanical
> replacements above. Are you prepared to do that?

Generally you don't to strcat's since you don't want to enforce word
order, doing so will make the messages sound like Yoda in some of the
target languages.

That does mean re-arranging some code if it's to be done properly.

>>               break;
>>       default:
>> -             die("bug: unhandled diff status %c", status);
>> +             die(_("bug: unhandled diff status %c"), status);
>>       }
>>       if (extra.len) {
>> -             color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "%s", extra.buf);
>> +             color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("%s"), extra.buf);
>
> Seriously?

No, as Jeff said it's just a proof of concept. That patch as-is
doesn't reflect good translation practices, it just bootstraps
gettext.

Which is very useful by the way, thanks Jeff.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18  7:40 ` Michael J Gruber
  2010-05-18  8:11   ` Ævar Arnfjörð Bjarmason
@ 2010-05-18 16:40   ` Jeff Epler
  2010-05-18 17:02     ` Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 12+ messages in thread
From: Jeff Epler @ 2010-05-18 16:40 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Jakub Narebski, Dmitry Potapov, Jan Hudec, Thomas Rast,
	Marc Weber

On Tue, May 18, 2010 at 09:40:23AM +0200, Michael J Gruber wrote:
> > -		color_fprintf(s->fp, c, "unmerged:   %s", one);
> > +		color_fprintf(s->fp, c, _("unmerged:   %s"), one);
> 
> I have no experience whatsover with gettext, but it looks quite
> dangerous to me to have printf format specifiers as part of the
> localized text. It means that our programs can crash depending on the
> LANG setting at run time if localisers mess up. We'll never catch this
> unless we run all tests in all languages!

This is exactly how gettext works.  Yes, you can get crashes if the
translated string does not have the right arguments--and I would not be
at all surprised to hear of at least one privilege escalation bug
due to a bad message catalog, since printf format errors can be used in
such interesting ways.

Anyway, for printf-style formats, 'msgfmt' can be directed to check for
this situation:
    $ cat bad.po
    msgid ""
    msgstr "Content-Type: text/plain; charset=UTF-8\n"

    #,c-format
    msgid "foo %s %d"
    msgstr "föö %d %d"

    $ msgfmt --check-format bad.po
    bad.po:6: format specifications in 'msgid' and 'msgstr' for argument 1 are not the same
    msgfmt: found 1 fatal error
 
> Also, the basic structure of the output should probably be independent
> of the language, preferring consistent structure across languages over
> linguistically consistent structure  within a language.

No, the ability of gettext+printf to use the right structure of the
user's language is a strength.  For instance, consider the translation
into Yoda's locale of the following sentence:

    printf("The %s is %s.\n", "Future", "Clouded");

The proper localized message is

    Clouded the Future is.

Anything else will range from confusing to unintelligible to the
native speaker.  You get that with gettext by writing

    printf(_("The %s is %s.\n"), _("Future"), _("Clouded"));

together with the message catalog entry
    msgid "The %s is %s.\n"
    msgfmt "%2$s the %1$s is.\n"

> >  	if (extra.len) {
> > -		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "%s", extra.buf);
> > +		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), _("%s"), extra.buf);
> 
> Seriously?

No, that one's a mistake.  I did not take care when choosing which
strings to mark, because I was mostly interested in showing a
proof-of-concept for using gettext to translate core parts of git.

The amount of work to mark all the source files and then to keep the
marks up to date should not be underestimated--and that's just the work
to enable translators to localize the software.  It is important to
gauge the interest in the git community in actually doing this work.

As my own primary language is English, I have only a theoretical
interest in this feature.  However, the existence of translations for
gitk and git-gui indicates to me that the community probably does desire
this.

Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18 16:40   ` Jeff Epler
@ 2010-05-18 17:02     ` Ævar Arnfjörð Bjarmason
  2010-05-20 16:02       ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-18 17:02 UTC (permalink / raw)
  To: Jeff Epler
  Cc: Michael J Gruber, Git Mailing List, Jakub Narebski,
	Dmitry Potapov, Jan Hudec, Thomas Rast, Marc Weber

On Tue, May 18, 2010 at 16:40, Jeff Epler <jepler@unpythonic.net> wrote:
> The amount of work to mark all the source files and then to keep the
> marks up to date should not be underestimated--and that's just the work
> to enable translators to localize the software.  It is important to
> gauge the interest in the git community in actually doing this work.

It's also something you shouldn't overestimate. I've been involved in
internationalizing several projects that were previously English-only.

The work of making things translatable can be done incrementally. You
also don't have to get everything right the first time, the current
proof of concept translation of `git status` for instance suffers from
numerous problems, but it's still better than nothing.

It can be used as-is and then incrementally improved by arranging the
strings more intelligently in the future.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18 17:02     ` Ævar Arnfjörð Bjarmason
@ 2010-05-20 16:02       ` Ævar Arnfjörð Bjarmason
  2010-05-21 18:02         ` Dévai Tamás
  0 siblings, 1 reply; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-20 16:02 UTC (permalink / raw)
  To: Jeff Epler
  Cc: Michael J Gruber, Git Mailing List, Jakub Narebski,
	Dmitry Potapov, Jan Hudec, Thomas Rast, Marc Weber

On Tue, May 18, 2010 at 17:02, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Tue, May 18, 2010 at 16:40, Jeff Epler <jepler@unpythonic.net> wrote:
>> The amount of work to mark all the source files and then to keep the
>> marks up to date should not be underestimated--and that's just the work
>> to enable translators to localize the software.  It is important to
>> gauge the interest in the git community in actually doing this work.
>
> It's also something you shouldn't overestimate. I've been involved in
> internationalizing several projects that were previously English-only.
>
> The work of making things translatable can be done incrementally. You
> also don't have to get everything right the first time, the current
> proof of concept translation of `git status` for instance suffers from
> numerous problems, but it's still better than nothing.
>
> It can be used as-is and then incrementally improved by arranging the
> strings more intelligently in the future.

I did some work on this in my branch:
http://github.com/avar/git/compare/master...topic/git-gettext

Fixed up the Makefile rules a bit, added appropriate gitignores, and
added a work in progress po/is.po.

Still have to go through the gettext manual to figure out how to
integrate this with our shellscripts.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-20 16:02       ` Ævar Arnfjörð Bjarmason
@ 2010-05-21 18:02         ` Dévai Tamás
  0 siblings, 0 replies; 12+ messages in thread
From: Dévai Tamás @ 2010-05-21 18:02 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List

2010. 05. 20, csütörtök keltezéssel 16.02-kor Ævar Arnfjörð Bjarmason
ezt írta:

> Still have to go through the gettext manual to figure out how to
> integrate this with our shellscripts.

info '(gettext)sh'

or the online help[1][2] might be of some help.

[1]: http://www.gnu.org/software/gettext/manual/gettext.html#sh_002dformat
[2]: http://www.gnu.org/software/gettext/manual/gettext.html#sh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18 16:07   ` Jeff Epler
@ 2010-05-18 16:47     ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-18 16:47 UTC (permalink / raw)
  To: Jeff Epler; +Cc: Git Mailing List

On Tue, May 18, 2010 at 16:07, Jeff Epler <jepler@unpythonic.net> wrote:
> Is it possible that you don't have is_IS language support?  On Ubuntu
> this comes from a package like language-pack-is-base.
>
> I tried your po entries in a file named po/is.po, and it didn't work.
> When I installed language-pack-is-base, it did.
>
> $ unset LANGUAGE LC_MESSAGES; export LANG=is_IS.UTF-8
> $ /usr/local/jepler/git/bin/git status | head -1
> # A greininni master

Yes, my system's settings were broken. Thanks, it works now.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
  2010-05-18 10:57 ` Ævar Arnfjörð Bjarmason
@ 2010-05-18 16:07   ` Jeff Epler
  2010-05-18 16:47     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Epler @ 2010-05-18 16:07 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List

Is it possible that you don't have is_IS language support?  On Ubuntu
this comes from a package like language-pack-is-base.

I tried your po entries in a file named po/is.po, and it didn't work.
When I installed language-pack-is-base, it did.

$ unset LANGUAGE LC_MESSAGES; export LANG=is_IS.UTF-8
$ /usr/local/jepler/git/bin/git status | head -1
# A greininni master

Comparing the bad strace with the good, and excluding ENOENT opens, it's
the presence of the is_IS language pack that is the first difference.
@@ -1,4 +1,8 @@
 open("/usr/share/locale/locale.alias", O_RDONLY) = 3
+open("/usr/lib/locale/is_IS.utf8/LC_MESSAGES", O_RDONLY) = 3
+open("/usr/lib/locale/is_IS.utf8/LC_MESSAGES/SYS_LC_MESSAGES", O_RDONLY) = 3
+open("/usr/lib/locale/is_IS.utf8/LC_CTYPE", O_RDONLY) = 3
 open("share/locale/", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 14
 open("share/locale/is/", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 15
 open("share/locale/is/LC_MESSAGES/", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 16
+open("/usr/local/jepler/git/share/locale/is/LC_MESSAGES/git.mo", O_RDONLY) = 12

Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] WIP: begin to translate git with gettext
       [not found] <20100517160208.GA20842@unpythonic.net>
@ 2010-05-18 10:57 ` Ævar Arnfjörð Bjarmason
  2010-05-18 16:07   ` Jeff Epler
  0 siblings, 1 reply; 12+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-05-18 10:57 UTC (permalink / raw)
  To: Jeff Epler, Git Mailing List

On Mon, May 17, 2010 at 16:02, Jeff Epler <jepler@unpythonic.net> wrote:
> Signed-off-by: Jeff Epler <jepler@unpythonic.net>
> ---
> While I'm certain that there are a lot of things to object to in this
> patch, it shows 90% of what is needed to use gettext to translate
> the portions of git written in c, without involving undesired gnu
> infrastructure such as automake.
>
> Makefile adds necessary rules for generating git.pot and for building
> and installing compiled message catalogs (.mo) from text message
> catalogs (.po).  It also adds a gettext support header and source file.
>
> Minimal changes are made to git to use the requested LC_CTYPE and
> LC_MESSAGES, and some messages for 'git status' are marked for
> translation.
>
> When I provided a gibberish translation of a message:
> #: wt-status.c:87
> msgid "# Changed but not updated:"
> msgstr "# Changes not blah blah blah"
>
> running 'git status' used the translation:
> $ git status
> # On branch master
> # Your branch is ahead of 'origin/master' by 1 commit.
> #
> # Changes not blah blah blah
> ...
>
> I ran with 'make install' and prefix set in config.mak.  It didn't seem
> to work when running from the source directory, and it may or may not
> work with runtime prefix.

Thanks a lot for this patch.

I'm having some trouble getting it to work though. I did:

    make pot

That made the po/git.pot file with all the relevant messages. Then I
made a po/is.po in poedit which has stuff like:

    #: wt-status.c:598
    msgid "On branch "
    msgstr "A greininni "

    #: wt-status.c:605
    msgid "Not currently on any branch."
    msgstr "Ekki á grænni grein.

I did:

    make prefix=/usr all
    sudo make prefix=/usr install

But any invocation of my new git-status (I uninstalled my system git
first) with either LC_ALL, LC_MESSAGES or LC_CTYPE set to is, is_IS or
is_IS.UTF-8 doest't yield the desired results:

    $ strings /usr/share/locale/is/LC_MESSAGES/git.mo |grep grein
    nni grein.
    A greininni

it doesn't seem to spot the file:

    $ strace git status 2>&1|grep locale
    open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE) = -1
ENOENT (No such file or directory)
    open("/usr/share/locale/locale.alias", O_RDONLY) = 3
    open("/usr/lib/locale/is/LC_MESSAGES", O_RDONLY) = -1 ENOENT (No
such file or directory)
    open("/usr/share/locale-langpack/is/LC_MESSAGES", O_RDONLY) = 3
    open("/usr/share/locale-langpack/is/LC_MESSAGES/SYS_LC_MESSAGES",
O_RDONLY) = -1 ENOENT (No such file or directory)
    open("/usr/lib/locale/is/LC_CTYPE", O_RDONLY) = -1 ENOENT (No such
file or directory)
    open("/usr/share/locale-langpack/is/LC_CTYPE", O_RDONLY) = -1
ENOENT (No such file or directory)
    open("share/locale/",
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 5
    open("share/locale/.gitignore", O_RDONLY|O_LARGEFILE) = -1 ENOENT
(No such file or directory)
    open("share/locale/is/",
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 6
    open("share/locale/is/.gitignore", O_RDONLY|O_LARGEFILE) = -1
ENOENT (No such file or directory)
    open("share/locale/is/LC_MESSAGES/",
O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_CLOEXEC) = 7
    open("share/locale/is/LC_MESSAGES/.gitignore",
O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)

And the UI remains untranslated.

I'm probably doing something very silly. I'd appreciate some help if
you can spot what it is, if I can get this working I can start
submitting some i18n patches.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-05-21 18:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-17 16:05 [PATCH] WIP: begin to translate git with gettext Jeff Epler
2010-05-17 23:29 ` Robert Buck
2010-05-18  4:23   ` Ævar Arnfjörð Bjarmason
2010-05-18  7:40 ` Michael J Gruber
2010-05-18  8:11   ` Ævar Arnfjörð Bjarmason
2010-05-18 16:40   ` Jeff Epler
2010-05-18 17:02     ` Ævar Arnfjörð Bjarmason
2010-05-20 16:02       ` Ævar Arnfjörð Bjarmason
2010-05-21 18:02         ` Dévai Tamás
     [not found] <20100517160208.GA20842@unpythonic.net>
2010-05-18 10:57 ` Ævar Arnfjörð Bjarmason
2010-05-18 16:07   ` Jeff Epler
2010-05-18 16:47     ` Ævar Arnfjörð Bjarmason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.