[PATCH 1/2] i18n: Extract msgs marked by sh gettext wrappers

[PATCH 1/2] i18n: Extract msgs marked by sh gettext wrappers

[Jiang Xin]

Since we have additional shell wrappers (gettextln and eval_gettextln)
for gettext, we need to take into account these wrapers when run
'make pot' to extract messages from shell scripts.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4592f..dc3fd 100644
--- a/Makefile
+++ b/Makefile
@@ -2333,7 +2333,8 @@ XGETTEXT_FLAGS = \
 	--from-code=UTF-8
 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
 	--keyword=_ --keyword=N_ --keyword="Q_:1,2"
-XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell
+XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
+	--keyword=gettextln --keyword=eval_gettextln
 XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl
 LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(MISC_H)
 LOCALIZED_SH := $(SCRIPT_SH)
-- 
1.7.10.2.559.g0ba0f00

[PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Jiang Xin]

In commit b9b9c22, Ævar addressed a issue when gettextized a message
started with "--", such as "--cached cannot be used with --files", the
first word in the message would be treated as an option of gettext, and
might raise a bad option error.

The solution Ævar provided is to add a extra "--" option between gettext
and the message. But Vincent fount out later that the extra "--" was
extracted as gettext message by xgettext instead of the real message.
See:

 * http://thread.gmane.org/gmane.comp.version-control.git/199042

In order to fix this dilemma, we move the extra "--" option from gettext
to gettext wrapper (gettextln, and eval_gettext), and gettextize the
message using the wrapper function instead of gettext itself.

