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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 16 Sep 2021 14:15:15 +0200

On Thu, Sep 16 2021, Bagas Sanjaya wrote:

> Mark user-faced strings as translatable (including PR message output).
>
> Cc: Ryan Anderson <[email-masked]>
> Cc: [email-masked] 
> Cc: [email-masked]
> Signed-off-by: Bagas Sanjaya <[email-masked]>
> ---
>  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,