i18n for git-am, git-rebase and git-merge

---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Makefile +2 -1

index b0b34..d3cd9 100644
@@ -2387,7 +2387,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) $(GENERATED_H)
 LOCALIZED_SH := $(SCRIPT_SH)

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jiang Xin

author

2016-06-15 22:54 UTC #2 lore raw

PATCH v4 2/7 i18n: rebase: mark strings for translation

Mark strings in git-rebase.sh for translation. Jonathan offers a help for reorgnization of the resolvemsg variable in 'git-rebase.sh', since there is a likely message in git-am.sh, I update it in this commit for consistency. And so does to 't/t0201-gettext-fallbacks.sh'.

Some test scripts are affected by this update, and would fail if tested with GETTEXT_POISON switch turned on. Using i18n-specific test functions, such as test_i18ngrep, in the related test scripts will fix these issues.

Signed-off-by Jiang Xin <[email protected]>

---
 git-am.sh                    |  6 ++---
 git-rebase.sh                | 64 +++++++++++++++++++++++---------------------
 t/t0201-gettext-fallbacks.sh |  8 +++---
 t/t3400-rebase.sh            |  8 +++---
 t/t3406-rebase-message.sh    |  2 +-
 5 files changed, 46 insertions(+), 42 deletions(-)

git-am.sh +3 -3

index c02e6..8961a 100755
@@ -102,9 +102,9 @@ stop_here_user_resolve () {
 	    printf '%s\n' "$resolvemsg"
 	    stop_here $1
     fi
-    eval_gettextln "When you have resolved this problem run \"\$cmdline --resolved\".
-If you would prefer to skip this patch, instead run \"\$cmdline --skip\".
-To restore the original branch and stop patching run \"\$cmdline --abort\"."
+    eval_gettextln "When you have resolved this problem, run \"\$cmdline --resolved\".
+If you prefer to skip this patch, run \"\$cmdline --skip\" instead.
+To restore the original branch and stop patching, run \"\$cmdline --abort\"."
 
     stop_here $1
 }

git-rebase.sh +34 ~~-30~~

184 lines not on this page · raw message

t/t0201-gettext-fallbacks.sh +4 -4

index 52b1c..5d80a 100755
@@ -51,16 +51,16 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v
 test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces' '
     cmdline="git am" &&
     export cmdline;
-    printf "When you have resolved this problem run git am --resolved." >expect &&
-    eval_gettext "When you have resolved this problem run \$cmdline --resolved." >actual
+    printf "When you have resolved this problem, run git am --resolved." >expect &&
+    eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual
     test_i18ncmp expect actual
 '
 
 test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces and quotes' '
     cmdline="git am" &&
     export cmdline;
-    printf "When you have resolved this problem run \"git am --resolved\"." >expect &&
-    eval_gettext "When you have resolved this problem run \"\$cmdline --resolved\"." >actual
+    printf "When you have resolved this problem, run \"git am --resolved\"." >expect &&
+    eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual
     test_i18ncmp expect actual
 '

t/t3400-rebase.sh +4 -4

index 7788a..1de0e 100755
@@ -68,24 +68,24 @@ test_expect_success 'rebase against master' '
 
 test_expect_success 'rebase against master twice' '
 	git rebase master >out &&
-	grep "Current branch my-topic-branch is up to date" out
+	test_i18ngrep "Current branch my-topic-branch is up to date" out
 '
 
 test_expect_success 'rebase against master twice with --force' '
 	git rebase --force-rebase master >out &&
-	grep "Current branch my-topic-branch is up to date, rebase forced" out
+	test_i18ngrep "Current branch my-topic-branch is up to date, rebase forced" out
 '
 
 test_expect_success 'rebase against master twice from another branch' '
 	git checkout my-topic-branch^ &&
 	git rebase master my-topic-branch >out &&
-	grep "Current branch my-topic-branch is up to date" out
+	test_i18ngrep "Current branch my-topic-branch is up to date" out
 '
 
 test_expect_success 'rebase fast-forward to master' '
 	git checkout my-topic-branch^ &&
 	git rebase my-topic-branch >out &&
-	grep "Fast-forwarded HEAD to my-topic-branch" out
+	test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
 '
 
 test_expect_success 'the rebase operation should not have destroyed author information' '

