[PATCH v3 0/7] i18n for git-am, git-rebase and git-merge

[PATCH v3 0/7] i18n for git-am, git-rebase and git-merge

[Jiang Xin]

Marked messages for translation in git-am, git-rebase, and git-merge.
Also fixed affected test cases when turn GETTEXT_POISON switch on.

Jiang Xin (7):
  i18n: New keywords for xgettext extraction from sh
  i18n: rebase: mark strings for translation
  i18n: Rewrite gettext messages start with dash
  Remove obsolete LONG_USAGE which breaks xgettext
  i18n: am: mark more strings for translation
  Remove unused and bad gettext block from git-am
  i18n: merge-recursive: mark strings for translation

 Makefile                             |   3 +-
 git-am.sh                            |  14 ++--
 git-rebase.sh                        |  87 ++++++++------------
 git-submodule.sh                     |   2 +-
 merge-recursive.c                    | 148 +++++++++++++++++++----------------
 t/t3400-rebase.sh                    |   8 +-
 t/t3404-rebase-interactive.sh        |   2 +-
 t/t3406-rebase-message.sh            |   2 +-
 t/t6022-merge-rename.sh              |  16 ++--
 t/t6042-merge-rename-corner-cases.sh |   2 +-
 10 files changed, 134 insertions(+), 150 deletions(-)

-- 
1.7.12.rc0.17.gcb766d3

[PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh

[Jiang Xin]

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

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

diff --git a/Makefile b/Makefile
index b0b34..d3cd9 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
-- 
1.7.12.rc0.17.gcb766d3

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

[Jiang Xin]

Mark strings in git-rebase.sh for translation.

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

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 git-rebase.sh             | 62 ++++++++++++++++++++++++-----------------------
 t/t3400-rebase.sh         |  8 +++---
 t/t3406-rebase-message.sh |  2 +-
 3 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 1cd06..f07269 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -65,6 +65,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 "
 . git-sh-setup
+. git-sh-i18n
 set_reflog_action rebase
 require_work_tree_exists
 cd_to_toplevel
@@ -72,11 +73,11 @@ cd_to_toplevel
 LF='
 '
 ok_to_skip_pre_rebase=
-resolvemsg="
-When you have resolved this problem run \"git rebase --continue\".
-If you would prefer to skip this patch, instead run \"git rebase --skip\".
-To check out the original branch and stop rebasing run \"git rebase --abort\".
-"
+resolvemsg=$(gettext '
+When you have resolved this problem run "git rebase --continue".
+If you would prefer to skip this patch, instead run "git rebase --skip".
+To check out the original branch and stop rebasing run "git rebase --abort".
+')
 unset onto
 cmd=
 strategy=
@@ -161,7 +162,7 @@ move_to_original_branch () {
 		git symbolic-ref \
 			-m "rebase finished: returning to $head_name" \
 			HEAD $head_name ||
-		die "Could not move back to $head_name"
+		die "$(gettext "Could not move back to $head_name")"
 		;;
 	esac
 }
@@ -180,12 +181,12 @@ run_pre_rebase_hook () {
 	   test -x "$GIT_DIR/hooks/pre-rebase"
 	then
 		"$GIT_DIR/hooks/pre-rebase" ${1+"$@"} ||
-		die "The pre-rebase hook refused to rebase."
+		die "$(gettext "The pre-rebase hook refused to rebase.")"
 	fi
 }
 
 test -f "$apply_dir"/applying &&
-	die 'It looks like git-am is in progress. Cannot rebase.'
+	die "$(gettext "It looks like git-am is in progress. Cannot rebase.")"
 
 if test -d "$apply_dir"
 then
@@ -316,12 +317,12 @@ test $# -gt 2 && usage
 if test -n "$cmd" &&
    test "$interactive_rebase" != explicit
 then
-	die "--exec option must be used with --interactive option"
+	die "$(gettext -- "--exec option must be used with --interactive option")"
 fi
 
 if test -n "$action"
 then
-	test -z "$in_progress" && die "No rebase in progress?"
+	test -z "$in_progress" && die "$(gettext "No rebase in progress?")"
 	# Only interactive rebase uses detailed reflog messages
 	if test "$type" = interactive && test "$GIT_REFLOG_ACTION" = rebase
 	then
@@ -334,11 +335,11 @@ case "$action" in
 continue)
 	# Sanity check
 	git rev-parse --verify HEAD >/dev/null ||
-		die "Cannot read HEAD"
+		die "$(gettext "Cannot read HEAD")"
 	git update-index --ignore-submodules --refresh &&
 	git diff-files --quiet --ignore-submodules || {
-		echo "You must edit all merge conflicts and then"
-		echo "mark them as resolved using git add"
+		echo "$(gettext "You must edit all merge conflicts and then
+mark them as resolved using git add")"
 		exit 1
 	}
 	read_basic_state
@@ -355,7 +356,7 @@ abort)
 	case "$head_name" in
 	refs/*)
 		git symbolic-ref -m "rebase: aborting" HEAD $head_name ||
-		die "Could not move back to $head_name"
+		die "$(eval_gettext "Could not move back to \$head_name")"
 		;;
 	esac
 	output git reset --hard $orig_head
@@ -367,15 +368,16 @@ esac
 # Make sure no rebase is in progress
 if test -n "$in_progress"
 then
-	die '
-It seems that there is already a '"${state_dir##*/}"' directory, and
+	state_dir_base=${state_dir##*/}
+	die "$(eval_gettext "
+It seems that there is already a \$state_dir_base directory, and
 I wonder if you are in the middle of another rebase.  If that is the
 case, please try
 	git rebase (--continue | --abort | --skip)
 If that is not the case, please
-	rm -fr '"$state_dir"'
+	rm -fr \"\$state_dir\"
 and run me again.  I am stopping in case you still have something
-valuable there.'
+valuable there.")"
 fi
 
 if test -n "$rebase_root" && test -z "$onto"
@@ -413,7 +415,7 @@ then
 		;;
 	esac
 	upstream=`git rev-parse --verify "${upstream_name}^0"` ||
-	die "invalid upstream $upstream_name"
+	die "$(eval_gettext "invalid upstream \$upstream_name")"
 	upstream_arg="$upstream_name"
 else
 	if test -z "$onto"
@@ -437,19 +439,19 @@ case "$onto_name" in
 	then
 		case "$onto" in
 		?*"$LF"?*)
