# [PATCH 0/2] git-request-pull i18n 10 messages from 2021-09-16 to 2021-09-17. Participants: Bagas Sanjaya, Ævar Arnfjörð Bjarmason, Paolo Bonzini, Đoàn Trần Công Danh, Junio C Hamano, Miklos Vajna. Thread: https://gitlist.dev/t/56513 ## Bagas Sanjaya, 2021-09-16 11:35 Subject: [PATCH 0/2] git-request-pull i18n Message-ID: <20210916113516.76445-1-bagasdotme@gmail.com> URL: https://gitlist.dev/e/20210916113516.76445-1-bagasdotme%40gmail.com ``` 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 ``` ## Bagas Sanjaya, 2021-09-16 11:35 Subject: [PATCH 1/2] request-pull: simplify "remote or HEAD" variable in warning messages Message-ID: <20210916113516.76445-2-bagasdotme@gmail.com> URL: https://gitlist.dev/e/20210916113516.76445-2-bagasdotme%40gmail.com In-Reply-To: <20210916113516.76445-1-bagasdotme@gmail.com> ``` 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 Cc: pbonzini@redhat.com Signed-off-by: Bagas Sanjaya --- 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 ``` ## Bagas Sanjaya, 2021-09-16 11:35 Subject: [PATCH 2/2] request-pull: mark translatable strings Message-ID: <20210916113516.76445-3-bagasdotme@gmail.com> URL: https://gitlist.dev/e/20210916113516.76445-3-bagasdotme%40gmail.com In-Reply-To: <20210916113516.76445-1-bagasdotme@gmail.com> ``` Mark user-faced strings as translatable (including PR message output). Cc: Ryan Anderson Cc: vmiklos@frugalware.org Cc: bedhanger@gmx.de Signed-off-by: Bagas Sanjaya --- 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 ``` ## Ævar Arnfjörð Bjarmason, 2021-09-16 12:15 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: <87sfy4g1cm.fsf@evledraar.gmail.com> URL: https://gitlist.dev/e/87sfy4g1cm.fsf%40evledraar.gmail.com In-Reply-To: <20210916113516.76445-3-bagasdotme@gmail.com> ``` On Thu, Sep 16 2021, Bagas Sanjaya wrote: > Mark user-faced strings as translatable (including PR message output). > > Cc: Ryan Anderson > Cc: vmiklos@frugalware.org > Cc: bedhanger@gmx.de > Signed-off-by: Bagas Sanjaya > --- > 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, ``` ## Paolo Bonzini, 2021-09-16 13:09 Subject: Re: [PATCH 1/2] request-pull: simplify "remote or HEAD" variable in warning messages Message-ID: <02a3f76e-cb59-e4c7-21f2-3772be3cad09@redhat.com> URL: https://gitlist.dev/e/02a3f76e-cb59-e4c7-21f2-3772be3cad09%40redhat.com In-Reply-To: <20210916113516.76445-2-bagasdotme@gmail.com> ``` 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 > there?" to be translatable, replace ${remote:-HEAD} (variable access > with defaults) in the message by referring it as $remote_or_head. > > Cc: Ryan Anderson > Cc: pbonzini@redhat.com > Signed-off-by: Bagas Sanjaya > --- > 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 > > ``` ## Đoàn Trần Công Danh, 2021-09-16 13:44 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: URL: https://gitlist.dev/e/YUNKIj44AlW0tkXk%40danh.dev In-Reply-To: <20210916113516.76445-3-bagasdotme@gmail.com> ``` Beside the problems pointed out by Ævar: On 2021-09-16 18:35:17+0700, Bagas Sanjaya 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 ``` ## Junio C Hamano, 2021-09-16 20:30 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: URL: https://gitlist.dev/e/xmqqbl4stg91.fsf%40gitster.g In-Reply-To: ``` Đoàn Trần Công Danh 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=" 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. ``` ## Bagas Sanjaya, 2021-09-17 07:41 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: <187b4b89-e037-6103-08f4-870ce8f1e4fd@gmail.com> URL: https://gitlist.dev/e/187b4b89-e037-6103-08f4-870ce8f1e4fd%40gmail.com In-Reply-To: ``` 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=" 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 ``` ## Junio C Hamano, 2021-09-17 16:37 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: URL: https://gitlist.dev/e/xmqq7dffi2ed.fsf%40gitster.g In-Reply-To: <187b4b89-e037-6103-08f4-870ce8f1e4fd@gmail.com> ``` Bagas Sanjaya 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=" 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. ``` ## Miklos Vajna, 2021-09-17 20:50 Subject: Re: [PATCH 2/2] request-pull: mark translatable strings Message-ID: URL: https://gitlist.dev/e/YUT%2FkfsQnMaOQHRk%40vmiklos.hu In-Reply-To: ``` Hi, On Thu, Sep 16, 2021 at 01:30:50PM -0700, Junio C Hamano 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 ```