t/t3406-rebase-message.sh +1 -1

index 68983..6eb28 100755
@@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat config' '
 
 test_expect_success 'rebase --onto outputs the invalid ref' '
 	test_must_fail git rebase --onto invalid-ref HEAD HEAD 2>err &&
-	grep "invalid-ref" err
+	test_i18ngrep "invalid-ref" err
 '
 
 test_done

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jonathan Nieder

2016-06-15 22:54 UTC #3 lore raw

Re: [PATCH v4 2/7] i18n: rebase: mark strings for translation

(cc-ing Duy because of a mention of his nice GETTEXT_POISON tweak[*]) Hi,

Quoted 4 lines · Jiang Xin

Mark strings in git-rebase.sh for translation. Jonathan offers a help for reorgnization of the resolvemsg variable in 'git-rebase.sh', since there is a likely message in git-am.sh, I update it in this commit for consistency. And so does to 't/t0201-gettext-fallbacks.sh'.

Ah. Looks like I tweaked the comma usage and sentence structure a little. Sorry, force of habit --- I shouldn't have.

Quoted 4 lines

Some test scripts are affected by this update, and would fail if tested with GETTEXT_POISON switch turned on. Using i18n-specific test functions, such as test_i18ngrep, in the related test scripts will fix these issues.

If we're going to keep the changes together, here's how I would phrase the commit message:

Mark messages in git-rebase.sh for translation. While doing this it was noticed that the comma usage and sentence structure of the resolvemsg was not quite right, so correct that and its cousins in git-am.sh and t/t0201-gettext-fallbacks.sh at the same time.

Some tests would start to fail with GETTEXT_POISON turned on after this update. Use test_i18ncmp and test_i18ngrep where appropriate to mark strings that should only be checked in the C locale output to avoid such issues.

Quoted 3 lines

Signed-off-by Jiang Xin <[email protected]>
Reviewed-by Stefano Lattarini <[email protected]>
Reviewed-by Jonathan Nieder <[email protected]>

I haven't tested or reviewed this patch in detail, so even though it looks good, I'd prefer it not to have my Reviewed-by. (See Documentation/SubmittingPatches: '"Reviewed-by:", unlike the other extra tags, can only be offered by the reviewer'.) If you'd like to credit my help, something like "With advice from Jonathan." would be fine.

[...]

t/t3406-rebase-message.sh test_expect_success 'rebase -n overrides config rebase.stat config' ' · line 68

Quoted 9 lines

t/t3406-rebase-message.sh +1 -1

@@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat config' '
 
 test_expect_success 'rebase --onto outputs the invalid ref' '
 	test_must_fail git rebase --onto invalid-ref HEAD HEAD 2>err &&
-	grep "invalid-ref" err
+	test_i18ngrep "invalid-ref" err
 '

Could we add a comment so others do not have to wonder what human-readable message prompts the test_i18ngrep here? e.g

# "Does not point to a valid commit: invalid-ref" #
# NEEDSWORK: This "grep" is fine in real non-C locales, but # GETTEXT_POISON poisons the refname along with the enclosing # error message.
test_i18ngrep invalid-ref err

In the long run we may be able to turn this back to a "grep" again, since any reasonable translation will keep the $onto_name somewhere in the message. But changing it to test_i18ngrep for now is the right thing to do, until something like Duy's more sophisticated version of GETTEXT_POISON arrives. (<-- [*])

Thanks,
Jonathan

Jiang Xin

author

2016-06-15 22:54 UTC #4 lore raw

Re: [PATCH v4 2/7] i18n: rebase: mark strings for translation

2012/7/25 Jonathan Nieder <[email protected]>:

Quoted 6 lines

I haven't tested or reviewed this patch in detail, so even though it looks good, I'd prefer it not to have my Reviewed-by. (See Documentation/SubmittingPatches: '"Reviewed-by:", unlike the other extra tags, can only be offered by the reviewer'.) If you'd like to credit my help, something like "With advice from Jonathan." would be fine.

How about Acked-by: ?

Signature · 1 line

Jiang Xin

Jonathan Nieder

2016-06-15 22:54 UTC #5 lore raw

