| Message ID | 20210916113516.76445-3-bagasdotme@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-request-pull i18n | expand |
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,
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.
Đ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.
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.
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.
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
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 "----------------------------------------------------------------"
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(-)