-			die "$onto_name: there are more than one merge bases"
+			die "$(eval_gettext "\$onto_name: there are more than one merge bases")"
 			;;
 		'')
-			die "$onto_name: there is no merge base"
+			die "$(eval_gettext "\$onto_name: there is no merge base")"
 			;;
 		esac
 	else
-		die "$onto_name: there is no merge base"
+		die "$(eval_gettext "\$onto_name: there is no merge base")"
 	fi
 	;;
 *)
 	onto=$(git rev-parse --verify "${onto_name}^0") ||
-	die "Does not point to a valid commit: $onto_name"
+	die "$(eval_gettext "Does not point to a valid commit: \$onto_name")"
 	;;
 esac
 
@@ -472,7 +474,7 @@ case "$#" in
 	then
 		head_name="detached HEAD"
 	else
-		die "fatal: no such branch: $1"
+		die "$(eval_gettext "fatal: no such branch: \$branch_name")"
 	fi
 	;;
 0)
@@ -492,7 +494,7 @@ case "$#" in
 	;;
 esac
 
-require_clean_work_tree "rebase" "Please commit or stash them."
+require_clean_work_tree "rebase" "$(gettext "Please commit or stash them.")"
 
 # Now we are rebasing commits $upstream..$orig_head (or with --root,
 # everything leading up to $orig_head) on top of $onto
@@ -510,10 +512,10 @@ then
 	then
 		# Lazily switch to the target branch if needed...
 		test -z "$switch_to" || git checkout "$switch_to" --
-		say "Current branch $branch_name is up to date."
+		say "$(eval_gettext "Current branch \$branch_name is up to date.")"
 		exit 0
 	else
-		say "Current branch $branch_name is up to date, rebase forced."
+		say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
 	fi
 fi
 
@@ -524,7 +526,7 @@ if test -n "$diffstat"
 then
 	if test -n "$verbose"
 	then
-		echo "Changes from $mb to $onto:"
+		echo "$(eval_gettext "Changes from \$mb to \$onto:")"
 	fi
 	# We want color (if set), but no pager
 	GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
@@ -533,7 +535,7 @@ fi
 test "$type" = interactive && run_specific_rebase
 
 # Detach HEAD and reset the tree
-say "First, rewinding head to replay your work on top of it..."
+say "$(gettext "First, rewinding head to replay your work on top of it...")"
 git checkout -q "$onto^0" || die "could not detach HEAD"
 git update-ref ORIG_HEAD $orig_head
 
@@ -541,7 +543,7 @@ git update-ref ORIG_HEAD $orig_head
 # we just fast-forwarded.
 if test "$mb" = "$orig_head"
 then
-	say "Fast-forwarded $branch_name to $onto_name."
+	say "$(eval_gettext "Fast-forwarded \$branch_name to \$onto_name.")"
 	move_to_original_branch
 	exit 0
 fi
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 7788a..1de0e 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -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' '
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 68983..6eb28 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -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
-- 
1.7.12.rc0.17.gcb766d3

[PATCH v3 3/7] i18n: Rewrite gettext messages start with dash

[Jiang Xin]

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 <worldhello.net@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reported-by: Vincent van Ravesteijn <vfr@lyx.org>
Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
 git-rebase.sh                 | 2 +-
 git-submodule.sh              | 2 +-
 t/t3404-rebase-interactive.sh | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index f07269..09a3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -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"
diff --git a/git-submodule.sh b/git-submodule.sh
index dba4d..5b019 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -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
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078..f206a 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -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
 '
 
-- 
1.7.12.rc0.17.gcb766d3

[PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext

[Jiang Xin]

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 <worldhello.net@gmail.com>
Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
 git-rebase.sh | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 09a3..63485 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -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="\
-- 
1.7.12.rc0.17.gcb766d3

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

[Jiang Xin]

Mark additional 3 strings for translation, and reduce one indentation
level for one gettextln clause introduced in commit de88c1c.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
 git-am.sh | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index c02e6..b7183 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -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 option has been a no-op for long time, and
+it will be removed. Please do not use it anymore.")
 		;;
 	-3|--3way)
 		threeway=t ;;
-- 
1.7.12.rc0.17.gcb766d3

[PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Jiang Xin]

Gettext message should not start with '-' nor '--'. Since the '-d' and
'--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to
remove the block.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
 git-am.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index b7183..a2e3d 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -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)
-- 
1.7.12.rc0.17.gcb766d3

[PATCH v3 7/7] i18n: merge-recursive: mark strings for translation

[Jiang Xin]