Re: [PATCH v4 2/7] i18n: rebase: mark strings for translation

Quoted 10 lines · Jiang Xin

2012/7/25 Jonathan Nieder <[email protected]>:
Quoted 6 lines
I haven't tested or reviewed this patch in detail, so even though it looks good, I'd prefer it not to have my Reviewed-by. (See Documentation/SubmittingPatches: '"Reviewed-by:", unlike the other extra tags, can only be offered by the reviewer'.) If you'd like to credit my help, something like "With advice from Jonathan." would be fine.
How about Acked-by: ?

I'm not area expert so my ack wouldn't count for "git rebase" or i18n, anyway. I'm also fine with going unsung --- too many names add up to so much noise, and in general there are some names (author, bug reporter, and so on) that are important and shouldn't be drowned out.

Jiang Xin

author

2016-06-15 22:54 UTC #6 lore raw

PATCH v4 3/7 i18n: Rewrite gettext messages start with dash

Gettext message in a shell script should not start with '-', one workaround is adding '--' between gettext and the message, like:

    gettext -- "--exec option ..."

But due to a bug in the xgettext extraction, xgettext can not extract the actual message for this case. Rewriting the message is a simpler and better solution.

Signed-off-by Jiang Xin <[email protected]>

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

Reported-by Vincent van Ravesteijn <[email protected]>

---
 git-rebase.sh                 | 2 +-
 git-submodule.sh              | 2 +-
 t/t3404-rebase-interactive.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

git-rebase.sh +1 -1

index 8710d..705bd 100755
@@ -317,7 +317,7 @@ test $# -gt 2 && usage
 if test -n "$cmd" &&
    test "$interactive_rebase" != explicit
 then
-	die "$(gettext -- "--exec option must be used with --interactive option")"
+	die "$(gettext "The --exec option must be used with the --interactive option")"
 fi
 
 if test -n "$action"

git-submodule.sh +1 -1

