Re: [PATCH 3/3] Revert "bash prompt: avoid command substitution when finalizing gitstring"

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 3/3] Revert "bash prompt: avoid command substitution when finalizing gitstring"

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:29

Brandon Casey [off-list ref] writes:
On Wed, Aug 21, 2013 at 2:47 PM, Junio C Hamano [off-list ref] wrote:
quoted
Brandon Casey [off-list ref] writes:
quoted
From: Brandon Casey <redacted>

This reverts commit 69a8141a5d81925b7e08cb228535e9ea4a7a02e3.

Old Bash (3.0) which is distributed with RHEL 4.X and other ancient
platforms that are still in wide use, does not have a printf that
supports -v.  Let's revert this patch and go back to using printf
in the traditional way.

Signed-off-by: Brandon Casey <redacted>
---
Is this something you can detect at load-time once, store the result
in a private variable and then switch on it at runtime, something
along the lines of...


        # on load...
        printf -v __git_printf_supports_v -- "%s" yes >/dev/null 2>&1

        ...

        if test "${__git_printf_supports_v}" = yes
        then
                printf -v gitstring -- "$printf_format" "$gitstring"
        else
                gitstring=$(printf -- "$printf_format" "$gitstring")
        fi
Yes, that appears to work.
A real patch needs to be a bit more careful, though.  The variable
needs to be cleared before all of the above, and the testing would
want to consider that the variable may not be set (i.e. use
"${var-}" when checking).

Thanks.
-Brandon

quoted
quoted
 contrib/completion/git-prompt.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a81ef5a..7698ec4 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -433,11 +433,7 @@ __git_ps1 ()
      local gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"

      if [ $pcmode = yes ]; then
-             if [[ -n ${ZSH_VERSION-} ]]; then
-                     gitstring=$(printf -- "$printf_format" "$gitstring")
-             else
-                     printf -v gitstring -- "$printf_format" "$gitstring"
-             fi
+             gitstring=$(printf -- "$printf_format" "$gitstring")
              PS1="$ps1pc_start$gitstring$ps1pc_end"
      else
              printf -- "$printf_format" "$gitstring"

Re: [PATCH 3/3] Revert "bash prompt: avoid command substitution when finalizing gitstring"

From: Brandon Casey <hidden>
Date: 2016-06-15 22:58:29

On Wed, Aug 21, 2013 at 5:22 PM, Junio C Hamano [off-list ref] wrote:
Brandon Casey [off-list ref] writes:
quoted
On Wed, Aug 21, 2013 at 2:47 PM, Junio C Hamano [off-list ref] wrote:
quoted
quoted
        # on load...
        printf -v __git_printf_supports_v -- "%s" yes >/dev/null 2>&1

        ...

        if test "${__git_printf_supports_v}" = yes
        then
                printf -v gitstring -- "$printf_format" "$gitstring"
        else
                gitstring=$(printf -- "$printf_format" "$gitstring")
        fi
Yes, that appears to work.
A real patch needs to be a bit more careful, though.  The variable
needs to be cleared before all of the above,
Agreed.
and the testing would
want to consider that the variable may not be set (i.e. use
"${var-}" when checking).
Why is "${var-}" necessary?  Wouldn't that be equivalent to "${var}"
or "$var"?  We obviously wouldn't want to do 'if test $var = yes', but
I would have thought it was sufficient to wrap the variable
dereference in quotes as your original did.

-Brandon

[PATCH] contrib/git-prompt.sh: handle missing 'printf -v' more gracefully

From: Brandon Casey <hidden>
Date: 2016-06-15 22:58:29

From: Brandon Casey <redacted>

Old Bash (3.0) which is distributed with RHEL 4.X and other ancient
platforms that are still in wide use, do not have a printf that
supports -v.  Neither does Zsh (which is already handled in the code).

As suggested by Junio, let's test whether printf supports the -v
option and store the result.  Then later, we can use it to
determine whether 'printf -v' can be used, or whether printf
must be called in a subshell.

Signed-off-by: Brandon Casey <redacted>
---

This replaces [PATCH 3/3] Revert "bash prompt: avoid command substitution
when finalizing gitstring".

This may or may not need to be updated to use "${var-}" depending on
your response to my other email, but this seems sufficient.

-Brandon

 contrib/completion/git-prompt.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a81ef5a..639888a 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -84,6 +84,10 @@
 # the colored output of "git status -sb" and are available only when
 # using __git_ps1 for PROMPT_COMMAND or precmd.
 
+# check whether printf supports -v
+__git_printf_supports_v=
+printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -433,10 +437,10 @@ __git_ps1 ()
 	local gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
 
 	if [ $pcmode = yes ]; then
-		if [[ -n ${ZSH_VERSION-} ]]; then
-			gitstring=$(printf -- "$printf_format" "$gitstring")
-		else
+		if test "$__git_printf_supports_v" = yes; then
 			printf -v gitstring -- "$printf_format" "$gitstring"
+		else
+			gitstring=$(printf -- "$printf_format" "$gitstring")
 		fi
 		PS1="$ps1pc_start$gitstring$ps1pc_end"
 	else
-- 
1.8.4.rc0.2.g6cf5c31


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH] contrib/git-prompt.sh: handle missing 'printf -v' more gracefully

From: Brandon Casey <hidden>
Date: 2016-06-15 22:58:29

From: Brandon Casey <redacted>

Old Bash (3.0) which is distributed with RHEL 4.X and other ancient
platforms that are still in wide use, do not have a printf that
supports -v.  Neither does Zsh (which is already handled in the code).

As suggested by Junio, let's test whether printf supports the -v
option and store the result.  Then later, we can use it to
determine whether 'printf -v' can be used, or whether printf
must be called in a subshell.

Signed-off-by: Brandon Casey <redacted>
---


On 8/21/2013 6:27 PM, Junio C Hamano wrote:> Brandon Casey [off-list ref] writes:
quoted
Why is "${var-}" necessary?  Wouldn't that be equivalent to "${var}"
or "$var"?
set -u
Ah.  Thanks.  Updated.  Also minor tweak to use [ ] instead of test ...
to conform with the rest of the script.

-Brandon


 contrib/completion/git-prompt.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a81ef5a..ca7fb35 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -84,6 +84,10 @@
 # the colored output of "git status -sb" and are available only when
 # using __git_ps1 for PROMPT_COMMAND or precmd.
 
+# check whether printf supports -v
+__git_printf_supports_v=
+printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -433,10 +437,10 @@ __git_ps1 ()
 	local gitstring="$c${b##refs/heads/}${f:+$z$f}$r$p"
 
 	if [ $pcmode = yes ]; then
-		if [[ -n ${ZSH_VERSION-} ]]; then
-			gitstring=$(printf -- "$printf_format" "$gitstring")
-		else
+		if [ "${__git_printf_supports_v-}" = yes ]; then
 			printf -v gitstring -- "$printf_format" "$gitstring"
+		else
+			gitstring=$(printf -- "$printf_format" "$gitstring")
 		fi
 		PS1="$ps1pc_start$gitstring$ps1pc_end"
 	else
-- 
1.8.4.rc0.2.g6cf5c31


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help