Mark strings in merge-recursive for translation.

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

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 merge-recursive.c                    | 148 +++++++++++++++++++----------------
 t/t6022-merge-rename.sh              |  16 ++--
 t/t6042-merge-rename-corner-cases.sh |   2 +-
 3 files changed, 88 insertions(+), 78 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 68093..8903 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -187,7 +187,7 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
 	else {
 		printf("%s ", find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
 		if (parse_commit(commit) != 0)
-			printf("(bad commit)\n");
+			printf(_("(bad commit)\n"));
 		else {
 			const char *title;
 			int len = find_commit_subject(commit->buffer, &title);
@@ -203,7 +203,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 	struct cache_entry *ce;
 	ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, refresh);
 	if (!ce)
-		return error("addinfo_cache failed for path '%s'", path);
+		return error(_("addinfo_cache failed for path '%s'"), path);
 	return add_cache_entry(ce, options);
 }
 
@@ -265,7 +265,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 	if (!cache_tree_fully_valid(active_cache_tree) &&
 	    cache_tree_update(active_cache_tree,
 			      active_cache, active_nr, 0) < 0)
-		die("error building trees");
+		die(_("error building trees"));
 
 	result = lookup_tree(active_cache_tree->sha1);
 
@@ -494,7 +494,7 @@ static struct string_list *get_renames(struct merge_options *o,
 	opts.show_rename_progress = o->show_rename_progress;
 	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	if (diff_setup_done(&opts) < 0)
-		die("diff setup failed");
+		die(_("diff setup failed"));
 	diff_tree_sha1(o_tree->object.sha1, tree->object.sha1, "", &opts);
 	diffcore_std(&opts);
 	if (opts.needed_rename_limit > o->needed_rename_limit)
@@ -624,7 +624,7 @@ static void flush_buffer(int fd, const char *buf, unsigned long size)
 				break;
 			die_errno("merge-recursive");
 		} else if (!ret) {
-			die("merge-recursive: disk full?");
+			die(_("merge-recursive: disk full?"));
 		}
 		size -= ret;
 		buf += ret;
@@ -687,7 +687,7 @@ static int would_lose_untracked(const char *path)
 static int make_room_for_path(struct merge_options *o, const char *path)
 {
 	int status, i;
-	const char *msg = "failed to create path '%s'%s";
+	const char *msg = _("failed to create path '%s'%s");
 
 	/* Unlink any D/F conflict files that are in the way */
 	for (i = 0; i < o->df_conflict_file_set.nr; i++) {
@@ -698,7 +698,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
 		    path[df_pathlen] == '/' &&
 		    strncmp(path, df_path, df_pathlen) == 0) {
 			output(o, 3,
-			       "Removing %s to make room for subdirectory\n",
+			       _("Removing %s to make room for subdirectory\n"),
 			       df_path);
 			unlink(df_path);
 			unsorted_string_list_delete_item(&o->df_conflict_file_set,
@@ -712,7 +712,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
 	if (status) {
 		if (status == -3) {
 			/* something else exists */
-			error(msg, path, ": perhaps a D/F conflict?");
+			error(msg, path, _(": perhaps a D/F conflict?"));
 			return -1;
 		}
 		die(msg, path, "");
@@ -723,7 +723,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
 	 * tracking it.
 	 */
 	if (would_lose_untracked(path))
-		return error("refusing to lose untracked file at '%s'",
+		return error(_("refusing to lose untracked file at '%s'"),
 			     path);
 
 	/* Successful unlink is good.. */
@@ -733,7 +733,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
 	if (errno == ENOENT)
 		return 0;
 	/* .. but not some other error (who really cares what?) */
-	return error(msg, path, ": perhaps a D/F conflict?");
+	return error(msg, path, _(": perhaps a D/F conflict?"));
 }
 
 static void update_file_flags(struct merge_options *o,
@@ -763,9 +763,9 @@ static void update_file_flags(struct merge_options *o,
 
 		buf = read_sha1_file(sha, &type, &size);
 		if (!buf)
-			die("cannot read object %s '%s'", sha1_to_hex(sha), path);
+			die(_("cannot read object %s '%s'"), sha1_to_hex(sha), path);
 		if (type != OBJ_BLOB)
-			die("blob expected for %s '%s'", sha1_to_hex(sha), path);
+			die(_("blob expected for %s '%s'"), sha1_to_hex(sha), path);
 		if (S_ISREG(mode)) {
 			struct strbuf strbuf = STRBUF_INIT;
 			if (convert_to_working_tree(path, buf, size, &strbuf)) {
@@ -788,7 +788,7 @@ static void update_file_flags(struct merge_options *o,
 				mode = 0666;
 			fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
 			if (fd < 0)
-				die_errno("failed to open '%s'", path);
+				die_errno(_("failed to open '%s'"), path);
 			flush_buffer(fd, buf, size);
 			close(fd);
 		} else if (S_ISLNK(mode)) {
@@ -796,10 +796,10 @@ static void update_file_flags(struct merge_options *o,
 			safe_create_leading_directories_const(path);
 			unlink(path);
 			if (symlink(lnk, path))
-				die_errno("failed to symlink '%s'", path);
+				die_errno(_("failed to symlink '%s'"), path);
 			free(lnk);
 		} else
-			die("do not know what to do with %06o %s '%s'",
+			die(_("do not know what to do with %06o %s '%s'"),
 			    mode, sha1_to_hex(sha), path);
 		free(buf);
 	}
@@ -936,11 +936,11 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
 						  branch1, branch2);
 
 			if ((merge_status < 0) || !result_buf.ptr)
-				die("Failed to execute internal merge");
+				die(_("Failed to execute internal merge"));
 
 			if (write_sha1_file(result_buf.ptr, result_buf.size,
 					    blob_type, result.sha))
-				die("Unable to add %s to database",
+				die(_("Unable to add %s to database"),
 				    a->path);
 
 			free(result_buf.ptr);
@@ -956,7 +956,7 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
 			if (!sha_eq(a->sha1, b->sha1))
 				result.clean = 0;
 		} else {
-			die("unsupported object type in the tree");
+			die(_("unsupported object type in the tree"));
 		}
 	}
 
@@ -1034,22 +1034,32 @@ static void handle_change_delete(struct merge_options *o,
 		remove_file_from_cache(path);
 		update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
 	} else if (!a_sha) {
-		output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
-		       "and %s in %s. Version %s of %s left in tree%s%s.",
-		       change, path, o->branch1,
-		       change_past, o->branch2, o->branch2, path,
-		       NULL == renamed ? "" : " at ",
-		       NULL == renamed ? "" : renamed);
-		update_file(o, 0, b_sha, b_mode, renamed ? renamed : path);
+		if (!renamed) {
+			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+			       "and %s in %s. Version %s of %s left in tree."),
+			       change, path, o->branch1, change_past,
+			       o->branch2, o->branch2, path);
+			update_file(o, 0, b_sha, b_mode, path);
+		} else {
+			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+			       "and %s in %s. Version %s of %s left in tree at %s."),
+			       change, path, o->branch1, change_past,
+			       o->branch2, o->branch2, path, renamed);
+			update_file(o, 0, b_sha, b_mode, renamed);
+		}
 	} else {
-		output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
-		       "and %s in %s. Version %s of %s left in tree%s%s.",
-		       change, path, o->branch2,
-		       change_past, o->branch1, o->branch1, path,
-		       NULL == renamed ? "" : " at ",
-		       NULL == renamed ? "" : renamed);
-		if (renamed)
+		if (!renamed) {
+			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+			       "and %s in %s. Version %s of %s left in tree."),
+			       change, path, o->branch2, change_past,
+			       o->branch1, o->branch1, path);
+		} else {
+			output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
+			       "and %s in %s. Version %s of %s left in tree at %s."),
+			       change, path, o->branch2, change_past,
+			       o->branch1, o->branch1, path, renamed);
 			update_file(o, 0, a_sha, a_mode, renamed);
