Re: [PATCH v3 13/39] i18n: git-sh-setup.sh: mark strings for translation

Subsystems: the rest

2 messages, 2 authors, 2016-06-16 · open the first message on its own page

Re: [PATCH v3 13/39] i18n: git-sh-setup.sh: mark strings for translation

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:42

Junio C Hamano [off-list ref] writes:
That is correct, but doesn't that hint that the current users of
git-sh-i18n no longer have to (and they probably do not want to)
dot-source it, as they are already dot-sourcing git-sh-setup?

Otherwise they will waste cycles reading the same thing twice
(assuming that git-sh-i18n is safely read twice, that is).
This part still stands.

If we assume that it is a good idea to i18n shell functions that are
supplied by git-sh-setup, it might be a better direction to retire
git-sh-i18n and include its text directly in git-sh-setup, so that
any scripted Porcelain has to open just one file to dot-source it,
instead of two.
quoted
Source "$(git --exec-path)"/git-sh-i18n instead of simply git-sh-i18n,
That is somewhat strange.  "git-rebase.sh" dot-sources both
git-sh-setup and git-sh-i18n with just ". $name".  What is so
special about sh-setup dot-sourcing sh-i18n your patch adds?
quoted
because latter case would fail test t2300-cd-to-toplevel.sh.
What is the real cause of this?

". git-sh-setup" is supposed to work because we prepend the
$GIT_EXEC_PATH in "git" itself and git-sh-setup is installed there.
". git-sh-i18n" in "git-rebase.sh" works the same way.  If it is
because is t2300 doing something strange, then we should fix that,
not adding $(git --exec-path) on the dot-source line to paper things
over.
This because what t2300 does is strange; I wouldn't call it "wrong"
per se, but it is borderline wrong.

Would it allow you to lose the $(git --exec-path) prefix in the new
dot-source to have this patch before applying your patch?

-- >8 --
Subject: t2300: run git-sh-setup in an environment that better mimics the real life

When we run scripted Porcelains, "git" potty has set up the $PATH by
prepending $GIT_EXEC_PATH, the path given by "git --exec-path=$there
$cmd", etc. already.  Because of this, scripted Porcelains can
dot-source shell script library like git-sh-setup with simple dot
without specifying any path.

t2300 however dot-sources git-sh-setup without adjusting $PATH like
the real "git" potty does.  This has not been a problem so far, but
once git-sh-setup wants to rely on the $PATH adjustment, just like
any scripted Porcelains already do, it would become one.  It cannot
for example dot-source another shell library without specifying the
full path to it by prefixing $(git --exec-path).

Signed-off-by: Junio C Hamano <redacted>
---
 t/t2300-cd-to-toplevel.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index 9965bc5..cccd7d9 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -8,7 +8,8 @@ test_cd_to_toplevel () {
 	test_expect_success $3 "$2" '
 		(
 			cd '"'$1'"' &&
-			. "$(git --exec-path)"/git-sh-setup &&
+			PATH="$(git --exec-path):$PATH" &&
+			. git-sh-setup &&
 			cd_to_toplevel &&
 			[ "$(pwd -P)" = "$TOPLEVEL" ]
 		)

Re: [PATCH v3 13/39] i18n: git-sh-setup.sh: mark strings for translation

From: Vasco Almeida <hidden>
Date: 2016-06-16 02:19:42

Às 20:30 de 01-06-2016, Junio C Hamano escreveu:
quoted hunk
Junio C Hamano [off-list ref] writes:
Would it allow you to lose the $(git --exec-path) prefix in the new
dot-source to have this patch before applying your patch?

-- >8 --
Subject: t2300: run git-sh-setup in an environment that better mimics the real life

When we run scripted Porcelains, "git" potty has set up the $PATH by
prepending $GIT_EXEC_PATH, the path given by "git --exec-path=$there
$cmd", etc. already.  Because of this, scripted Porcelains can
dot-source shell script library like git-sh-setup with simple dot
without specifying any path.

t2300 however dot-sources git-sh-setup without adjusting $PATH like
the real "git" potty does.  This has not been a problem so far, but
once git-sh-setup wants to rely on the $PATH adjustment, just like
any scripted Porcelains already do, it would become one.  It cannot
for example dot-source another shell library without specifying the
full path to it by prefixing $(git --exec-path).

Signed-off-by: Junio C Hamano <redacted>
---
 t/t2300-cd-to-toplevel.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index 9965bc5..cccd7d9 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -8,7 +8,8 @@ test_cd_to_toplevel () {
 	test_expect_success $3 "$2" '
 		(
 			cd '"'$1'"' &&
-			. "$(git --exec-path)"/git-sh-setup &&
+			PATH="$(git --exec-path):$PATH" &&
+			. git-sh-setup &&
 			cd_to_toplevel &&
 			[ "$(pwd -P)" = "$TOPLEVEL" ]
 		)
Yes, this patch allows to have . git-sh-i18n instead of
. "$(git --exec-path)"/git-sh-i18n in git-sh-setup.sh file.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help