{"thread":{"id":"38225","subject":"[PATCH] git-prompt: preserve command exit status","startedAt":"2014-12-22T14:30:03Z","lastAt":"2015-01-14T12:10:06Z","messageCount":9,"participants":["Tony Finch","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"253920","messageId":"alpine.LSU.2.00.1412221429490.28934@hermes-1.csi.cam.ac.uk","threadId":"38225","inReplyTo":null,"subject":"[PATCH] git-prompt: preserve command exit status","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2014-12-22T14:30:03Z","receivedAt":"2014-12-22T14:30:03Z","isPatch":true,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"Signed-off-by: Tony Finch <dot@dotat.at>\n---\n contrib/completion/git-prompt.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex c5473dc..5fe69d0 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -288,6 +288,7 @@ __git_eread ()\n # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true\n __git_ps1 ()\n {\n+\tlocal exit=$?\n \tlocal pcmode=no\n \tlocal detached=no\n \tlocal ps1pc_start='\\u@\\h:\\w '\n@@ -511,4 +512,7 @@ __git_ps1 ()\n \telse\n \t\tprintf -- \"$printf_format\" \"$gitstring\"\n \tfi\n+\n+\t# preserve exit status\n+\treturn $exit\n }\n-- \n2.1.0.rc1.12.g1e9b79d\n"},{"id":"253928","messageId":"xmqqa92fbo0j.fsf@gitster.dls.corp.google.com","threadId":"38225","inReplyTo":"alpine.LSU.2.00.1412221429490.28934@hermes-1.csi.cam.ac.uk","subject":"Re: [PATCH] git-prompt: preserve command exit status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-22T17:19:40Z","receivedAt":"2014-12-22T17:19:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tony Finch <dot@dotat.at> writes:\n\n> Signed-off-by: Tony Finch <dot@dotat.at>\n> ---\n>  contrib/completion/git-prompt.sh | 4 ++++\n>  1 file changed, 4 insertions(+)\n>\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index c5473dc..5fe69d0 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -288,6 +288,7 @@ __git_eread ()\n>  # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true\n>  __git_ps1 ()\n>  {\n> +\tlocal exit=$?\n>  \tlocal pcmode=no\n>  \tlocal detached=no\n>  \tlocal ps1pc_start='\\u@\\h:\\w '\n> @@ -511,4 +512,7 @@ __git_ps1 ()\n>  \telse\n>  \t\tprintf -- \"$printf_format\" \"$gitstring\"\n>  \tfi\n> +\n> +\t# preserve exit status\n> +\treturn $exit\n>  }\n\nHmmmm.  I thought \"The patch trivially makes sense!  Why didn't\nanybody notice this before?!?\", but then noticed that I never\nsuffered from an obvious consequence from the current lack of the\nexit-code-preserving:\n\n    : gitster git.git/master; echo \"<$PS1>\"\n    <: \\h \\W$(__git_ps1 \"/%s\"); >\n    : gitster git.git/master; false\n    : gitster git.git/master; echo $?\n    1\n\nAnd it does not seem that it is needed, at least for my use\npattern:\n\n    : gitster git.git/master; ps1func () { echo \"What Now: \"; exit 8; }\n    : gitster git.git/master; PS1='$(ps1func)'\n    What Now: echo $?\n    0\n    What Now: (exit 6)\n    What Now: echo $?\n    6\n\nAlso it does not seem that it is needed for the other style to use\nPROMPT_COMMAND:\n\n    : gitster git.git/master; sh\n    $ . contrib/completion/git-prompt.sh\n    $ PROMPT_COMMAND='__git_ps1 \"\\h \\W\" \"; \"'\n    gitster git.git (master); (exit 13)\n    gitster git.git (master); echo $?\n    13\n    gitster git.git (master); true\n    gitster git.git (master); echo $?\n    0\n\nSo, what are you fixing?  In other words, please describe how it\nfails in the log message.\n\nPuzzled...\n"},{"id":"253936","messageId":"alpine.LSU.2.00.1412221808110.2546@hermes-1.csi.cam.ac.uk","threadId":"38225","inReplyTo":"xmqqa92fbo0j.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2] git-prompt: preserve value of $? inside shell prompt","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2014-12-22T18:09:25Z","receivedAt":"2014-12-22T18:09:25Z","isPatch":true,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"If you have a prompt which displays the command exit status,\n__git_ps1 without this change corrupts it, although it has\nthe correct value in the parent shell:\n\n\t~/src/git (master) 0 $ set | grep ^PS1\n\tPS1='\\w$(__git_ps1) $? \\$ '\n\t~/src/git (master) 0 $ false\n\t~/src/git (master) 0 $ echo $?\n\t1\n\t~/src/git (master) 0 $\n\nThere is a slightly ugly workaround:\n\n\t~/src/git (master) 0 $ set | grep ^PS1\n\tPS1='\\w$(x=$?; __git_ps1; exit $x) $? \\$ '\n\t~/src/git (master) 0 $ false\n\t~/src/git (master) 1 $\n\nThis change makes the workaround unnecessary.\n\nSigned-off-by: Tony Finch <dot@dotat.at>\n---\n contrib/completion/git-prompt.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\nI hope that explains it properly :-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex c5473dc..5fe69d0 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -288,6 +288,7 @@ __git_eread ()\n # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true\n __git_ps1 ()\n {\n+\tlocal exit=$?\n \tlocal pcmode=no\n \tlocal detached=no\n \tlocal ps1pc_start='\\u@\\h:\\w '\n@@ -511,4 +512,7 @@ __git_ps1 ()\n \telse\n \t\tprintf -- \"$printf_format\" \"$gitstring\"\n \tfi\n+\n+\t# preserve exit status\n+\treturn $exit\n }\n-- \n2.2.1.68.g56d9796\n"},{"id":"253944","messageId":"xmqqsig78nim.fsf@gitster.dls.corp.google.com","threadId":"38225","inReplyTo":"alpine.LSU.2.00.1412221808110.2546@hermes-1.csi.cam.ac.uk","subject":"Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-22T19:58:41Z","receivedAt":"2014-12-22T19:58:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tony Finch <dot@dotat.at> writes:\n\n> If you have a prompt which displays the command exit status,\n> __git_ps1 without this change corrupts it, although it has\n> the correct value in the parent shell:\n>\n> \t~/src/git (master) 0 $ set | grep ^PS1\n> \tPS1='\\w$(__git_ps1) $? \\$ '\n> \t~/src/git (master) 0 $ false\n> \t~/src/git (master) 0 $ echo $?\n> \t1\n> \t~/src/git (master) 0 $\n>\n> There is a slightly ugly workaround:\n>\n> \t~/src/git (master) 0 $ set | grep ^PS1\n> \tPS1='\\w$(x=$?; __git_ps1; exit $x) $? \\$ '\n> \t~/src/git (master) 0 $ false\n> \t~/src/git (master) 1 $\n>\n> This change makes the workaround unnecessary.\n>\n> Signed-off-by: Tony Finch <dot@dotat.at>\n> ---\n>  contrib/completion/git-prompt.sh | 4 ++++\n>  1 file changed, 4 insertions(+)\n>\n> I hope that explains it properly :-)\n\nYes.  I wouldn't have spent 20 minutes experimenting with various\nhypothetical use cases if the above were there in the first place.\n\nThanks.  Will queue.\n\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index c5473dc..5fe69d0 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -288,6 +288,7 @@ __git_eread ()\n>  # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true\n>  __git_ps1 ()\n>  {\n> +\tlocal exit=$?\n>  \tlocal pcmode=no\n>  \tlocal detached=no\n>  \tlocal ps1pc_start='\\u@\\h:\\w '\n> @@ -511,4 +512,7 @@ __git_ps1 ()\n>  \telse\n>  \t\tprintf -- \"$printf_format\" \"$gitstring\"\n>  \tfi\n> +\n> +\t# preserve exit status\n> +\treturn $exit\n>  }\n"},{"id":"253957","messageId":"alpine.LSU.2.00.1412222216120.26823@hermes-1.csi.cam.ac.uk","threadId":"38225","inReplyTo":"xmqqsig78nim.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2014-12-22T22:18:46Z","receivedAt":"2014-12-22T22:18:46Z","isPatch":true,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Yes.  I wouldn't have spent 20 minutes experimenting with various\n> hypothetical use cases if the above were there in the first place.\n\nSorry for wasting your time, and thanks for reviewing the patch.\n\n(I am so used to having $? in my prompt it took me ages to find and\nexplain the problem too... Sigh!)\n\nTony.\n-- \nf.anthony.n.finch  <dot@dotat.at>  http://dotat.at/\nTrafalgar: Easterly 6 to gale 8 far southeast, otherwise northeasterly veering\nsoutheasterly 4 or 5. Slight or moderate, occasionally rough at first in\nsouth. Occasional drizzle. Good, occasionally moderate.\n"},{"id":"254632","messageId":"20150114005726.Horde.idyLC0Or9SvaghEN_N_pRg1@webmail.informatik.kit.edu","threadId":"38225","inReplyTo":"alpine.LSU.2.00.1412221808110.2546@hermes-1.csi.cam.ac.uk","subject":"Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-01-13T23:57:26Z","receivedAt":"2015-01-13T23:57:26Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nQuoting Tony Finch <dot@dotat.at>:\n> If you have a prompt which displays the command exit status,\n> __git_ps1 without this change corrupts it, although it has\n> the correct value in the parent shell:\n>\n> \t~/src/git (master) 0 $ set | grep ^PS1\n> \tPS1='\\w$(__git_ps1) $? \\$ '\n> \t~/src/git (master) 0 $ false\n> \t~/src/git (master) 0 $ echo $?\n> \t1\n> \t~/src/git (master) 0 $\n>\n> There is a slightly ugly workaround:\n>\n> \t~/src/git (master) 0 $ set | grep ^PS1\n> \tPS1='\\w$(x=$?; __git_ps1; exit $x) $? \\$ '\n> \t~/src/git (master) 0 $ false\n> \t~/src/git (master) 1 $\n>\n> This change makes the workaround unnecessary.\n>\n> Signed-off-by: Tony Finch <dot@dotat.at>\n> ---\n>    contrib/completion/git-prompt.sh | 4 ++++\n>    1 file changed, 4 insertions(+)\n>\n> I hope that explains it properly :-)\n>\n> diff --git a/contrib/completion/git-prompt.sh\n> b/contrib/completion/git-prompt.sh\n> index c5473dc..5fe69d0 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -288,6 +288,7 @@ __git_eread ()\n>    # In this mode you can request colored hints using\n> GIT_PS1_SHOWCOLORHINTS=true\n>    __git_ps1 ()\n>    {\n> +\tlocal exit=$?\n>    \tlocal pcmode=no\n>    \tlocal detached=no\n>    \tlocal ps1pc_start='\\u@\\h:\\w '\n> @@ -511,4 +512,7 @@ __git_ps1 ()\n>    \telse\n>    \t\tprintf -- \"$printf_format\" \"$gitstring\"\n>    \tfi\n> +\n> +\t# preserve exit status\n> +\treturn $exit\n>    }\n> --\n> 2.2.1.68.g56d9796\n\nMakes sense, but the patch doesn't cover all cases, because  \n__git_ps1() can exit early, off the top of my head without actually  \nhaving a git clone at hand to look at the code, if:\n\n  * pwd is not in a git repo, which is a quite common case to worry about.\n  * .git/HEAD becomes unreadable while __git_ps1() is being executed.   \nIt's an unlikely race condition so I wouldn't worry much about it, but  \nfor consistency's sake I think it's better to return $? there as well.\n\nBest,\nGábor\n"},{"id":"254637","messageId":"alpine.LSU.2.00.1501141003400.23307@hermes-1.csi.cam.ac.uk","threadId":"38225","inReplyTo":"20150114005726.Horde.idyLC0Or9SvaghEN_N_pRg1@webmail.informatik.kit.edu","subject":"Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2015-01-14T10:05:56Z","receivedAt":"2015-01-14T10:05:56Z","isPatch":true,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> wrote:\n>\n> Makes sense, but the patch doesn't cover all cases, because\n> __git_ps1() can exit early\n\nThanks for looking at the patch. I feel quite silly for missing the other\nreturn points :-( Follow-up patch on the way...\n\nTony.\n-- \nf.anthony.n.finch  <dot@dotat.at>  http://dotat.at/\nFaeroes, Southeast Iceland: Cyclonic for a time in Faeroes, otherwise\nnortheasterly 6 to gale 8, increasing gale 8 to storm 10. Very rough or high.\nRain, snow or wintry showers. Moderate or poor, occasionally very poor."},{"id":"254638","messageId":"alpine.LSU.2.00.1501141005560.23307@hermes-1.csi.cam.ac.uk","threadId":"38225","inReplyTo":"20150114005726.Horde.idyLC0Or9SvaghEN_N_pRg1@webmail.informatik.kit.edu","subject":"[PATCH] git-prompt: preserve value of $? in all cases","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2015-01-14T10:06:28Z","receivedAt":"2015-01-14T10:06:28Z","isPatch":true,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"Signed-off-by: Tony Finch <dot@dotat.at>\n---\n contrib/completion/git-prompt.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 3c3fc6d..3e70e74 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -288,6 +288,7 @@ __git_eread ()\n # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true\n __git_ps1 ()\n {\n+\t# preserve exit status\n \tlocal exit=$?\n \tlocal pcmode=no\n \tlocal detached=no\n@@ -303,7 +304,7 @@ __git_ps1 ()\n \t\t;;\n \t\t0|1)\tprintf_format=\"${1:-$printf_format}\"\n \t\t;;\n-\t\t*)\treturn\n+\t\t*)\treturn $exit\n \t\t;;\n \tesac\n\n@@ -355,7 +356,7 @@ __git_ps1 ()\n \t\t\t#In PC mode PS1 always needs to be set\n \t\t\tPS1=\"$ps1pc_start$ps1pc_end\"\n \t\tfi\n-\t\treturn\n+\t\treturn $exit\n \tfi\n\n \tlocal short_sha\n@@ -416,7 +417,7 @@ __git_ps1 ()\n \t\t\t\tif [ $pcmode = yes ]; then\n \t\t\t\t\tPS1=\"$ps1pc_start$ps1pc_end\"\n \t\t\t\tfi\n-\t\t\t\treturn\n+\t\t\t\treturn $exit\n \t\t\tfi\n \t\t\t# is it a symbolic ref?\n \t\t\tb=\"${head#ref: }\"\n@@ -513,6 +514,5 @@ __git_ps1 ()\n \t\tprintf -- \"$printf_format\" \"$gitstring\"\n \tfi\n\n-\t# preserve exit status\n \treturn $exit\n }\n-- \n2.2.1.68.g56d9796\n"},{"id":"254643","messageId":"20150114131006.Horde._hnEBDLPm_RUjO-IJlS9dw1@webmail.informatik.kit.edu","threadId":"38225","inReplyTo":"alpine.LSU.2.00.1501141005560.23307@hermes-1.csi.cam.ac.uk","subject":"Re: [PATCH] git-prompt: preserve value of $? in all cases","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-01-14T12:10:06Z","receivedAt":"2015-01-14T12:10:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nQuoting Tony Finch <dot@dotat.at>:\n> Signed-off-by: Tony Finch <dot@dotat.at>\n> ---\n>  contrib/completion/git-prompt.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/completion/git-prompt.sh  \n> b/contrib/completion/git-prompt.sh\n> index 3c3fc6d..3e70e74 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -288,6 +288,7 @@ __git_eread ()\n>  # In this mode you can request colored hints using  \n> GIT_PS1_SHOWCOLORHINTS=true\n>  __git_ps1 ()\n>  {\n> +\t# preserve exit status\n>  \tlocal exit=$?\n>  \tlocal pcmode=no\n>  \tlocal detached=no\n> @@ -303,7 +304,7 @@ __git_ps1 ()\n>  \t\t;;\n>  \t\t0|1)\tprintf_format=\"${1:-$printf_format}\"\n>  \t\t;;\n> -\t\t*)\treturn\n> +\t\t*)\treturn $exit\n>  \t\t;;\n>  \tesac\n>\n> @@ -355,7 +356,7 @@ __git_ps1 ()\n>  \t\t\t#In PC mode PS1 always needs to be set\n>  \t\t\tPS1=\"$ps1pc_start$ps1pc_end\"\n>  \t\tfi\n> -\t\treturn\n> +\t\treturn $exit\n>  \tfi\n>\n>  \tlocal short_sha\n> @@ -416,7 +417,7 @@ __git_ps1 ()\n>  \t\t\t\tif [ $pcmode = yes ]; then\n>  \t\t\t\t\tPS1=\"$ps1pc_start$ps1pc_end\"\n>  \t\t\t\tfi\n> -\t\t\t\treturn\n> +\t\t\t\treturn $exit\n>  \t\t\tfi\n>  \t\t\t# is it a symbolic ref?\n>  \t\t\tb=\"${head#ref: }\"\n> @@ -513,6 +514,5 @@ __git_ps1 ()\n>  \t\tprintf -- \"$printf_format\" \"$gitstring\"\n>  \tfi\n>\n> -\t# preserve exit status\n>  \treturn $exit\n>  }\n> --\n> 2.2.1.68.g56d9796\n\nThanks for the quick turnaround, looks good to me.  I didn't remember  \nthe early return in the second hunk.\n\nI wonder whether we could test this behavior...  but how could we set  \n$? and pass it to __git_ps1()?\n\nJunio,\nas far as I can judge from the last What's cooking and the relevant  \npatch emails on Gmane, this patch will have a textual conflict with  \nthe first patch in 'rh/hide-prompt-in-ignored-directory'.  While the  \nconflict is trivial (maybe git would even be able to resolve it by  \nitself?), the second patch in that series adds yet another early  \nreturn to __git_ps1().  Please be sure to add 'return $exit' when  \nmerging.\n\n\nBest,\nGábor\n"}]}