+		}
 		/*
 		 * No need to call update_file() on path when !renamed, since
 		 * that would needlessly touch path.  We could call
@@ -1085,7 +1095,7 @@ static void conflict_rename_delete(struct merge_options *o,
 			     orig->sha1, orig->mode,
 			     a_sha, a_mode,
 			     b_sha, b_mode,
-			     "rename", "renamed");
+			     _("rename"), _("renamed"));
 
 	if (o->call_depth) {
 		remove_file_from_cache(dest->path);
@@ -1141,7 +1151,7 @@ static void handle_file(struct merge_options *o,
 	} else {
 		if (dir_in_way(rename->path, !o->call_depth)) {
 			dst_name = unique_path(o, rename->path, cur_branch);
-			output(o, 1, "%s is a directory in %s adding as %s instead",
+			output(o, 1, _("%s is a directory in %s adding as %s instead"),
 			       rename->path, other_branch, dst_name);
 		}
 	}
@@ -1163,12 +1173,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
 	struct diff_filespec *a = ci->pair1->two;
 	struct diff_filespec *b = ci->pair2->two;
 
-	output(o, 1, "CONFLICT (rename/rename): "
+	output(o, 1, _("CONFLICT (rename/rename): "
 	       "Rename \"%s\"->\"%s\" in branch \"%s\" "
-	       "rename \"%s\"->\"%s\" in \"%s\"%s",
+	       "rename \"%s\"->\"%s\" in \"%s\"%s"),
 	       one->path, a->path, ci->branch1,
 	       one->path, b->path, ci->branch2,
-	       o->call_depth ? " (left unresolved)" : "");
+	       o->call_depth ? _(" (left unresolved)") : "");
 	if (o->call_depth) {
 		struct merge_file_info mfi;
 		struct diff_filespec other;
@@ -1222,9 +1232,9 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
 	struct merge_file_info mfi_c1;
 	struct merge_file_info mfi_c2;
 
-	output(o, 1, "CONFLICT (rename/rename): "
+	output(o, 1, _("CONFLICT (rename/rename): "
 	       "Rename %s->%s in %s. "
-	       "Rename %s->%s in %s",
+	       "Rename %s->%s in %s"),
 	       a->path, c1->path, ci->branch1,
 	       b->path, c2->path, ci->branch2);
 
@@ -1252,7 +1262,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
 	} else {
 		char *new_path1 = unique_path(o, path, ci->branch1);
 		char *new_path2 = unique_path(o, path, ci->branch2);
-		output(o, 1, "Renaming %s to %s and %s to %s instead",
+		output(o, 1, _("Renaming %s to %s and %s to %s instead"),
 		       a->path, new_path1, b->path, new_path2);
 		remove_file(o, 0, path, 0);
 		update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1);
@@ -1451,8 +1461,8 @@ static int process_renames(struct merge_options *o,
 			} else if (!sha_eq(dst_other.sha1, null_sha1)) {
 				clean_merge = 0;
 				try_merge = 1;
-				output(o, 1, "CONFLICT (rename/add): Rename %s->%s in %s. "
-				       "%s added in %s",
+				output(o, 1, _("CONFLICT (rename/add): Rename %s->%s in %s. "
+				       "%s added in %s"),
 				       ren1_src, ren1_dst, branch1,
 				       ren1_dst, branch2);
 				if (o->call_depth) {
@@ -1461,12 +1471,12 @@ static int process_renames(struct merge_options *o,
 							 ren1->pair->two->sha1, ren1->pair->two->mode,
 							 dst_other.sha1, dst_other.mode,
 							 branch1, branch2);
-					output(o, 1, "Adding merged %s", ren1_dst);
+					output(o, 1, _("Adding merged %s"), ren1_dst);
 					update_file(o, 0, mfi.sha, mfi.mode, ren1_dst);
 					try_merge = 0;
 				} else {
 					char *new_path = unique_path(o, ren1_dst, branch2);
-					output(o, 1, "Adding as %s instead", new_path);
+					output(o, 1, _("Adding as %s instead"), new_path);
 					update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
 					free(new_path);
 				}
@@ -1517,10 +1527,10 @@ static int read_sha1_strbuf(const unsigned char *sha1, struct strbuf *dst)
 	unsigned long size;
 	buf = read_sha1_file(sha1, &type, &size);
 	if (!buf)
-		return error("cannot read object %s", sha1_to_hex(sha1));
+		return error(_("cannot read object %s"), sha1_to_hex(sha1));
 	if (type != OBJ_BLOB) {
 		free(buf);
-		return error("object %s is not a blob", sha1_to_hex(sha1));
+		return error(_("object %s is not a blob"), sha1_to_hex(sha1));
 	}
 	strbuf_attach(dst, buf, size, size + 1);
 	return 0;
@@ -1568,7 +1578,7 @@ static void handle_modify_delete(struct merge_options *o,
 			     o_sha, o_mode,
 			     a_sha, a_mode,
 			     b_sha, b_mode,
-			     "modify", "modified");
+			     _("modify"), _("modified"));
 }
 
 static int merge_content(struct merge_options *o,
@@ -1578,14 +1588,14 @@ static int merge_content(struct merge_options *o,
 			 unsigned char *b_sha, int b_mode,
 			 struct rename_conflict_info *rename_conflict_info)
 {
-	const char *reason = "content";
+	const char *reason = _("content");
 	const char *path1 = NULL, *path2 = NULL;
 	struct merge_file_info mfi;
 	struct diff_filespec one, a, b;
 	unsigned df_conflict_remains = 0;
 
 	if (!o_sha) {
-		reason = "add/add";
+		reason = _("add/add");
 		o_sha = (unsigned char *)null_sha1;
 	}
 	one.path = a.path = b.path = (char *)path;
@@ -1619,7 +1629,7 @@ static int merge_content(struct merge_options *o,
 	if (mfi.clean && !df_conflict_remains &&
 	    sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) {
 		int path_renamed_outside_HEAD;
-		output(o, 3, "Skipped %s (merged same as existing)", path);
+		output(o, 3, _("Skipped %s (merged same as existing)"), path);
 		/*
 		 * The content merge resulted in the same file contents we
 		 * already had.  We can return early if those file contents
@@ -1633,12 +1643,12 @@ static int merge_content(struct merge_options *o,
 			return mfi.clean;
 		}
 	} else
-		output(o, 2, "Auto-merging %s", path);
+		output(o, 2, _("Auto-merging %s"), path);
 
 	if (!mfi.clean) {
 		if (S_ISGITLINK(mfi.mode))
-			reason = "submodule";
-		output(o, 1, "CONFLICT (%s): Merge conflict in %s",
+			reason = _("submodule");
+		output(o, 1, _("CONFLICT (%s): Merge conflict in %s"),
 				reason, path);
 		if (rename_conflict_info && !df_conflict_remains)
 			update_stages(path, &one, &a, &b);
@@ -1664,7 +1674,7 @@ static int merge_content(struct merge_options *o,
 
 		}
 		new_path = unique_path(o, path, rename_conflict_info->branch1);
-		output(o, 1, "Adding as %s instead", new_path);
+		output(o, 1, _("Adding as %s instead"), new_path);
 		update_file(o, 0, mfi.sha, mfi.mode, new_path);
 		free(new_path);
 		mfi.clean = 0;
@@ -1728,7 +1738,7 @@ static int process_entry(struct merge_options *o,
 			/* Deleted in both or deleted in one and
 			 * unchanged in the other */
 			if (a_sha)