But for system with libintl's gettext.sh, eval_gettext and
eval_gettextln won't have this fix.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
 git-sh-i18n.sh   | 8 ++++++--
 git-submodule.sh | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 6a27f..1c61e4 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -49,7 +49,7 @@ gnu)
 gettext_without_eval_gettext)
 	# Solaris has a gettext(1) but no eval_gettext(1)
 	eval_gettext () {
-		gettext "$1" | (
+		gettext -- "$1" | (
 			export PATH $(git sh-i18n--envsubst --variables "$1");
 			git sh-i18n--envsubst "$1"
 		)
@@ -68,10 +68,14 @@ poison)
 	;;
 *)
 	gettext () {
+		# Bypass options, such as '--'.
+		shift $(($# - 1))
 		printf "%s" "$1"
 	}
 
 	eval_gettext () {
+		# Bypass options, such as '--'.
+		shift $(($# - 1))
 		printf "%s" "$1" | (
 			export PATH $(git sh-i18n--envsubst --variables "$1");
 			git sh-i18n--envsubst "$1"
@@ -82,7 +86,7 @@ esac
 
 # Git-specific wrapper functions
 gettextln () {
-	gettext "$1"
+	gettext -- "$1"
 	echo
 }
 
diff --git a/git-submodule.sh b/git-submodule.sh
index 5c61a..bb9f6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -710,7 +710,7 @@ cmd_summary() {
 	if [ -n "$files" ]
 	then
 		test -n "$cached" &&
-		die "$(gettext -- "--cached cannot be used with --files")"
+		die "$(gettextln "--cached cannot be used with --files")"
 		diff_cmd=diff-files
 		head=
 	fi
-- 
1.7.10.2.559.g0ba0f00

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Ævar Arnfjörð Bjarmason]

On Mon, Jun 4, 2012 at 1:09 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
> In commit b9b9c22, Ævar addressed a issue when gettextized a message
> started with "--", such as "--cached cannot be used with --files", the
> first word in the message would be treated as an option of gettext, and
> might raise a bad option error.
>
> The solution Ævar provided is to add a extra "--" option between gettext
> and the message. But Vincent fount out later that the extra "--" was
> extracted as gettext message by xgettext instead of the real message.
> See:
>
>  * http://thread.gmane.org/gmane.comp.version-control.git/199042
>
> In order to fix this dilemma, we move the extra "--" option from gettext
> to gettext wrapper (gettextln, and eval_gettext), and gettextize the
> message using the wrapper function instead of gettext itself.
>
> But for system with libintl's gettext.sh, eval_gettext and
> eval_gettextln won't have this fix.

I think a better solution to this is to just apply this patch:

-               die "$(gettext -- "--cached cannot be used with --files")"
+               die "$(gettext "the --cached option is incompatible
with the --files option")"

It's one message only, and this is due to a bug in the xgettext
extraction. I think changing the only message suffering from this
issue (or likely to suffer from it) beats a solution where starting a
message with -- only works for some of the xgettext
commands/functions.

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Jiang Xin]

2012/6/4 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:
> I think a better solution to this is to just apply this patch:
>
> -               die "$(gettext -- "--cached cannot be used with --files")"
> +               die "$(gettext "the --cached option is incompatible
> with the --files option")"

The rewrite message is a bit odd for there are 40 more alike
messages (extracted from C files) start with "--”, such as:

    msgid "--delete only accepts plain target ref names"
    msgid "--all and --tags are incompatible"
    msgid "--all can't be combined with refspecs"
    msgid "--mirror and --tags are incompatible"
    msgid "--mirror can't be combined with refspecs"

> I think changing the only message suffering from this
> issue (or likely to suffer from it) beats a solution where starting a
> message with -- only works for some of the xgettext
> commands/functions.

If I had not changed like this, I would not find out there are 27
marked messages (by gettext wrappers) have not been
extracted to "po/git.pot". ;-)

-- 
Jiang Xin

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> 2012/6/4 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:
>> I think a better solution to this is to just apply this patch:
>>
>> -               die "$(gettext -- "--cached cannot be used with --files")"
>> +               die "$(gettext "the --cached option is incompatible
>> with the --files option")"
>
> The rewrite message is a bit odd for there are 40 more alike
> messages (extracted from C files) start with "--”, such as:
>
>     msgid "--delete only accepts plain target ref names"
>     msgid "--all and --tags are incompatible"
>     msgid "--all can't be combined with refspecs"
>     msgid "--mirror and --tags are incompatible"
>     msgid "--mirror can't be combined with refspecs"
>
>> I think changing the only message suffering from this
>> issue (or likely to suffer from it) beats a solution where starting a
>> message with -- only works for some of the xgettext
>> commands/functions.
>
> If I had not changed like this, I would not find out there are 27
> marked messages (by gettext wrappers) have not been
> extracted to "po/git.pot". ;-)

I think there is value for having a generic solution than declaring
"No message shall begin with a dash".

I am not convinced that it is ideal for the implementation of
gettext_ln to prepend "--" in front when it calls gettext, though.
Shouldn't the caller of gettext_ln be the one who is responsible for
doing that?  After all, that is the one that knows that the MSGID
argument needs the quoting.

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Jiang Xin]

2012/6/5 Junio C Hamano <gitster@pobox.com>:
> I think there is value for having a generic solution than declaring
> "No message shall begin with a dash".
>
> I am not convinced that it is ideal for the implementation of
> gettext_ln to prepend "--" in front when it calls gettext, though.

I saw Ævar's bug report on xgettext to the GNU gettext list:

 * http://git.661346.n2.nabble.com/GETTEXT-BUG-xgettext-1-can-t-extract-quot-gettext-foo-quot-td7560744.html

May be post to a more official mailing list, such as bug-gnu-utils at
https://lists.gnu.org/mailman/listinfo/bug-gnu-utils will get some response,
but I doubt there won't be a clear resolution in xgettext.

The default configurations of xgettext for shell scripts are defined in file
'gettext-tools/src/x-sh.c':

      x_sh_keyword ("gettext");
      x_sh_keyword ("ngettext:1,2");
      x_sh_keyword ("eval_gettext");
      x_sh_keyword ("eval_ngettext:1,2");

Keyword "gettext" above is the same as "gettext:1". xgettext has
no idea of options and arguments, if there is a seperator ('--') between
gettext and the message, message is argument 2. For example:

    gettext -- "--cached cannot be used with --files"

We can override the default settings of xgettext by passing '-k' and
multiple '--keyword', '--flags' options to xgettext.

    xgettext  ... --language=Shell \
                  -k  --keyword=gettext:2 \
                  --flag=gettext:2:pass-sh-format  ...

But "gettext message" will be broken.

Write a wrapper for gettext  and extract messages using
'xgettext --keyword=<gettext-wrapper>' is a reasonable solution.


-- 
Jiang Xin

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Jonathan Nieder]

Jiang Xin wrote:

> I saw Ævar's bug report on xgettext to the GNU gettext list:
>
>  * http://git.661346.n2.nabble.com/GETTEXT-BUG-xgettext-1-can-t-extract-quot-gettext-foo-quot-td7560744.html
>
> May be post to a more official mailing list, such as bug-gnu-utils at
> https://lists.gnu.org/mailman/listinfo/bug-gnu-utils will get some response,

I think bug-gettext is the right place:
http://lists.gnu.org/archive/html/bug-gettext/2012-06/msg00003.html

> but I doubt there won't be a clear resolution in xgettext.

The double negative is leaving me confused. :)  Are you saying you
doubt that anyone will come up with a patch to fix xgettext, or that
you doubt that the result will be satisfactory?

[...]
> Write a wrapper for gettext  and extract messages using
> 'xgettext --keyword=<gettext-wrapper>' is a reasonable solution.

For what it's worth that does seem to me like the most sensible
workaround in the short term. :/

	gittext () {	# or sane_gettext or literal_gettext or something
		gettext -- "$1"
	}

	gittextln () {
		gettext -- "$1"
		echo
	}

Thanks,
Jonathan

Re: [PATCH 2/2] i18n: Add extra -- to seperate gettext and message

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> For what it's worth that does seem to me like the most sensible
> workaround in the short term. :/
>
> 	gittext () {	# or sane_gettext or literal_gettext or something
> 		gettext -- "$1"
> 	}
>
> 	gittextln () {
> 		gettext -- "$1"
> 		echo
> 	}
>
> Thanks,
> Jonathan

Thanks for rephrasing ;-)  That "g*I*ttext" looks a bit too cute for
my taste, but something along that line does seem to be a reasonable
way forward.