index dba4d..5b019 100755
@@ -748,7 +748,7 @@ cmd_summary() {
 	if [ -n "$files" ]
 	then
 		test -n "$cached" &&
-		die "$(gettext -- "--cached cannot be used with --files")"
+		die "$(gettext "The --cached option cannot be used with the --files option")"
 		diff_cmd=diff-files
 		head=
 	fi

t/t3404-rebase-interactive.sh +1 -1

index 8078..f206a 100755
@@ -858,7 +858,7 @@ test_expect_success 'rebase -ix with --autosquash' '
 test_expect_success 'rebase --exec without -i shows error message' '
 	git reset --hard execute &&
 	test_must_fail git rebase --exec "git show HEAD" HEAD~2 2>actual &&
-	echo "--exec option must be used with --interactive option" >expected &&
+	echo "The --exec option must be used with the --interactive option" >expected &&
 	test_i18ncmp expected actual
 '

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jiang Xin

author

2016-06-15 22:54 UTC #7 lore raw

PATCH v4 4/7 Remove obsolete LONG_USAGE which breaks xgettext

The obsolete LONG_USAGE variable has the following message in it:

    A'\''--B'\''--C'\''

And such complex LONG_USAGE message will breaks xgettext when extracting l10n messages. But if single quotes are removed from the message, xgettext works fine on 'git-rebase.sh'.

Since there is a modern OPTIONS_SPEC variable in use in this script, it's safe to remove the obsolete USAGE and LONG_USAGE variables.

Signed-off-by Jiang Xin <[email protected]>

---
 git-rebase.sh | 25 -------------------------
 1 file changed, 25 deletions(-)

git-rebase.sh +0 ~~-25~~

index 705bd..0e6fd 100755
@@ -3,31 +3,6 @@
 # Copyright (c) 2005 Junio C Hamano.
 #
 
-USAGE='[--interactive | -i] [--exec | -x <cmd>] [-v] [--force-rebase | -f]
-       [--no-ff] [--onto <newbase>] [<upstream>|--root] [<branch>] [--quiet | -q]'
-LONG_USAGE='git-rebase replaces <branch> with a new branch of the
-same name.  When the --onto option is provided the new branch starts
-out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
-It then attempts to create a new commit for each commit from the original
-<branch> that does not exist in the <upstream> branch.
-
-It is possible that a merge failure will prevent this process from being
-completely automatic.  You will have to resolve any such merge failure
-and run git rebase --continue.  Another option is to bypass the commit
-that caused the merge failure with git rebase --skip.  To check out the
-original <branch> and remove the .git/rebase-apply working files, use the
-command git rebase --abort instead.
-
-Note that if <branch> is not specified on the command line, the
-currently checked out branch is used.
-
-Example:       git-rebase master~1 topic
-
-	A---B---C topic                   A'\''--B'\''--C'\'' topic
-       /                   -->           /
-  D---E---F---G master          D---E---F---G master
-'
-
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jiang Xin

author

2016-06-15 22:54 UTC #8 lore raw

PATCH v4 5/7 i18n: am: mark more strings for translation

Mark strings in 'git-am.sh' for translation. In the last chunk, I changed '$1' to '-b/--binary' for this reason:

* First, if there is a variable in the l10n message, we could not use gettext. Because the variable will be expanded (evaluated) and will never match the entry in the po file.

* Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context.

Also reduce one indentation level for one gettextln clause introduced in commit de88c1c.

Signed-off-by Jiang Xin <[email protected]>

---
 git-am.sh | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

git-am.sh +5 -6

index 8961a..3f654 100755
@@ -92,7 +92,7 @@ safe_to_abort () {
 	then
 		return 0
 	fi
-		gettextln "You seem to have moved HEAD since the last 'am' failure.
+	gettextln "You seem to have moved HEAD since the last 'am' failure.
 Not rewinding to ORIG_HEAD" >&2
 	return 1
 }

@@ -136,7 +136,7 @@ fall_back_3way () {
     git write-tree >"$dotest/patch-merge-base+" ||
     cannot_fallback "$(gettext "Repository lacks necessary blobs to fall back on 3-way merge.")"
 
-    say Using index info to reconstruct a base tree...
+    say "$(gettext "Using index info to reconstruct a base tree...")"
 
     cmd='GIT_INDEX_FILE="$dotest/patch-merge-tmp-index"'

@@ -176,8 +176,7 @@ It does not apply to blobs recorded in its index.")"
     fi
     git-merge-recursive $orig_tree -- HEAD $his_tree || {
 	    git rerere $allow_rerere_autoupdate
-	    echo Failed to merge in the changes.
-	    exit 1
+	    die "$(gettext "Failed to merge in the changes.")"
     }
     unset GITHEAD_$his_tree
 }

@@ -387,8 +386,8 @@ do
 	-i|--interactive)
 		interactive=t ;;
 	-b|--binary)
-		echo >&2 "The $1 option has been a no-op for long time, and"
-		echo >&2 "it will be removed. Please do not use it anymore."
+		echo >&2 $(gettext "The -b/--binary option has been a no-op for long time, and
+it will be removed. Please do not use it anymore.")
 		;;
 	-3|--3way)
 		threeway=t ;;

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jonathan Nieder

Reviewed-by on v4 5/7

2016-06-15 22:54 UTC #9 lore raw

Re: [PATCH v4 5/7] i18n: am: mark more strings for translation

Quoted 11 lines · Jiang Xin

Mark strings in 'git-am.sh' for translation. In the last chunk, I changed '$1' to '-b/--binary' for this reason:
* First, if there is a variable in the l10n message, we could not use gettext. Because the variable will be expanded (evaluated) and will never match the entry in the po file.
* Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context.

Yes, I think it's a good change.

Quoted 2 lines

git-am.sh +0 -0

[...]

git-am.sh do · line 390

Quoted 8 lines

git-am.sh +2 -2

@@ -387,8 +386,8 @@ do
 	-i|--interactive)
 		interactive=t ;;
 	-b|--binary)