-				output(o, 2, "Removing %s", path);
+				output(o, 2, _("Removing %s"), path);
 			/* do not touch working file if it did not exist */
 			remove_file(o, 1, path, !a_sha);
 		} else {
@@ -1753,19 +1763,19 @@ static int process_entry(struct merge_options *o,
 			other_branch = o->branch2;
 			mode = a_mode;
 			sha = a_sha;
-			conf = "file/directory";
+			conf = _("file/directory");
 		} else {
 			add_branch = o->branch2;
 			other_branch = o->branch1;
 			mode = b_mode;
 			sha = b_sha;
-			conf = "directory/file";
+			conf = _("directory/file");
 		}
 		if (dir_in_way(path, !o->call_depth)) {
 			char *new_path = unique_path(o, path, add_branch);
 			clean_merge = 0;
-			output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
-			       "Adding %s as %s",
+			output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
+			       "Adding %s as %s"),
 			       conf, path, other_branch, path, new_path);
 			if (o->call_depth)
 				remove_file_from_cache(path);
@@ -1774,7 +1784,7 @@ static int process_entry(struct merge_options *o,
 				remove_file_from_cache(path);
 			free(new_path);
 		} else {
-			output(o, 2, "Adding %s", path);
+			output(o, 2, _("Adding %s"), path);
 			/* do not overwrite file if already present */
 			update_file_flags(o, sha, mode, path, 1, !a_sha);
 		}
@@ -1791,7 +1801,7 @@ static int process_entry(struct merge_options *o,
 		 */
 		remove_file(o, 1, path, !a_mode);
 	} else
-		die("Fatal merge failure, shouldn't happen.");
+		die(_("Fatal merge failure, shouldn't happen."));
 
 	return clean_merge;
 }
@@ -1810,7 +1820,7 @@ int merge_trees(struct merge_options *o,
 	}
 
 	if (sha_eq(common->object.sha1, merge->object.sha1)) {
-		output(o, 0, "Already up-to-date!");
+		output(o, 0, _("Already up-to-date!"));
 		*result = head;
 		return 1;
 	}
