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

[Jonathan Nieder <jrnieder@gmail.com>]

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