[PATCH 0/2] git-request-pull i18n

[PATCH 0/2] git-request-pull i18n

[Bagas Sanjaya]

Internationalize git-request-pull.

The use case is when sending pull request message to developer mailing
lists that accepts contributions in non-English languages (for example
"foo-dev-it" for Foo developer list in Italian).

The first patch is small preparation to make translatable strings can be
marked as such, while the second patch is actual marking.

Bagas Sanjaya (2):
  request-pull: simplify "remote or HEAD" variable in warning messages
  request-pull: mark translatable strings

 git-request-pull.sh | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)


base-commit: 4c719308ce59dc70e606f910f40801f2c6051b24
-- 
2.25.1

[PATCH 1/2] request-pull: simplify "remote or HEAD" variable in warning messages

[Bagas Sanjaya]

In order for waring message "Are you sure you pushed 'remote or HEAD'
there?" to be translatable, replace ${remote:-HEAD} (variable access
with defaults) in the message by referring it as $remote_or_head.

Cc: Ryan Anderson <ryan@michonline.com>
Cc: pbonzini@redhat.com
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 git-request-pull.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 2d0e44656c..9e1d2be9eb 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -116,15 +116,17 @@ set fnord $(git ls-remote "$url" | @@PERL@@ -e "$find_matching_ref" "${remote:-H
 remote_sha1=$2
 ref=$3
 
+remote_or_head=${remote:-HEAD}
+
 if test -z "$ref"
 then
 	echo "warn: No match for commit $headrev found at $url" >&2
-	echo "warn: Are you sure you pushed '${remote:-HEAD}' there?" >&2
+	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
 	status=1
 elif test "$local_sha1" != "$remote_sha1"
 then
 	echo "warn: $head found at $url but points to a different object" >&2
-	echo "warn: Are you sure you pushed '${remote:-HEAD}' there?" >&2
+	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
 	status=1
 fi
 
-- 
2.25.1

[PATCH 2/2] request-pull: mark translatable strings

[Bagas Sanjaya]

Mark user-faced strings as translatable (including PR message output).

Cc: Ryan Anderson <ryan@michonline.com>
Cc: vmiklos@frugalware.org 
Cc: bedhanger@gmx.de
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 git-request-pull.sh | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 9e1d2be9eb..8aa3a3f342 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -40,7 +40,7 @@ test -n "$base" && test -n "$url" || usage
 baserev=$(git rev-parse --verify --quiet "$base"^0)
 if test -z "$baserev"
 then
-    die "fatal: Not a valid revision: $base"
+    die "$(eval_gettext "fatal: Not a valid revision: \$base")"
 fi
 
 #
@@ -58,12 +58,12 @@ head=${head:-$(git show-ref --heads --tags "$local" | cut -d' ' -f2)}
 head=${head:-$(git rev-parse --quiet --verify "$local")}
 
 # None of the above? Bad.
-test -z "$head" && die "fatal: Not a valid revision: $local"
+test -z "$head" && die "$(eval_gettext "fatal: Not a valid revision: \$local")"
 
 # This also verifies that the resulting head is unique:
 # "git show-ref" could have shown multiple matching refs..
 headrev=$(git rev-parse --verify --quiet "$head"^0)
-test -z "$headrev" && die "fatal: Ambiguous revision: $local"
+test -z "$headrev" && die "$(eval_gettext "fatal: Ambiguous revision: \$local")"
 
 local_sha1=$(git rev-parse --verify --quiet "$head")
 
@@ -76,7 +76,7 @@ then
 fi
 
 merge_base=$(git merge-base $baserev $headrev) ||
-die "fatal: No commits in common between $base and $head"
+die "$(eval_gettext "fatal: No commits in common between \$base and \$head")"
 
 # $head is the refname from the command line.
 # Find a ref with the same name as $head that exists at the remote
@@ -120,13 +120,13 @@ remote_or_head=${remote:-HEAD}
 
 if test -z "$ref"
 then
-	echo "warn: No match for commit $headrev found at $url" >&2
-	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
+	echo "$(eval_gettext "warn: No match for commit \$headrev found at \$url")" >&2
+	echo "$(eval_gettext "warn: Are you sure you pushed '\$remote_or_head' there?")" >&2
 	status=1
 elif test "$local_sha1" != "$remote_sha1"
 then
-	echo "warn: $head found at $url but points to a different object" >&2
-	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
+	echo "$(eval_gettext "warn: \$head found at \$url but points to a different object")" >&2
+	echo "$(eval_gettext "warn: Are you sure you pushed '\$remote_or_head' there?")" >&2
 	status=1
 fi
 
@@ -138,19 +138,22 @@ fi
 
 url=$(git ls-remote --get-url "$url")
 
-git show -s --format='The following changes since commit %H:
+git show -s --format="
+$(gettext 'The following changes since commit %H:
 
   %s (%ci)
 
 are available in the Git repository at:
-' $merge_base &&
+')
+" $merge_base &&
 echo "  $url $pretty_remote" &&
-git show -s --format='
+git show -s --format="
+$(gettext '
 for you to fetch changes up to %H:
 
   %s (%ci)
 
-----------------------------------------------------------------' $headrev &&
+----------------------------------------------------------------')" $headrev &&
 
 if test $(git cat-file -t "$head") = tag
 then
@@ -162,7 +165,7 @@ fi &&
 
 if test -n "$branch_name"
 then
-	echo "(from the branch description for $branch_name local branch)"
+	echo "$(eval_gettext "(from the branch description for \$branch_name local branch)")"
 	echo
 	git config "branch.$branch_name.description"
 	echo "----------------------------------------------------------------"
-- 
2.25.1

Re: [PATCH 2/2] request-pull: mark translatable strings

[Ævar Arnfjörð Bjarmason]

On Thu, Sep 16 2021, Bagas Sanjaya wrote:

> Mark user-faced strings as translatable (including PR message output).
>
> Cc: Ryan Anderson <ryan@michonline.com>
> Cc: vmiklos@frugalware.org 
> Cc: bedhanger@gmx.de
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>  git-request-pull.sh | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 9e1d2be9eb..8aa3a3f342 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -40,7 +40,7 @@ test -n "$base" && test -n "$url" || usage
>  baserev=$(git rev-parse --verify --quiet "$base"^0)
>  if test -z "$baserev"
>  then
> -    die "fatal: Not a valid revision: $base"
> +    die "$(eval_gettext "fatal: Not a valid revision: \$base")"
>  fi
>  
>  #
> @@ -58,12 +58,12 @@ head=${head:-$(git show-ref --heads --tags "$local" | cut -d' ' -f2)}
>  head=${head:-$(git rev-parse --quiet --verify "$local")}
>  
>  # None of the above? Bad.
> -test -z "$head" && die "fatal: Not a valid revision: $local"
> +test -z "$head" && die "$(eval_gettext "fatal: Not a valid revision: \$local")"
>  
>  # This also verifies that the resulting head is unique:
>  # "git show-ref" could have shown multiple matching refs..
>  headrev=$(git rev-parse --verify --quiet "$head"^0)
> -test -z "$headrev" && die "fatal: Ambiguous revision: $local"
> +test -z "$headrev" && die "$(eval_gettext "fatal: Ambiguous revision: \$local")"
>  
>  local_sha1=$(git rev-parse --verify --quiet "$head")
>  
> @@ -76,7 +76,7 @@ then
>  fi
>  
>  merge_base=$(git merge-base $baserev $headrev) ||
> -die "fatal: No commits in common between $base and $head"
> +die "$(eval_gettext "fatal: No commits in common between \$base and \$head")"

Looks good.

>  # $head is the refname from the command line.
>  # Find a ref with the same name as $head that exists at the remote
> @@ -120,13 +120,13 @@ remote_or_head=${remote:-HEAD}
>  
>  if test -z "$ref"
>  then
> -	echo "warn: No match for commit $headrev found at $url" >&2
> -	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
> +	echo "$(eval_gettext "warn: No match for commit \$headrev found at \$url")" >&2
> +	echo "$(eval_gettext "warn: Are you sure you pushed '\$remote_or_head' there?")" >&2
>  	status=1
>  elif test "$local_sha1" != "$remote_sha1"
>  then
> -	echo "warn: $head found at $url but points to a different object" >&2
> -	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
> +	echo "$(eval_gettext "warn: \$head found at \$url but points to a different object")" >&2
> +	echo "$(eval_gettext "warn: Are you sure you pushed '\$remote_or_head' there?")" >&2
>  	status=1
>  fi

Messages like these should probably be combined into one this one's
mostly on the edge, but the "are you sure" reads like a continuation of
the "no match for" or "$head found at" sentence, so translators may want
to re-orderthat wording...

> @@ -138,19 +138,22 @@ fi
>  
>  url=$(git ls-remote --get-url "$url")
>  
> -git show -s --format='The following changes since commit %H:
> +git show -s --format="
> +$(gettext 'The following changes since commit %H:
>  

The newline added at the start here looks like a bug or unrelated
change.

>    %s (%ci)
>  
>  are available in the Git repository at:
> -' $merge_base &&
> +')
> +" $merge_base &&

And this likewise looks like an unrelated formatting change.

>  echo "  $url $pretty_remote" &&
> -git show -s --format='
> +git show -s --format="
> +$(gettext '

And likewise here maybe we want to include the first \n?

>  for you to fetch changes up to %H:
>  
>    %s (%ci)
>  
> -----------------------------------------------------------------' $headrev &&
> +----------------------------------------------------------------')" $headrev &&
>  
>  if test $(git cat-file -t "$head") = tag
>  then
> @@ -162,7 +165,7 @@ fi &&
>  
>  if test -n "$branch_name"
>  then
> -	echo "(from the branch description for $branch_name local branch)"
> +	echo "$(eval_gettext "(from the branch description for \$branch_name local branch)")"
>  	echo
>  	git config "branch.$branch_name.description"
>  	echo "----------------------------------------------------------------"

The rest looks good/correct,

Re: [PATCH 1/2] request-pull: simplify "remote or HEAD" variable in warning messages

[Paolo Bonzini]

On 16/09/21 13:35, Bagas Sanjaya wrote:
> In order for waring message "Are you sure you pushed 'remote or HEAD'
                ^^^^^^

warning

Otherwise,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> there?" to be translatable, replace ${remote:-HEAD} (variable access
> with defaults) in the message by referring it as $remote_or_head.
> 
> Cc: Ryan Anderson <ryan@michonline.com>
> Cc: pbonzini@redhat.com
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>   git-request-pull.sh | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 2d0e44656c..9e1d2be9eb 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -116,15 +116,17 @@ set fnord $(git ls-remote "$url" | @@PERL@@ -e "$find_matching_ref" "${remote:-H
>   remote_sha1=$2
>   ref=$3
>   
> +remote_or_head=${remote:-HEAD}
> +
>   if test -z "$ref"
>   then
>   	echo "warn: No match for commit $headrev found at $url" >&2
> -	echo "warn: Are you sure you pushed '${remote:-HEAD}' there?" >&2
> +	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
>   	status=1
>   elif test "$local_sha1" != "$remote_sha1"
>   then
>   	echo "warn: $head found at $url but points to a different object" >&2
> -	echo "warn: Are you sure you pushed '${remote:-HEAD}' there?" >&2
> +	echo "warn: Are you sure you pushed '$remote_or_head' there?" >&2
>   	status=1
>   fi
>   
> 

Re: [PATCH 2/2] request-pull: mark translatable strings

[Đoàn Trần Công Danh]

Beside the problems pointed out by Ævar:

On 2021-09-16 18:35:17+0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> Mark user-faced strings as translatable (including PR message output).

I would argue request-pull message shouldn't be translated.

The person who creates the request may prefer to use a different
language, let's say French, for day-to-day work.

However, the recipients may not understand French, and prefer to
receive English message.

And this change break their workflow badly.

> @@ -138,19 +138,22 @@ fi
>  
>  url=$(git ls-remote --get-url "$url")
>  
> -git show -s --format='The following changes since commit %H:
> +git show -s --format="
> +$(gettext 'The following changes since commit %H:
>  
>    %s (%ci)
>  
>  are available in the Git repository at:
> -' $merge_base &&
> +')

Hence, I think this message shouldn't be translated.

> +" $merge_base &&
>  echo "  $url $pretty_remote" &&
> -git show -s --format='
> +git show -s --format="
> +$(gettext '
>  for you to fetch changes up to %H:
>  
>    %s (%ci)

And neither should this message.

>  
> -----------------------------------------------------------------' $headrev &&
> +----------------------------------------------------------------')" $headrev &&
>  
>  if test $(git cat-file -t "$head") = tag
>  then
> @@ -162,7 +165,7 @@ fi &&
>  
>  if test -n "$branch_name"
>  then
> -	echo "(from the branch description for $branch_name local branch)"
> +	echo "$(eval_gettext "(from the branch description for \$branch_name local branch)")"
>  	echo
>  	git config "branch.$branch_name.description"
>  	echo "----------------------------------------------------------------"

Ditto.

-- 
Danh

Re: [PATCH 2/2] request-pull: mark translatable strings

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> I would argue request-pull message shouldn't be translated.
>
> The person who creates the request may prefer to use a different
> language, let's say French, for day-to-day work.
>
> However, the recipients may not understand French, and prefer to
> receive English message.
>
> And this change break their workflow badly.

[jc: devil's advocate hat on]

While that may be true, it would be a nice-to-have if we had an
option to help developers who usually work in $French when they
contribute to a project where $French is the official tongue (assign
any value other than English to variable $French).

[jc: devil's advocate hat off]

I haven't done or seen any official survey, but I would not be
surprised if English were used as the official project language by
the majority of projects that accept pull request messages.

In that sense, the output that gets translated for the user's usual
locale by default, like the patch in question does, is misdesigned.
The consequence of the design is that among those who do not usually
run in C or en_XX locale, the number of people who will be forced to
say

	LANG=C LC_ALL=C git request-pull ...

to override their usual local in order to send the untranslated
message to their project that do not want translated requests would
be far greater than those who can just say

	git request-pull ...

to send a message in local language to a local project.

So a good middle ground may be

 - allow translation, like these patches attempt

 - introduce the command line option "--l10n=<value>" and
   the requestpull.l10n configuration variable that gives the
   default for the option:

   - when it is set to 'true', end-user's local taken from the
     environment is used as the target for translation.

   - when it is set to 'false', translation is turned off.

   - when it is set to any other value, the locale is set to the
     value of that variable (imagine a Japanese developer
     contributing to a German project).

perhaps?   I dunno.

Re: [PATCH 2/2] request-pull: mark translatable strings

[Bagas Sanjaya]

On 17/09/21 03.30, Junio C Hamano wrote:
> So a good middle ground may be
> 
>   - allow translation, like these patches attempt
> 
>   - introduce the command line option "--l10n=<value>" and
>     the requestpull.l10n configuration variable that gives the
>     default for the option:
> 
>     - when it is set to 'true', end-user's local taken from the
>       environment is used as the target for translation.
> 
>     - when it is set to 'false', translation is turned off.
> 
>     - when it is set to any other value, the locale is set to the
>       value of that variable (imagine a Japanese developer
>       contributing to a German project).
> 
> perhaps?   I dunno.
> 

I'm leaning towards second option.

However, I proposed that --l10n and corresponding config 
requestpull.l10n just take locale value set, and defaults to English 
(en_US or C) if empty.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 2/2] request-pull: mark translatable strings

[Junio C Hamano]

Bagas Sanjaya <bagasdotme@gmail.com> writes:

> On 17/09/21 03.30, Junio C Hamano wrote:
>> So a good middle ground may be
>>   - allow translation, like these patches attempt
>>   - introduce the command line option "--l10n=<value>" and
>>     the requestpull.l10n configuration variable that gives the
>>     default for the option:
>>     - when it is set to 'true', end-user's local taken from the
>>       environment is used as the target for translation.
>>     - when it is set to 'false', translation is turned off.
>>     - when it is set to any other value, the locale is set to the
>>       value of that variable (imagine a Japanese developer
>>       contributing to a German project).
>> perhaps?   I dunno.
>> 
>
> I'm leaning towards second option.

I didn't give that many options for there to exist the second one,
though ;-)

> However, I proposed that --l10n and corresponding config
> requestpull.l10n just take locale value set, and defaults to English 
> (en_US or C) if empty.

I do not quite see merit in that tweak over what I outlined before,
though.

But all of the above depends on the assumption that it is a good use
of our engineering bandwidth to make request-pull localizable, and
more importantly if the "C locale is much more appropriate than the
local one when it comes to request-pull" is important enough to make
it behave quite differently from other subcommands in our toolbox.

To put it differently, my "I dunno" above still stands---I am not
sure if that is a _good_ middle ground, even though it is a middle
ground.

Thanks.

Re: [PATCH 2/2] request-pull: mark translatable strings

[Miklos Vajna]

Hi,

On Thu, Sep 16, 2021 at 01:30:50PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> I haven't done or seen any official survey, but I would not be
> surprised if English were used as the official project language by
> the majority of projects that accept pull request messages.

That would be also my expectation.

But I don't have a strong opinion on this, I already have LC_MESSAGES=C
configured, and I assume many developers have a similar setting (to e.g.
get easily searchable error messages).

Regards,

Miklos