@@ -1819,7 +1829,7 @@ int merge_trees(struct merge_options *o,
 
 	if (code != 0) {
 		if (show(o, 4) || o->call_depth)
-			die("merging of trees %s and %s failed",
+			die(_("merging of trees %s and %s failed"),
 			    sha1_to_hex(head->object.sha1),
 			    sha1_to_hex(merge->object.sha1));
 		else
@@ -1849,7 +1859,7 @@ int merge_trees(struct merge_options *o,
 		for (i = 0; i < entries->nr; i++) {
 			struct stage_data *e = entries->items[i].util;
 			if (!e->processed)
-				die("Unprocessed path??? %s",
+				die(_("Unprocessed path??? %s"),
 				    entries->items[i].string);
 		}
 
@@ -1894,7 +1904,7 @@ int merge_recursive(struct merge_options *o,
 	int clean;
 
 	if (show(o, 4)) {
-		output(o, 4, "Merging:");
+		output(o, 4, _("Merging:"));
 		output_commit_title(o, h1);
 		output_commit_title(o, h2);
 	}
@@ -1905,7 +1915,7 @@ int merge_recursive(struct merge_options *o,
 	}
 
 	if (show(o, 5)) {
-		output(o, 5, "found %u common ancestor(s):", commit_list_count(ca));
+		output(o, 5, _("found %u common ancestor(s):"), commit_list_count(ca));
 		for (iter = ca; iter; iter = iter->next)
 			output_commit_title(o, iter->item);
 	}
@@ -1941,7 +1951,7 @@ int merge_recursive(struct merge_options *o,
 		o->call_depth--;
 
 		if (!merged_common_ancestors)
-			die("merge returned no commit");
+			die(_("merge returned no commit"));
 	}
 
 	discard_cache();
@@ -1998,7 +2008,7 @@ int merge_recursive_generic(struct merge_options *o,
 		for (i = 0; i < num_base_list; ++i) {
 			struct commit *base;
 			if (!(base = get_ref(base_list[i], sha1_to_hex(base_list[i]))))
-				return error("Could not parse object '%s'",
+				return error(_("Could not parse object '%s'"),
 					sha1_to_hex(base_list[i]));
 			commit_list_insert(base, &ca);
 		}
@@ -2010,7 +2020,7 @@ int merge_recursive_generic(struct merge_options *o,
 	if (active_cache_changed &&
 			(write_cache(index_fd, active_cache, active_nr) ||
 			 commit_locked_index(lock)))
-		return error("Unable to write index.");
+		return error(_("Unable to write index."));
 
 	return clean ? 0 : 1;
 }
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 11042..c680f 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -242,10 +242,10 @@ test_expect_success 'merge of identical changes in a renamed file' '
 	rm -f A M N &&
 	git reset --hard &&
 	git checkout change+rename &&
-	GIT_MERGE_VERBOSITY=3 git merge change | grep "^Skipped B" &&
+	GIT_MERGE_VERBOSITY=3 git merge change | test_i18ngrep "^Skipped B" &&
 	git reset --hard HEAD^ &&
 	git checkout change &&
-	GIT_MERGE_VERBOSITY=3 git merge change+rename | grep "^Skipped B"
+	GIT_MERGE_VERBOSITY=3 git merge change+rename | test_i18ngrep "^Skipped B"
 '
 
 test_expect_success 'setup for rename + d/f conflicts' '
@@ -303,9 +303,9 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 	git checkout -q renamed-file-has-no-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-in-way >output &&
 
-	grep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
-	grep "Auto-merging dir" output &&
-	grep "Adding as dir~HEAD instead" output &&
+	test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
+	test_i18ngrep "Auto-merging dir" output &&
+	test_i18ngrep "Adding as dir~HEAD instead" output &&
 
 	test 3 -eq "$(git ls-files -u | wc -l)" &&
 	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
@@ -325,9 +325,9 @@ test_expect_success 'Same as previous, but merged other way' '
 	test_must_fail git merge --strategy=recursive renamed-file-has-no-conflicts >output 2>errors &&
 
 	! grep "error: refusing to lose untracked file at" errors &&
-	grep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
-	grep "Auto-merging dir" output &&
-	grep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
+	test_i18ngrep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
+	test_i18ngrep "Auto-merging dir" output &&
+	test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
 
 	test 3 -eq "$(git ls-files -u | wc -l)" &&
 	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 466fa3..4115 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -380,7 +380,7 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' '
 	git checkout B^0 &&
 
 	test_must_fail git merge -s recursive C^0 >out &&
-	grep "CONFLICT (rename/rename)" out &&
+	test_i18ngrep "CONFLICT (rename/rename)" out &&
 
 	test 2 -eq $(git ls-files -s | wc -l) &&
 	test 2 -eq $(git ls-files -u | wc -l) &&
-- 
1.7.12.rc0.17.gcb766d3

Re: [PATCH v3 7/7] i18n: merge-recursive: mark strings for translation

[Stefano Lattarini]

On 07/24/2012 08:59 AM, Jiang Xin wrote:
> Mark strings in merge-recursive for translation.
> 
> Some test scripts are affected by this update, and would fail if are
> tested with GETTEXT_POISON switch turned on. Use i18n-specific test
> functions, such as test_i18ngrep in the related test scripts will fix
> these issues.
>
The same comments I made for [PATCH 2/7] in this series apply here.

Regards,
  Stefano

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Jonathan Nieder]

Hi,

Jiang Xin wrote:

> Gettext message should not start with '-' nor '--'. Since the '-d' and
> '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to
> remove the block.

The above justification is not a sufficient reason to stop giving
helpful advice when someone uses an option that was historically
supported:

> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -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.")"
> -		;;

Luckily the support was removed 4 years ago and I don't think anyone
is going to run into this, so a different justification could apply.

	Support for the git am -d/--dotest option was removed four years ago
	(see e72c7406, "am: remove support for -d .dotest", 2008-03-04) and
	presumably no one is trying to use it any more.  Simplify the
	code and free the short-and-sweet "-d" option for other uses by
	no longer parsing it.

	The motivation: xgettext copes poorly with messages starting
	with '-'.  Rather than fixing this ancient message, let's take
	this as a reminder to remove it.

See http://thread.gmane.org/gmane.comp.version-control.git/75896 for
context.  Though also see
http://thread.gmane.org/gmane.comp.security.selinux/1424/focus=1430

Hope that helps,
Jonathan

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> -	-d|--dotest)
>> -		die "$(gettext "-d option is no longer supported.  Do not use.")"
>> -		;;
>
> Luckily the support was removed 4 years ago and I don't think anyone
> is going to run into this, so a different justification could apply.

Still I'd prefer a deprecation/removal not buried in an unrelated
topic.  Can we just leave it untranslated, and send a removal patch
during pre 1.8.0 timeframe?

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Martin von Zweigbergk]

On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Jiang Xin wrote:
>
>> Gettext message should not start with '-' nor '--'. Since the '-d' and
>> '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to
>> remove the block.
>
> The above justification is not a sufficient reason to stop giving
> helpful advice when someone uses an option that was historically
> supported:

I think Jiang is saying that "git am --dotest=..." already errors out
because "dotest" is not in the OPTIONS_SPEC. See 98ef23b (git-am:
minor cleanups, 2009-01-28). Or am I missing something?

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Hi,
>>
>> Jiang Xin wrote:
>>
>>> Gettext message should not start with '-' nor '--'. Since the '-d' and
>>> '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to
>>> remove the block.
>>
>> The above justification is not a sufficient reason to stop giving
>> helpful advice when someone uses an option that was historically
>> supported:
>
> I think Jiang is saying that "git am --dotest=..." already errors out
> because "dotest" is not in the OPTIONS_SPEC. See 98ef23b (git-am:
> minor cleanups, 2009-01-28). Or am I missing something?

No you are not missing anything, I would think.

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Jonathan Nieder]

Martin von Zweigbergk wrote:
> On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Jiang Xin wrote:

>>> Gettext message should not start with '-' nor '--'. Since the '-d' and
>>> '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to
>>> remove the block.
>>
>> The above justification is not a sufficient reason to stop giving
>> helpful advice when someone uses an option that was historically
>> supported:
>
> I think Jiang is saying that "git am --dotest=..." already errors out
> because "dotest" is not in the OPTIONS_SPEC. See 98ef23b (git-am:
> minor cleanups, 2009-01-28).

Oh, I see now.

I guess that means the intended justification is the following?

	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.

	Noticed because the error message starts with a '-' and
	xgettext does not cope well with that.

Re: [PATCH v3 6/7] Remove unused and bad gettext block from git-am

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> I guess that means the intended justification is the following?
>
> 	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.
>
> 	Noticed because the error message starts with a '-' and
> 	xgettext does not cope well with that.

Looks good.

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

[Jonathan Nieder]

Hi,

Jiang Xin wrote:

> Mark additional 3 strings for translation, and reduce one indentation
> level for one gettextln clause introduced in commit de88c1c.

The above description doesn't mention:

[...]
> @@ -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 option has been a no-op for long time, and
> +it will be removed. Please do not use it anymore.")

... that this changes the message when the --binary option is passed.
Before this patch, it says

	The --binary option has been a no-op for a long time, and ...

After the patch, it says

	The -b option has been a no-op for a long time, and ...

Intentional?  That may be a good change or a bad one (I haven't
thought clearly about it), but it seems at least worth mentioning.
Cc-ing Thomas in case he has advice.

Thanks,
Jonathan

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>>  	-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 option has been a no-op for long time, and
>> +it will be removed. Please do not use it anymore.")
>
> ... that this changes the message when the --binary option is passed.
> Before this patch, it says
>
> 	The --binary option has been a no-op for a long time, and ...
>
> After the patch, it says
>
> 	The -b option has been a no-op for a long time, and ...
>
> Intentional?  That may be a good change or a bad one (I haven't
> thought clearly about it), but it seems at least worth mentioning.
> Cc-ing Thomas in case he has advice.

If we really care we could printf $1, but I think we usually do

	The -b/--binary option has been...

in a case like this, especially in codepaths that no longer has an
easy access to $1 after parsing the command line but knows that
either one of them is given from the parse result, and that would be
an appropriate solution for this particular one as well.

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

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Before this patch, it says
>>
>> 	The --binary option has been a no-op for a long time, and ...
>>
>> After the patch, it says
>>
>> 	The -b option has been a no-op for a long time, and ...
>>
>> Intentional?  That may be a good change or a bad one (I haven't
>> thought clearly about it), but it seems at least worth mentioning.
>> Cc-ing Thomas in case he has advice.
>
> If we really care we could printf $1, but I think we usually do
>
> 	The -b/--binary option has been...
>
> in a case like this, especially in codepaths that no longer has an
> easy access to $1 after parsing the command line but knows that
> either one of them is given from the parse result, and that would be
> an appropriate solution for this particular one as well.

Yes.  The original plan was to free up -b, but since we don't need
--binary any more either, it's better if we can get rid of it the same
way.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Jiang Xin]

2012/7/25 Jonathan Nieder <jrnieder@gmail.com>:
>>       -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 option has been a no-op for long time, and
>> +it will be removed. Please do not use it anymore.")
>
> ... that this changes the message when the --binary option is passed.
> Before this patch, it says
>
>         The --binary option has been a no-op for a long time, and ...
>
> After the patch, it says
>
>         The -b option has been a no-op for a long time, and ...
>
> Intentional?  That may be a good change or a bad one (I haven't
> thought clearly about it), but it seems at least worth mentioning.
> Cc-ing Thomas in case he has advice.

It's intentional.

 * First, if a variable in the message, we could not use gettext,
   for the variable will be expanded (evaluated) and never match
   the entry in 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.

-- 
Jiang Xin

Re: [PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext

[Jonathan Nieder]

Hi,

Jiang Xin wrote:

> The obsolete LONG_USAGE variable
[...]

It's a shame to lose the information that was in the LONG_USAGE
message, though.  Maybe it could be incorporated into the OPTIONS_SPEC
before the opening "--", or maybe it could be used to clarify the
description in git-rebase(1).

Cc-ing Martin who carried out the parseoptification for advice.  Patch
left unsnipped below for reference.

Thanks and hope that helps,
Jonathan

[...]
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -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="\
> -- 
> 1.7.12.rc0.17.gcb766d3
> 

Re: [PATCH v3 4/7] Remove obsolete LONG_USAGE which breaks xgettext

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Jiang Xin wrote:
>
>> The obsolete LONG_USAGE variable
> [...]
>
> It's a shame to lose the information that was in the LONG_USAGE
> message, though.  Maybe it could be incorporated into the OPTIONS_SPEC
> before the opening "--", or maybe it could be used to clarify the
> description in git-rebase(1).

I personally think the original long-usage was overkill to be part
of the help text, and I am happy to see it go.

I wouldn't mind seeing it incorporated in the documentation if there
is something in there that is missing, but I suspect that the first
part of the DESCRIPTION should be sufficiently clear already.

> [...]
>> --- a/git-rebase.sh
>> +++ b/git-rebase.sh
>> @@ -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="\
>> -- 
>> 1.7.12.rc0.17.gcb766d3
>> 

Re: [PATCH v3 3/7] i18n: Rewrite gettext messages start with dash

[Stefano Lattarini]

On 07/24/2012 08:59 AM, Jiang Xin wrote:
> Gettext message in a shell script should not start with '-', one
>
I think using a semicolon instead of a comma after "one" here would
be clearer.

> 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.
> 

Regards,
  Stefano

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

[Stefano Lattarini]

On 07/24/2012 08:59 AM, Jiang Xin wrote:
> Mark strings in git-rebase.sh for translation.
> 
> Some test scripts are affected by this update, and would fail if are
>
s/if are/if/

> tested with GETTEXT_POISON switch turned on. Use i18n-specific test
>
s/Use/Using/, or s/Use/Use of/

> functions, such as test_i18ngrep
>
Missing comma after 'test_i18ngrep' here.

> in the related test scripts will fix these issues.
> 

Regards,
  Stefano

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

[Jonathan Nieder]

Hi,

Jiang Xin wrote:

> Mark strings in git-rebase.sh for translation.

Thanks.

[...]
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -65,6 +65,7 @@ abort!             abort and check out the original branch
>  skip!              skip current patch and continue
>  "
>  . git-sh-setup
> +. git-sh-i18n
>  set_reflog_action rebase
>  require_work_tree_exists
>  cd_to_toplevel
> @@ -72,11 +73,11 @@ cd_to_toplevel
>  LF='
>  '
>  ok_to_skip_pre_rebase=
> -resolvemsg="
> -When you have resolved this problem run \"git rebase --continue\".
> -If you would prefer to skip this patch, instead run \"git rebase --skip\".
> -To check out the original branch and stop rebasing run \"git rebase --abort\".
> -"
> +resolvemsg=$(gettext '
> +When you have resolved this problem run "git rebase --continue".
> +If you would prefer to skip this patch, instead run "git rebase --skip".
> +To check out the original branch and stop rebasing run "git rebase --abort".
> +')

Functional change: command substitution strips off the trailing newline.
Intentional?

Probably it would make sense to do

	resolvemsg="
	$(gettext 'When you have resolved this problem, run "git rebase --continue".
	If you prefer to skip this patch, run "git rebase --skip" instead.
	To check out the original branch and stop rebasing, run "git rebase --abort".')
	"

anyway, so the translators could have fewer newlines at the edges to
fuss about.

[...]
>  	git diff-files --quiet --ignore-submodules || {
> -		echo "You must edit all merge conflicts and then"
> -		echo "mark them as resolved using git add"
> +		echo "$(gettext "You must edit all merge conflicts and then
> +mark them as resolved using git add")"
>  		exit 1

Nice.

[...]
> @@ -367,15 +368,16 @@ esac
>  # Make sure no rebase is in progress
>  if test -n "$in_progress"
>  then
> -	die '
> -It seems that there is already a '"${state_dir##*/}"' directory, and
> +	state_dir_base=${state_dir##*/}
> +	die "$(eval_gettext "
> +It seems that there is already a \$state_dir_base directory, and
>  I wonder if you are in the middle of another rebase.  If that is the
>  case, please try
>  	git rebase (--continue | --abort | --skip)
>  If that is not the case, please
> -	rm -fr '"$state_dir"'
> +	rm -fr \"\$state_dir\"
>  and run me again.  I am stopping in case you still have something
> -valuable there.'
> +valuable there.")"

Maybe, to allow changing the commands without having to update
translations:

	state_dir_base=...
	cmd_live_rebase='git rebase (--continue | --abort | --skip)'
	cmd_clear_stale_rebase="rm -fr \"$state_dir\""
	die "
	$(eval_gettext 'It seems that there is already a $state_dir_base directory, and
	I wonder if you ware in the middle of another rebase.  If that is the
	case, please try
		$cmd_live_rebase
	If that is not the case, please
		$cmd_clear_stale_rebase
	and run me again.  I am stopping in case you still have something
	valuable there.')"

[...]
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -68,24 +68,24 @@ test_expect_success 'rebase against master' '

Thanks for updating tests!  The expected output you had to change all
seems to be intended for humans, which is a good sign.

[...]
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -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
>  '

This is probably part of a message intended for humans, but the test
does not say.  What is the full message being checked?

Hope that helps,
Jonathan

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Probably it would make sense to do
>
> 	resolvemsg="
> 	$(gettext 'When you have resolved this problem, run "git rebase --continue".
> 	If you prefer to skip this patch, run "git rebase --skip" instead.
> 	To check out the original branch and stop rebasing, run "git rebase --abort".')
> 	"
>
> anyway, so the translators could have fewer newlines at the edges to
> fuss about.

Nice.

> Maybe, to allow changing the commands without having to update
> translations:
>
> 	state_dir_base=...
> 	cmd_live_rebase='git rebase (--continue | --abort | --skip)'
> 	cmd_clear_stale_rebase="rm -fr \"$state_dir\""
> 	die "
> 	$(eval_gettext 'It seems that there is already a $state_dir_base directory, and
> 	I wonder if you ware in the middle of another rebase.  If that is the
> 	case, please try
> 		$cmd_live_rebase
> 	If that is not the case, please
> 		$cmd_clear_stale_rebase
> 	and run me again.  I am stopping in case you still have something
> 	valuable there.')"

Again, nice.

Thanks.

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

[Jiang Xin]

2012/7/25 Jonathan Nieder <jrnieder@gmail.com>:
>> @@ -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
>>  '
>
> This is probably part of a message intended for humans, but the test
> does not say.  What is the full message being checked?
>

The error messages are:

    fatal: Needed a single revision
    Does not point to a valid commit: invalid-ref

The first line has not been marked for translation (comes from
builtin/rev-parse.c), but the second line is marked for translation
in this patch. The output in my locale would be:

    fatal: Needed a single revision
    没有指向一个有效的提交：invalid-ref

-- 
Jiang Xin

Re: [PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh

[Stefano Lattarini]

On 07/24/2012 08:59 AM, Jiang Xin wrote:
> Since we have additional shell wrappers (gettextln and eval_gettextln)
> for gettext, we need to take into account these wrappers when run
>
s/when run/when running/ or s/when we run/.

Sorry for not spotting that in my first review!

> 'make pot' to extract messages from shell scripts.
> 

Regards,
  Stefano

Re: [PATCH v3 1/7] i18n: New keywords for xgettext extraction from sh

[Jonathan Nieder]

Jiang Xin wrote:

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

Yes, thanks for fixing it.

As Stefano mentioned, s/run/running/ would make the above clearer.

With or without that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>