Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
SZEDER Gábor wrote: > > Makes sense, but the patch doesn't cover all cases, because > __git_ps1() can exit early Thanks for looking at the patch. I feel quite silly for missing the other return points :-( Follow-up patch on the way... Tony. -- f.anthony.n.finchhttp://dotat.at/ Faeroes, Southeast Iceland: Cyclonic for a time in Faeroes, otherwise northeasterly 6 to gale 8, increasing gale 8 to storm 10. Very rough or high. Rain, snow or wintry showers. Moderate or poor, occasionally very poor.
Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
Hi,
Quoting Tony Finch :
If you have a prompt which displays the command exit status,
__git_ps1 without this change corrupts it, although it has
the correct value in the parent shell:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(__git_ps1) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 0 $ echo $?
1
~/src/git (master) 0 $
There is a slightly ugly workaround:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(x=$?; __git_ps1; exit $x) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 1 $
This change makes the workaround unnecessary.
Signed-off-by: Tony Finch
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
I hope that explains it properly :-)
diff --git a/contrib/completion/git-prompt.sh
b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using
GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- "$printf_format" "$gitstring"
fi
+
+ # preserve exit status
+ return $exit
}
--
2.2.1.68.g56d9796
Makes sense, but the patch doesn't cover all cases, because
__git_ps1() can exit early, off the top of my head without actually
having a git clone at hand to look at the code, if:
* pwd is not in a git repo, which is a quite common case to worry about.
* .git/HEAD becomes unreadable while __git_ps1() is being executed.
It's an unlikely race condition so I wouldn't worry much about it, but
for consistency's sake I think it's better to return $? there as well.
Best,
Gábor
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
Junio C Hamano wrote: > > Yes. I wouldn't have spent 20 minutes experimenting with various > hypothetical use cases if the above were there in the first place. Sorry for wasting your time, and thanks for reviewing the patch. (I am so used to having $? in my prompt it took me ages to find and explain the problem too... Sigh!) Tony. -- f.anthony.n.finchhttp://dotat.at/ Trafalgar: Easterly 6 to gale 8 far southeast, otherwise northeasterly veering southeasterly 4 or 5. Slight or moderate, occasionally rough at first in south. Occasional drizzle. Good, occasionally moderate. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
Tony Finch writes:
> If you have a prompt which displays the command exit status,
> __git_ps1 without this change corrupts it, although it has
> the correct value in the parent shell:
>
> ~/src/git (master) 0 $ set | grep ^PS1
> PS1='\w$(__git_ps1) $? \$ '
> ~/src/git (master) 0 $ false
> ~/src/git (master) 0 $ echo $?
> 1
> ~/src/git (master) 0 $
>
> There is a slightly ugly workaround:
>
> ~/src/git (master) 0 $ set | grep ^PS1
> PS1='\w$(x=$?; __git_ps1; exit $x) $? \$ '
> ~/src/git (master) 0 $ false
> ~/src/git (master) 1 $
>
> This change makes the workaround unnecessary.
>
> Signed-off-by: Tony Finch
> ---
> contrib/completion/git-prompt.sh | 4
> 1 file changed, 4 insertions(+)
>
> I hope that explains it properly :-)
Yes. I wouldn't have spent 20 minutes experimenting with various
hypothetical use cases if the above were there in the first place.
Thanks. Will queue.
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index c5473dc..5fe69d0 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -288,6 +288,7 @@ __git_eread ()
> # In this mode you can request colored hints using
> GIT_PS1_SHOWCOLORHINTS=true
> __git_ps1 ()
> {
> + local exit=$?
> local pcmode=no
> local detached=no
> local ps1pc_start='\u@\h:\w '
> @@ -511,4 +512,7 @@ __git_ps1 ()
> else
> printf -- "$printf_format" "$gitstring"
> fi
> +
> + # preserve exit status
> + return $exit
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-prompt: preserve value of $? inside shell prompt
If you have a prompt which displays the command exit status,
__git_ps1 without this change corrupts it, although it has
the correct value in the parent shell:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(__git_ps1) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 0 $ echo $?
1
~/src/git (master) 0 $
There is a slightly ugly workaround:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(x=$?; __git_ps1; exit $x) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 1 $
This change makes the workaround unnecessary.
Signed-off-by: Tony Finch
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
I hope that explains it properly :-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- "$printf_format" "$gitstring"
fi
+
+ # preserve exit status
+ return $exit
}
--
2.2.1.68.g56d9796
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