-		echo >&2 "The $1 option has been a no-op for long time, and"
-		echo >&2 "it will be removed. Please do not use it anymore."
+		echo >&2 $(gettext "The -b/--binary option has been a no-op for long time, and
+it will be removed. Please do not use it anymore.")

Could this be simplified to

		gettextln >&2 'The -b/--binary option...

... any more.'

gettextln 'The -b/--binary option ...
... any more.' >&2

or would that confuse xgettext? That would be more comforting than "echo" because if the translated string includes backslashes then the behavior of echo can be unpredictable.

Sorry I missed that last time.

The rest is indeed

Thanks.

Stefano Lattarini

2016-06-15 22:54 UTC #10 lore raw

Re: [PATCH v4 5/7] i18n: am: mark more strings for translation

Sorry to be so nit-picky, but ...

Quoted 12 lines · Jiang Xin

Mark strings in 'git-am.sh' for translation. In the last chunk, I changed '$1' to '-b/--binary' for this reason:
* First, if there is a variable in the l10n message, we could not use gettext. Because the variable will be expanded (evaluated) and will never match the entry in the po file.
* Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context.

... here you should s/it's/its/.

Regards,
Stefano

Jiang Xin

author

2016-06-15 22:54 UTC #11 lore raw

Re: [PATCH v4 5/7] i18n: am: mark more strings for translation

2012/7/25 Stefano Lattarini <[email protected]>:

Quoted 6 lines

Quoted 5 lines
* Second, if there is a positional parameter ($1, $2,...) in the message, we could not use eval_gettext either. Because eval_gettext may be a wapper for gettext, and the positional parameter would loose it's original context.
... here you should s/it's/its/.

OMG. You find what some grammer/spelling checking website hasn't. If there are no other serious problems, I won't make noises (send another series of patches) to this list, should I?

Signature · 1 line

Jiang Xin

Jiang Xin

author

2016-06-15 22:54 UTC #12 lore raw

PATCH v4 6/7 Remove dead code which contains bad gettext block

Found this dead code when I examine gettext messages in shell scripts start with dash ('-' or '--'). An error will be raised for this case, like:

    $ gettext "-d option is no longer supported.  Do not use."
    gettext: missing arguments

Indead, this code has been left as dead for a long time, as Junathan points out:

The git am -d/--dotest option has errored out with a message since e72c7406 (am: remove support for -d .dotest, 2008-03-04). The error message about lack of support was eliminated along with other cleanups (probably by mistake) a year later by removing the option from the option table in 98ef23b3 (git-am:
minor cleanups, 2009-01-28).

But the code to handle -d and --dotest stayed around even though ever since then it could not be tripped. Remove this dead code.

Signed-off-by Jiang Xin <[email protected]>

---
 git-am.sh | 3 ---
 1 file changed, 3 deletions(-)

git-am.sh +0 -3

index 3f654..c81a2 100755
@@ -413,9 +413,6 @@ it will be removed. Please do not use it anymore.")
 		abort=t ;;
 	--rebasing)
 		rebasing=t threeway=t ;;
-	-d|--dotest)
-		die "$(gettext "-d option is no longer supported.  Do not use.")"
-		;;
 	--resolvemsg)
 		shift; resolvemsg=$1 ;;
 	--whitespace|--directory|--exclude|--include)

Signature · 1 line

1.7.12.rc0.16.gf4916ac

Jonathan Nieder

Reviewed-by on v4 6/7

2016-06-15 22:54 UTC #13 lore raw

Re: [PATCH v4 6/7] Remove dead code which contains bad gettext block

Quoted 9 lines · Jiang Xin

Found this dead code when I examine gettext messages in shell scripts start with dash ('-' or '--'). An error will be raised for this case, like:
    $ gettext "-d option is no longer supported.  Do not use."
    gettext: missing arguments
Indead, this code has been left as dead for a long time, as Junathan points out:

Jonathan, not Junathan. :)

Quoted 11 lines

The git am -d/--dotest option has errored out with a message since e72c7406 (am: remove support for -d .dotest, 2008-03-04). The error message about lack of support was eliminated along with other cleanups (probably by mistake) a year later by removing the option from the option table in 98ef23b3 (git-am:
minor cleanups, 2009-01-28).
But the code to handle -d and --dotest stayed around even though ever since then it could not be tripped. Remove this dead code.
Signed-off-by Jiang Xin <[email protected]>

Your explanation is certainly clearer than mine. So: yes, for what it's worth this is

Jiang Xin

author

2016-06-15 22:54 UTC #14 lore raw

PATCH v4 7/7 i18n: merge-recursive: mark strings for translation

Mark strings in merge-recursive for translation.

Signed-off-by Jiang Xin <[email protected]>