All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
@ 2022-02-25 11:44 Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-25 11:44 UTC (permalink / raw)
  To: git; +Cc: Justin Donnelly

These patches are about the characters and words that can be configured to
display in the PS1 prompt after the branch name. I've been unable to find a
consistent terminology. I refer to them as follows: [short | long] [type]
state indicator where short is for characters (e.g. ?), long is for words
(e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
I'd be happy to change the commit messages to a different terminology if
that's preferred.

There are a few inconsistencies with the PS1 prompt upstream state indicator
(GIT_PS1_SHOWUPSTREAM).

 * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
   indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
   upstream state indicator appears adjacent to the branch name (e.g.
   (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
   (main =)).
 * If there are long state indicators (e.g. |SPARSE), a short upstream state
   indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
   state indicator (e.g. (main +|SPARSE=)) instead of with the other short
   state indicators (e.g. (main +=|SPARSE)).
 * The long upstream state indicator (e.g. GIT_PS1_SHOWUPSTREAM="verbose")
   is separated from other (short or long) state indicators by a hard-coded
   SP. Other long state indicators are separated by a hard-coded pipe (|).

These patches are to make the upstream state indicators more consistent with
other state indicators.

Justin Donnelly (4):
  git-prompt: rename `upstream` to `upstream_type`
  git-prompt: make upstream state indicator location consistent
  git-prompt: make long upstream state indicator consistent
  git-prompt: put upstream comments together

 contrib/completion/git-prompt.sh | 59 ++++++++++++++++----------------
 1 file changed, 30 insertions(+), 29 deletions(-)


base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1162%2Fjustinrdonnelly%2Fgit-prompt-upstream-consistency-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1162/justinrdonnelly/git-prompt-upstream-consistency-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1162
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] git-prompt: rename `upstream` to `upstream_type`
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
@ 2022-02-25 11:44 ` Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-25 11:44 UTC (permalink / raw)
  To: git; +Cc: Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

In `__git_ps1_show_upstream` rename the variable `upstream` to
`upstream_type`. This allows `__git_ps1_show_upstream` to reference a
variable named `upstream` that is declared `local` in `__git_ps1`, which
calls `__git_ps1_show_upstream`.

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index db7c0068fb5..3997e099aa7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -115,7 +115,7 @@ __git_ps1_show_upstream ()
 {
 	local key value
 	local svn_remote svn_url_pattern count n
-	local upstream=git legacy="" verbose="" name=""
+	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
 	# get some config options from git-config
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 		svn-remote.*.url)
 			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
 			svn_url_pattern="$svn_url_pattern\\|$value"
-			upstream=svn+git # default upstream is SVN if available, else git
+			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
 	done <<< "$output"
@@ -141,16 +141,16 @@ __git_ps1_show_upstream ()
 	local option
 	for option in ${GIT_PS1_SHOWUPSTREAM}; do
 		case "$option" in
-		git|svn) upstream="$option" ;;
+		git|svn) upstream_type="$option" ;;
 		verbose) verbose=1 ;;
 		legacy)  legacy=1  ;;
 		name)    name=1 ;;
 		esac
 	done
 
-	# Find our upstream
-	case "$upstream" in
-	git)    upstream="@{upstream}" ;;
+	# Find our upstream type
+	case "$upstream_type" in
+	git)    upstream_type="@{upstream}" ;;
 	svn*)
 		# get the upstream from the "git-svn-id: ..." in a commit message
 		# (git-svn uses essentially the same procedure internally)
@@ -167,12 +167,12 @@ __git_ps1_show_upstream ()
 
 			if [[ -z "$svn_upstream" ]]; then
 				# default branch name for checkouts with no layout:
-				upstream=${GIT_SVN_ID:-git-svn}
+				upstream_type=${GIT_SVN_ID:-git-svn}
 			else
-				upstream=${svn_upstream#/}
+				upstream_type=${svn_upstream#/}
 			fi
-		elif [[ "svn+git" = "$upstream" ]]; then
-			upstream="@{upstream}"
+		elif [[ "svn+git" = "$upstream_type" ]]; then
+			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
@@ -180,11 +180,11 @@ __git_ps1_show_upstream ()
 	# Find how many commits we are ahead/behind our upstream
 	if [[ -z "$legacy" ]]; then
 		count="$(git rev-list --count --left-right \
-				"$upstream"...HEAD 2>/dev/null)"
+				"$upstream_type"...HEAD 2>/dev/null)"
 	else
 		# produce equivalent output to --count for older versions of git
 		local commits
-		if commits="$(git rev-list --left-right "$upstream"...HEAD 2>/dev/null)"
+		if commits="$(git rev-list --left-right "$upstream_type"...HEAD 2>/dev/null)"
 		then
 			local commit behind=0 ahead=0
 			for commit in $commits
@@ -229,7 +229,7 @@ __git_ps1_show_upstream ()
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
-				--abbrev-ref "$upstream" 2>/dev/null)
+				--abbrev-ref "$upstream_type" 2>/dev/null)
 			if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
 				p="$p \${__git_ps1_upstream_name}"
 			else
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/4] git-prompt: make upstream state indicator location consistent
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
@ 2022-02-25 11:44 ` Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-25 11:44 UTC (permalink / raw)
  To: git; +Cc: Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Make upstream state indicator location more consistent with similar
state indicators (e.g. sparse). Group the short state indicator (`=`,
`<`, `>`, or `<>`) with other short state indicators immediately after
the branch name. Group the long state indicator (e.g. `u+2-1
origin/main`) with other long state indicators after the short state
indicators. Previously short and long upstream state indicators appeared
after all other state indicators.

Use a separator (`SP` or `GIT_PS1_STATESEPARATOR`) between branch name
and short upstream state indicator. Previously the short upstream state
indicator would sometimes appear directly adjacent to the branch name
(e.g. `(main=)`) instead of being separated (e.g. `(main =)`).

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 3997e099aa7..613389a53bc 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -109,7 +109,7 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
-# stores the divergence from upstream in $p
+# stores the divergence from upstream in $p (for short status) or $upstream (for verbose status)
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
 {
@@ -214,26 +214,26 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			p="<>" ;;
 		esac
-	else
+	else # verbose, set upstream instead of p
 		case "$count" in
 		"") # no upstream
-			p="" ;;
+			upstream="" ;;
 		"0	0") # equal to upstream
-			p=" u=" ;;
+			upstream=" u=" ;;
 		"0	"*) # ahead of upstream
-			p=" u+${count#0	}" ;;
+			upstream=" u+${count#0	}" ;;
 		*"	0") # behind upstream
-			p=" u-${count%	0}" ;;
+			upstream=" u-${count%	0}" ;;
 		*)	    # diverged from upstream
-			p=" u+${count#*	}-${count%	*}" ;;
+			upstream=" u+${count#*	}-${count%	*}" ;;
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
 				--abbrev-ref "$upstream_type" 2>/dev/null)
 			if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
-				p="$p \${__git_ps1_upstream_name}"
+				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
-				p="$p ${__git_ps1_upstream_name}"
+				upstream="$upstream ${__git_ps1_upstream_name}"
 				# not needed anymore; keep user's
 				# environment clean
 				unset __git_ps1_upstream_name
@@ -512,7 +512,8 @@ __git_ps1 ()
 	local u=""
 	local h=""
 	local c=""
-	local p=""
+	local p="" # short version of upstream state indicator
+	local upstream="" # verbose version of upstream state indicator
 
 	if [ "true" = "$inside_gitdir" ]; then
 		if [ "true" = "$bare_repo" ]; then
@@ -568,8 +569,8 @@ __git_ps1 ()
 		b="\${__git_ps1_branch_name}"
 	fi
 
-	local f="$h$w$i$s$u"
-	local gitstring="$c$b${f:+$z$f}${sparse}$r$p"
+	local f="$h$w$i$s$u$p"
+	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}"
 
 	if [ $pcmode = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/4] git-prompt: make long upstream state indicator consistent
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
@ 2022-02-25 11:44 ` Justin Donnelly via GitGitGadget
  2022-02-25 11:44 ` [PATCH 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-25 11:44 UTC (permalink / raw)
  To: git; +Cc: Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Use a pipe as a delimiter between short state indicators and long
upstream state indicator (e.g. `(main *|u+2-1 origin/main)` instead of
`(main * u+2-1 origin/main)`) . This is consistent with long state
indicators for sparse and in-progress operations (e.g. merge).

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 613389a53bc..2772f990888 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -109,7 +109,7 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
-# stores the divergence from upstream in $p (for short status) or $upstream (for verbose status)
+# stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
 {
@@ -219,13 +219,13 @@ __git_ps1_show_upstream ()
 		"") # no upstream
 			upstream="" ;;
 		"0	0") # equal to upstream
-			upstream=" u=" ;;
+			upstream="|u=" ;;
 		"0	"*) # ahead of upstream
-			upstream=" u+${count#0	}" ;;
+			upstream="|u+${count#0	}" ;;
 		*"	0") # behind upstream
-			upstream=" u-${count%	0}" ;;
+			upstream="|u-${count%	0}" ;;
 		*)	    # diverged from upstream
-			upstream=" u+${count#*	}-${count%	*}" ;;
+			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/4] git-prompt: put upstream comments together
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
                   ` (2 preceding siblings ...)
  2022-02-25 11:44 ` [PATCH 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
@ 2022-02-25 11:44 ` Justin Donnelly via GitGitGadget
  2022-02-25 12:22 ` [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
  5 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-25 11:44 UTC (permalink / raw)
  To: git; +Cc: Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Commit 6d158cba28 (bash completion: Support "divergence from upstream"
messages in __git_ps1, 2010-06-17) introduced support for indicating
divergence from upstream in the PS1 prompt. The comments at the top of
git-prompt.sh that were introduced with that commit are several
paragraphs long. Over the years, other comments have been inserted in
between the paragraphs relating to divergence from upstream.

This commit puts the comments relating to divergence from upstream back
together.

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 2772f990888..87b2b916c03 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -66,6 +66,11 @@
 #     git           always compare HEAD to @{upstream}
 #     svn           always compare HEAD to your SVN upstream
 #
+# By default, __git_ps1 will compare HEAD to your SVN upstream if it can
+# find one, or @{upstream} otherwise.  Once you have set
+# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
+# setting the bash.showUpstream config variable.
+#
 # You can change the separator between the branch name and the above
 # state symbols by setting GIT_PS1_STATESEPARATOR. The default separator
 # is SP.
@@ -79,11 +84,6 @@
 # single '?' character by setting GIT_PS1_COMPRESSSPARSESTATE, or omitted
 # by setting GIT_PS1_OMITSPARSESTATE.
 #
-# By default, __git_ps1 will compare HEAD to your SVN upstream if it can
-# find one, or @{upstream} otherwise.  Once you have set
-# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
-# setting the bash.showUpstream config variable.
-#
 # If you would like to see more information about the identity of
 # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE
 # to one of these values:
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
                   ` (3 preceding siblings ...)
  2022-02-25 11:44 ` [PATCH 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
@ 2022-02-25 12:22 ` Ævar Arnfjörð Bjarmason
  2022-02-27  0:32   ` Justin Donnelly
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
  5 siblings, 1 reply; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-02-25 12:22 UTC (permalink / raw)
  To: Justin Donnelly via GitGitGadget; +Cc: git, Justin Donnelly


On Fri, Feb 25 2022, Justin Donnelly via GitGitGadget wrote:

I couldn't find any glaring issues here on a quick review, just a note.

> These patches are about the characters and words that can be configured to
> display in the PS1 prompt after the branch name. I've been unable to find a
> consistent terminology. I refer to them as follows: [short | long] [type]
> state indicator where short is for characters (e.g. ?), long is for words
> (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
> I'd be happy to change the commit messages to a different terminology if
> that's preferred.

I think that terminology is correct, in case you haven't seen it
git-for-each-ref(1) talks about the "short" here as "short",
"trackshort" etc.

> There are a few inconsistencies with the PS1 prompt upstream state indicator
> (GIT_PS1_SHOWUPSTREAM).
>
>  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
>    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
>    upstream state indicator appears adjacent to the branch name (e.g.
>    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
>    (main =)).
>  * If there are long state indicators (e.g. |SPARSE), a short upstream state
>    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
>    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
>    state indicators (e.g. (main +=|SPARSE)).

I think it would really help to in each commit message have a
before/after comparison of the relevant PS1 output that's being changed.

I'm not sure how to readthis example. So before we said "main +=|SPARSE"
but now we'll say "main +|SPARSE=", but without sparse we'll say
"main="?

Aren't both of those harder to read than they need to be, shouldn't it
be closer to:

    main= |SPARSE

Or:

    main= |+SPARSE

Or:

    main= +|SPARSE

I can't recall what the "+" there is (if any).

I.e. the "=" refers to the ahead/behind state of "main, it seems odd in
both versions of your example that we're splitting it off from "main"
because we have "SPARSE" too.

But maybe I'm missing something...

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-02-25 12:22 ` [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
@ 2022-02-27  0:32   ` Justin Donnelly
  2022-02-27 10:19     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 15+ messages in thread
From: Justin Donnelly @ 2022-02-27  0:32 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Justin Donnelly via GitGitGadget, git

Thanks for the feedback. Comments interleaved below.

On Fri, Feb 25, 2022 at 7:26 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Fri, Feb 25 2022, Justin Donnelly via GitGitGadget wrote:
>
> I couldn't find any glaring issues here on a quick review, just a note.
>
> > These patches are about the characters and words that can be configured to
> > display in the PS1 prompt after the branch name. I've been unable to find a
> > consistent terminology. I refer to them as follows: [short | long] [type]
> > state indicator where short is for characters (e.g. ?), long is for words
> > (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
> > I'd be happy to change the commit messages to a different terminology if
> > that's preferred.
>
> I think that terminology is correct, in case you haven't seen it
> git-for-each-ref(1) talks about the "short" here as "short",
> "trackshort" etc.
>
> > There are a few inconsistencies with the PS1 prompt upstream state indicator
> > (GIT_PS1_SHOWUPSTREAM).
> >
> >  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
> >    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
> >    upstream state indicator appears adjacent to the branch name (e.g.
> >    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
> >    (main =)).
> >  * If there are long state indicators (e.g. |SPARSE), a short upstream state
> >    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
> >    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
> >    state indicators (e.g. (main +=|SPARSE)).
>
> I think it would really help to in each commit message have a
> before/after comparison of the relevant PS1 output that's being changed.

I agree that a before/after comparison would probably make it easier
to understand. Maybe some examples without upstream (for a baseline to
compare against) and a table that shows before/after for upstream.

`__git_ps1` examples without upstream:
(main)
(main %)
(main *%)
(main|SPARSE)
(main %|SPARSE)
(main *%|SPARSE)
(main|SPARSE|REBASE 1/2)
(main %|SPARSE|REBASE 1/2)

Of note:
1. If there are short state indicators, they appear together after the
branch name and separated from it by `SP` or `GIT_PS1_STATESEPARATOR`.
2. If there are long state indicators, they appear after short state
indicators if there are any, or after the branch name if there are no
short state indicators. Each long state indicator begins with a pipe
(`|`) as a separator.

Patch 2 before/after:
| Before           | After            |
| ---------------- | ---------------- |
| (main=)          | (main =)         |
| (main|SPARSE=)   | (main =|SPARSE)  |
| (main %|SPARSE=) | (main %=|SPARSE) |

Patch 3 before/after:
| Before                          | After                           |
| ------------------------------- | ------------------------------- |
| (main u=)                       | (main|u=)                       |
| (main u= origin/main)           | (main|u= origin/main)           |
| (main u+1)                      | (main|u+1)                      |
| (main u+1 origin/main)          | (main|u+1 origin/main)          |
| (main % u=)                     | (main %|u=)                     |
| (main % u= origin/main)         | (main %|u= origin/main)         |
| (main % u+1)                    | (main %|u+1)                    |
| (main % u+1 origin/main)        | (main %|u+1 origin/main)        |
| (main|SPARSE u=)                | (main|SPARSE|u=)                |
| (main|SPARSE u= origin/main)    | (main|SPARSE|u= origin/main)    |
| (main|SPARSE u+1)               | (main|SPARSE|u+1)               |
| (main|SPARSE u+1 origin/main)   | (main|SPARSE|u+1 origin/main)   |
| (main %|SPARSE u=)              | (main %|SPARSE|u=)              |
| (main %|SPARSE u= origin/main)  | (main %|SPARSE|u= origin/main)  |
| (main %|SPARSE u+1)             | (main %|SPARSE|u+1)             |
| (main %|SPARSE u+1 origin/main) | (main %|SPARSE|u+1 origin/main) |

Note: These tables are inspired by [Markdown Guide extended
syntax](https://www.markdownguide.org/extended-syntax/#tables), but I
didn't wrap the PS1 prompt text in backticks or escape the pipe
because I thought that would make it more confusing. In short, they're
meant to be viewed as (monospaced font) text, not Markdown.

>
>
> I'm not sure how to readthis example. So before we said "main +=|SPARSE"
> but now we'll say "main +|SPARSE=", but without sparse we'll say
> "main="?
>
> Aren't both of those harder to read than they need to be, shouldn't it
> be closer to:
>
>     main= |SPARSE
>
> Or:
>
>     main= |+SPARSE
>
> Or:
>
>     main= +|SPARSE
>
> I can't recall what the "+" there is (if any).


`+` is for staged changes (if `GIT_PS1_SHOWDIRTYSTATE` is a nonempty
value). So it's not directly related to upstream, but the addition of
another short state indicator changes things.

>
>
> I.e. the "=" refers to the ahead/behind state of "main, it seems odd in
> both versions of your example that we're splitting it off from "main"
> because we have "SPARSE" too.
>
> But maybe I'm missing something...

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-02-27  0:32   ` Justin Donnelly
@ 2022-02-27 10:19     ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-02-27 10:19 UTC (permalink / raw)
  To: Justin Donnelly; +Cc: Justin Donnelly via GitGitGadget, git


On Sat, Feb 26 2022, Justin Donnelly wrote:

> Thanks for the feedback. Comments interleaved below.
>
> On Fri, Feb 25, 2022 at 7:26 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Fri, Feb 25 2022, Justin Donnelly via GitGitGadget wrote:
>>
>> I couldn't find any glaring issues here on a quick review, just a note.
>>
>> > These patches are about the characters and words that can be configured to
>> > display in the PS1 prompt after the branch name. I've been unable to find a
>> > consistent terminology. I refer to them as follows: [short | long] [type]
>> > state indicator where short is for characters (e.g. ?), long is for words
>> > (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
>> > I'd be happy to change the commit messages to a different terminology if
>> > that's preferred.
>>
>> I think that terminology is correct, in case you haven't seen it
>> git-for-each-ref(1) talks about the "short" here as "short",
>> "trackshort" etc.
>>
>> > There are a few inconsistencies with the PS1 prompt upstream state indicator
>> > (GIT_PS1_SHOWUPSTREAM).
>> >
>> >  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
>> >    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
>> >    upstream state indicator appears adjacent to the branch name (e.g.
>> >    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
>> >    (main =)).
>> >  * If there are long state indicators (e.g. |SPARSE), a short upstream state
>> >    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
>> >    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
>> >    state indicators (e.g. (main +=|SPARSE)).
>>
>> I think it would really help to in each commit message have a
>> before/after comparison of the relevant PS1 output that's being changed.
>
> I agree that a before/after comparison would probably make it easier
> to understand. Maybe some examples without upstream (for a baseline to
> compare against) and a table that shows before/after for upstream.
>
> `__git_ps1` examples without upstream:
> (main)
> (main %)
> (main *%)
> (main|SPARSE)
> (main %|SPARSE)
> (main *%|SPARSE)
> (main|SPARSE|REBASE 1/2)
> (main %|SPARSE|REBASE 1/2)
>
> Of note:
> 1. If there are short state indicators, they appear together after the
> branch name and separated from it by `SP` or `GIT_PS1_STATESEPARATOR`.
> 2. If there are long state indicators, they appear after short state
> indicators if there are any, or after the branch name if there are no
> short state indicators. Each long state indicator begins with a pipe
> (`|`) as a separator.
>
> Patch 2 before/after:
> | Before           | After            |
> | ---------------- | ---------------- |
> | (main=)          | (main =)         |
> | (main|SPARSE=)   | (main =|SPARSE)  |
> | (main %|SPARSE=) | (main %=|SPARSE) |
>
> Patch 3 before/after:
> | Before                          | After                           |
> | ------------------------------- | ------------------------------- |
> | (main u=)                       | (main|u=)                       |
> | (main u= origin/main)           | (main|u= origin/main)           |
> | (main u+1)                      | (main|u+1)                      |
> | (main u+1 origin/main)          | (main|u+1 origin/main)          |
> | (main % u=)                     | (main %|u=)                     |
> | (main % u= origin/main)         | (main %|u= origin/main)         |
> | (main % u+1)                    | (main %|u+1)                    |
> | (main % u+1 origin/main)        | (main %|u+1 origin/main)        |
> | (main|SPARSE u=)                | (main|SPARSE|u=)                |
> | (main|SPARSE u= origin/main)    | (main|SPARSE|u= origin/main)    |
> | (main|SPARSE u+1)               | (main|SPARSE|u+1)               |
> | (main|SPARSE u+1 origin/main)   | (main|SPARSE|u+1 origin/main)   |
> | (main %|SPARSE u=)              | (main %|SPARSE|u=)              |
> | (main %|SPARSE u= origin/main)  | (main %|SPARSE|u= origin/main)  |
> | (main %|SPARSE u+1)             | (main %|SPARSE|u+1)             |
> | (main %|SPARSE u+1 origin/main) | (main %|SPARSE|u+1 origin/main) |
>
> Note: These tables are inspired by [Markdown Guide extended
> syntax](https://www.markdownguide.org/extended-syntax/#tables), but I
> didn't wrap the PS1 prompt text in backticks or escape the pipe
> because I thought that would make it more confusing. In short, they're
> meant to be viewed as (monospaced font) text, not Markdown.

Thanks. These comparisons are really useful & would be nice to work into
relevant commit messages in a re-roll.

I withdraw any suggestions about making this "main|SPARSE|u=" instead of
"main=|SPARSE|u" or whatever. I think such a thing might still make
sense, but it's clearly unrelated to the improvements you're making
here.

>>
>>
>> I'm not sure how to readthis example. So before we said "main +=|SPARSE"
>> but now we'll say "main +|SPARSE=", but without sparse we'll say
>> "main="?
>>
>> Aren't both of those harder to read than they need to be, shouldn't it
>> be closer to:
>>
>>     main= |SPARSE
>>
>> Or:
>>
>>     main= |+SPARSE
>>
>> Or:
>>
>>     main= +|SPARSE
>>
>> I can't recall what the "+" there is (if any).
>
>
> `+` is for staged changes (if `GIT_PS1_SHOWDIRTYSTATE` is a nonempty
> value). So it's not directly related to upstream, but the addition of
> another short state indicator changes things.

Thanks!

>>
>>
>> I.e. the "=" refers to the ahead/behind state of "main, it seems odd in
>> both versions of your example that we're splitting it off from "main"
>> because we have "SPARSE" too.
>>
>> But maybe I'm missing something...


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
                   ` (4 preceding siblings ...)
  2022-02-25 12:22 ` [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
@ 2022-02-27 19:57 ` Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
                     ` (4 more replies)
  5 siblings, 5 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-27 19:57 UTC (permalink / raw)
  To: git; +Cc: Ævar Arnfjörð Bjarmason, Justin Donnelly

These patches are about the characters and words that can be configured to
display in the PS1 prompt after the branch name. I've been unable to find a
consistent terminology. I refer to them as follows: [short | long] [type]
state indicator where short is for characters (e.g. ?), long is for words
(e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
I'd be happy to change the commit messages to a different terminology if
that's preferred.

There are a few inconsistencies with the PS1 prompt upstream state indicator
(GIT_PS1_SHOWUPSTREAM).

 * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
   indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
   upstream state indicator appears adjacent to the branch name (e.g.
   (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
   (main =)).
 * If there are long state indicators (e.g. |SPARSE), a short upstream state
   indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
   state indicator (e.g. (main +|SPARSE=)) instead of with the other short
   state indicators (e.g. (main +=|SPARSE)).
 * The long upstream state indicator (e.g. GIT_PS1_SHOWUPSTREAM="verbose")
   is separated from other (short or long) state indicators by a hard-coded
   SP. Other long state indicators are separated by a hard-coded pipe (|).

These patches are to make the upstream state indicators more consistent with
other state indicators.

----------------------------------------------------------------------------

Changes since v1:

 * Added __git_ps1 examples and before/after tables to commit messages where
   applicable. This should make it clearer what the behavior is for other
   (not upstream) state indicators, and how the patches make the upstream
   state indicator more consistent.
 * Removed some extraneous information about long state indicators from
   patch 2 commit message. This wasn't really helpful, and was a
   distraction.

----------------------------------------------------------------------------

Justin Donnelly (4):
  git-prompt: rename `upstream` to `upstream_type`
  git-prompt: make upstream state indicator location consistent
  git-prompt: make long upstream state indicator consistent
  git-prompt: put upstream comments together

 contrib/completion/git-prompt.sh | 59 ++++++++++++++++----------------
 1 file changed, 30 insertions(+), 29 deletions(-)


base-commit: 4c53a8c20f8984adb226293a3ffd7b88c3f4ac1a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1162%2Fjustinrdonnelly%2Fgit-prompt-upstream-consistency-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1162/justinrdonnelly/git-prompt-upstream-consistency-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1162

Range-diff vs v1:

 1:  1db836bb309 = 1:  1db836bb309 git-prompt: rename `upstream` to `upstream_type`
 2:  b503cac5ae3 ! 2:  4bf120b1bf8 git-prompt: make upstream state indicator location consistent
     @@ Commit message
          git-prompt: make upstream state indicator location consistent
      
          Make upstream state indicator location more consistent with similar
     -    state indicators (e.g. sparse). Group the short state indicator (`=`,
     -    `<`, `>`, or `<>`) with other short state indicators immediately after
     -    the branch name. Group the long state indicator (e.g. `u+2-1
     -    origin/main`) with other long state indicators after the short state
     -    indicators. Previously short and long upstream state indicators appeared
     -    after all other state indicators.
     +    state indicators (e.g. sparse). Group the short upstream state indicator
     +    (`=`, `<`, `>`, or `<>`) with other short state indicators immediately
     +    after the branch name. Previously short and long upstream state
     +    indicators appeared after all other state indicators.
      
          Use a separator (`SP` or `GIT_PS1_STATESEPARATOR`) between branch name
          and short upstream state indicator. Previously the short upstream state
          indicator would sometimes appear directly adjacent to the branch name
     -    (e.g. `(main=)`) instead of being separated (e.g. `(main =)`).
     +    instead of being separated.
     +
     +    For comparison, `__git_ps1` examples without upstream state indicator:
     +    (main)
     +    (main %)
     +    (main *%)
     +    (main|SPARSE)
     +    (main %|SPARSE)
     +    (main *%|SPARSE)
     +    (main|SPARSE|REBASE 1/2)
     +    (main %|SPARSE|REBASE 1/2)
     +
     +    Note that if there are short state indicators, they appear together
     +    after the branch name and separated from it by `SP` or
     +    `GIT_PS1_STATESEPARATOR`.
     +
     +    Before/after examples with short upstream state indicator:
     +    | Before           | After            |
     +    | ---------------- | ---------------- |
     +    | (main=)          | (main =)         |
     +    | (main|SPARSE=)   | (main =|SPARSE)  |
     +    | (main %|SPARSE=) | (main %=|SPARSE) |
      
          Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
      
 3:  83766e33614 ! 3:  0af083413b8 git-prompt: make long upstream state indicator consistent
     @@ Metadata
       ## Commit message ##
          git-prompt: make long upstream state indicator consistent
      
     -    Use a pipe as a delimiter between short state indicators and long
     -    upstream state indicator (e.g. `(main *|u+2-1 origin/main)` instead of
     -    `(main * u+2-1 origin/main)`) . This is consistent with long state
     -    indicators for sparse and in-progress operations (e.g. merge).
     +    Use a pipe as a separator before long upstream state indicator. This is
     +    consistent with long state indicators for sparse and in-progress
     +    operations (e.g. merge).
     +
     +    For comparison, `__git_ps1` examples without upstream state indicator:
     +    (main)
     +    (main %)
     +    (main *%)
     +    (main|SPARSE)
     +    (main %|SPARSE)
     +    (main *%|SPARSE)
     +    (main|SPARSE|REBASE 1/2)
     +    (main %|SPARSE|REBASE 1/2)
     +
     +    Note that if there are long state indicators, they appear after short
     +    state indicators if there are any, or after the branch name if there are
     +    no short state indicators. Each long state indicator begins with a pipe
     +    (`|`) as a separator.
     +
     +    Before/after examples with long upstream state indicator:
     +    | Before                          | After                           |
     +    | ------------------------------- | ------------------------------- |
     +    | (main u=)                       | (main|u=)                       |
     +    | (main u= origin/main)           | (main|u= origin/main)           |
     +    | (main u+1)                      | (main|u+1)                      |
     +    | (main u+1 origin/main)          | (main|u+1 origin/main)          |
     +    | (main % u=)                     | (main %|u=)                     |
     +    | (main % u= origin/main)         | (main %|u= origin/main)         |
     +    | (main % u+1)                    | (main %|u+1)                    |
     +    | (main % u+1 origin/main)        | (main %|u+1 origin/main)        |
     +    | (main|SPARSE u=)                | (main|SPARSE|u=)                |
     +    | (main|SPARSE u= origin/main)    | (main|SPARSE|u= origin/main)    |
     +    | (main|SPARSE u+1)               | (main|SPARSE|u+1)               |
     +    | (main|SPARSE u+1 origin/main)   | (main|SPARSE|u+1 origin/main)   |
     +    | (main %|SPARSE u=)              | (main %|SPARSE|u=)              |
     +    | (main %|SPARSE u= origin/main)  | (main %|SPARSE|u= origin/main)  |
     +    | (main %|SPARSE u+1)             | (main %|SPARSE|u+1)             |
     +    | (main %|SPARSE u+1 origin/main) | (main %|SPARSE|u+1 origin/main) |
      
          Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
      
 4:  58ff3d8affe = 4:  06e51dc5093 git-prompt: put upstream comments together

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/4] git-prompt: rename `upstream` to `upstream_type`
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
@ 2022-02-27 19:57   ` Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-27 19:57 UTC (permalink / raw)
  To: git
  Cc: Ævar Arnfjörð Bjarmason, Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

In `__git_ps1_show_upstream` rename the variable `upstream` to
`upstream_type`. This allows `__git_ps1_show_upstream` to reference a
variable named `upstream` that is declared `local` in `__git_ps1`, which
calls `__git_ps1_show_upstream`.

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index db7c0068fb5..3997e099aa7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -115,7 +115,7 @@ __git_ps1_show_upstream ()
 {
 	local key value
 	local svn_remote svn_url_pattern count n
-	local upstream=git legacy="" verbose="" name=""
+	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
 	# get some config options from git-config
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 		svn-remote.*.url)
 			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
 			svn_url_pattern="$svn_url_pattern\\|$value"
-			upstream=svn+git # default upstream is SVN if available, else git
+			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
 	done <<< "$output"
@@ -141,16 +141,16 @@ __git_ps1_show_upstream ()
 	local option
 	for option in ${GIT_PS1_SHOWUPSTREAM}; do
 		case "$option" in
-		git|svn) upstream="$option" ;;
+		git|svn) upstream_type="$option" ;;
 		verbose) verbose=1 ;;
 		legacy)  legacy=1  ;;
 		name)    name=1 ;;
 		esac
 	done
 
-	# Find our upstream
-	case "$upstream" in
-	git)    upstream="@{upstream}" ;;
+	# Find our upstream type
+	case "$upstream_type" in
+	git)    upstream_type="@{upstream}" ;;
 	svn*)
 		# get the upstream from the "git-svn-id: ..." in a commit message
 		# (git-svn uses essentially the same procedure internally)
@@ -167,12 +167,12 @@ __git_ps1_show_upstream ()
 
 			if [[ -z "$svn_upstream" ]]; then
 				# default branch name for checkouts with no layout:
-				upstream=${GIT_SVN_ID:-git-svn}
+				upstream_type=${GIT_SVN_ID:-git-svn}
 			else
-				upstream=${svn_upstream#/}
+				upstream_type=${svn_upstream#/}
 			fi
-		elif [[ "svn+git" = "$upstream" ]]; then
-			upstream="@{upstream}"
+		elif [[ "svn+git" = "$upstream_type" ]]; then
+			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
@@ -180,11 +180,11 @@ __git_ps1_show_upstream ()
 	# Find how many commits we are ahead/behind our upstream
 	if [[ -z "$legacy" ]]; then
 		count="$(git rev-list --count --left-right \
-				"$upstream"...HEAD 2>/dev/null)"
+				"$upstream_type"...HEAD 2>/dev/null)"
 	else
 		# produce equivalent output to --count for older versions of git
 		local commits
-		if commits="$(git rev-list --left-right "$upstream"...HEAD 2>/dev/null)"
+		if commits="$(git rev-list --left-right "$upstream_type"...HEAD 2>/dev/null)"
 		then
 			local commit behind=0 ahead=0
 			for commit in $commits
@@ -229,7 +229,7 @@ __git_ps1_show_upstream ()
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
-				--abbrev-ref "$upstream" 2>/dev/null)
+				--abbrev-ref "$upstream_type" 2>/dev/null)
 			if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
 				p="$p \${__git_ps1_upstream_name}"
 			else
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/4] git-prompt: make upstream state indicator location consistent
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
@ 2022-02-27 19:57   ` Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-27 19:57 UTC (permalink / raw)
  To: git
  Cc: Ævar Arnfjörð Bjarmason, Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Make upstream state indicator location more consistent with similar
state indicators (e.g. sparse). Group the short upstream state indicator
(`=`, `<`, `>`, or `<>`) with other short state indicators immediately
after the branch name. Previously short and long upstream state
indicators appeared after all other state indicators.

Use a separator (`SP` or `GIT_PS1_STATESEPARATOR`) between branch name
and short upstream state indicator. Previously the short upstream state
indicator would sometimes appear directly adjacent to the branch name
instead of being separated.

For comparison, `__git_ps1` examples without upstream state indicator:
(main)
(main %)
(main *%)
(main|SPARSE)
(main %|SPARSE)
(main *%|SPARSE)
(main|SPARSE|REBASE 1/2)
(main %|SPARSE|REBASE 1/2)

Note that if there are short state indicators, they appear together
after the branch name and separated from it by `SP` or
`GIT_PS1_STATESEPARATOR`.

Before/after examples with short upstream state indicator:
| Before           | After            |
| ---------------- | ---------------- |
| (main=)          | (main =)         |
| (main|SPARSE=)   | (main =|SPARSE)  |
| (main %|SPARSE=) | (main %=|SPARSE) |

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 3997e099aa7..613389a53bc 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -109,7 +109,7 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
-# stores the divergence from upstream in $p
+# stores the divergence from upstream in $p (for short status) or $upstream (for verbose status)
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
 {
@@ -214,26 +214,26 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			p="<>" ;;
 		esac
-	else
+	else # verbose, set upstream instead of p
 		case "$count" in
 		"") # no upstream
-			p="" ;;
+			upstream="" ;;
 		"0	0") # equal to upstream
-			p=" u=" ;;
+			upstream=" u=" ;;
 		"0	"*) # ahead of upstream
-			p=" u+${count#0	}" ;;
+			upstream=" u+${count#0	}" ;;
 		*"	0") # behind upstream
-			p=" u-${count%	0}" ;;
+			upstream=" u-${count%	0}" ;;
 		*)	    # diverged from upstream
-			p=" u+${count#*	}-${count%	*}" ;;
+			upstream=" u+${count#*	}-${count%	*}" ;;
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
 				--abbrev-ref "$upstream_type" 2>/dev/null)
 			if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
-				p="$p \${__git_ps1_upstream_name}"
+				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
-				p="$p ${__git_ps1_upstream_name}"
+				upstream="$upstream ${__git_ps1_upstream_name}"
 				# not needed anymore; keep user's
 				# environment clean
 				unset __git_ps1_upstream_name
@@ -512,7 +512,8 @@ __git_ps1 ()
 	local u=""
 	local h=""
 	local c=""
-	local p=""
+	local p="" # short version of upstream state indicator
+	local upstream="" # verbose version of upstream state indicator
 
 	if [ "true" = "$inside_gitdir" ]; then
 		if [ "true" = "$bare_repo" ]; then
@@ -568,8 +569,8 @@ __git_ps1 ()
 		b="\${__git_ps1_branch_name}"
 	fi
 
-	local f="$h$w$i$s$u"
-	local gitstring="$c$b${f:+$z$f}${sparse}$r$p"
+	local f="$h$w$i$s$u$p"
+	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}"
 
 	if [ $pcmode = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/4] git-prompt: make long upstream state indicator consistent
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
@ 2022-02-27 19:57   ` Justin Donnelly via GitGitGadget
  2022-02-27 19:57   ` [PATCH v2 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
  2022-03-22 12:25   ` [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-27 19:57 UTC (permalink / raw)
  To: git
  Cc: Ævar Arnfjörð Bjarmason, Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Use a pipe as a separator before long upstream state indicator. This is
consistent with long state indicators for sparse and in-progress
operations (e.g. merge).

For comparison, `__git_ps1` examples without upstream state indicator:
(main)
(main %)
(main *%)
(main|SPARSE)
(main %|SPARSE)
(main *%|SPARSE)
(main|SPARSE|REBASE 1/2)
(main %|SPARSE|REBASE 1/2)

Note that if there are long state indicators, they appear after short
state indicators if there are any, or after the branch name if there are
no short state indicators. Each long state indicator begins with a pipe
(`|`) as a separator.

Before/after examples with long upstream state indicator:
| Before                          | After                           |
| ------------------------------- | ------------------------------- |
| (main u=)                       | (main|u=)                       |
| (main u= origin/main)           | (main|u= origin/main)           |
| (main u+1)                      | (main|u+1)                      |
| (main u+1 origin/main)          | (main|u+1 origin/main)          |
| (main % u=)                     | (main %|u=)                     |
| (main % u= origin/main)         | (main %|u= origin/main)         |
| (main % u+1)                    | (main %|u+1)                    |
| (main % u+1 origin/main)        | (main %|u+1 origin/main)        |
| (main|SPARSE u=)                | (main|SPARSE|u=)                |
| (main|SPARSE u= origin/main)    | (main|SPARSE|u= origin/main)    |
| (main|SPARSE u+1)               | (main|SPARSE|u+1)               |
| (main|SPARSE u+1 origin/main)   | (main|SPARSE|u+1 origin/main)   |
| (main %|SPARSE u=)              | (main %|SPARSE|u=)              |
| (main %|SPARSE u= origin/main)  | (main %|SPARSE|u= origin/main)  |
| (main %|SPARSE u+1)             | (main %|SPARSE|u+1)             |
| (main %|SPARSE u+1 origin/main) | (main %|SPARSE|u+1 origin/main) |

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 613389a53bc..2772f990888 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -109,7 +109,7 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
-# stores the divergence from upstream in $p (for short status) or $upstream (for verbose status)
+# stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
 {
@@ -219,13 +219,13 @@ __git_ps1_show_upstream ()
 		"") # no upstream
 			upstream="" ;;
 		"0	0") # equal to upstream
-			upstream=" u=" ;;
+			upstream="|u=" ;;
 		"0	"*) # ahead of upstream
-			upstream=" u+${count#0	}" ;;
+			upstream="|u+${count#0	}" ;;
 		*"	0") # behind upstream
-			upstream=" u-${count%	0}" ;;
+			upstream="|u-${count%	0}" ;;
 		*)	    # diverged from upstream
-			upstream=" u+${count#*	}-${count%	*}" ;;
+			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
 		if [[ -n "$count" && -n "$name" ]]; then
 			__git_ps1_upstream_name=$(git rev-parse \
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/4] git-prompt: put upstream comments together
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
                     ` (2 preceding siblings ...)
  2022-02-27 19:57   ` [PATCH v2 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
@ 2022-02-27 19:57   ` Justin Donnelly via GitGitGadget
  2022-03-22 12:25   ` [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 15+ messages in thread
From: Justin Donnelly via GitGitGadget @ 2022-02-27 19:57 UTC (permalink / raw)
  To: git
  Cc: Ævar Arnfjörð Bjarmason, Justin Donnelly, Justin Donnelly

From: Justin Donnelly <justinrdonnelly@gmail.com>

Commit 6d158cba28 (bash completion: Support "divergence from upstream"
messages in __git_ps1, 2010-06-17) introduced support for indicating
divergence from upstream in the PS1 prompt. The comments at the top of
git-prompt.sh that were introduced with that commit are several
paragraphs long. Over the years, other comments have been inserted in
between the paragraphs relating to divergence from upstream.

This commit puts the comments relating to divergence from upstream back
together.

Signed-off-by: Justin Donnelly <justinrdonnelly@gmail.com>
---
 contrib/completion/git-prompt.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 2772f990888..87b2b916c03 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -66,6 +66,11 @@
 #     git           always compare HEAD to @{upstream}
 #     svn           always compare HEAD to your SVN upstream
 #
+# By default, __git_ps1 will compare HEAD to your SVN upstream if it can
+# find one, or @{upstream} otherwise.  Once you have set
+# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
+# setting the bash.showUpstream config variable.
+#
 # You can change the separator between the branch name and the above
 # state symbols by setting GIT_PS1_STATESEPARATOR. The default separator
 # is SP.
@@ -79,11 +84,6 @@
 # single '?' character by setting GIT_PS1_COMPRESSSPARSESTATE, or omitted
 # by setting GIT_PS1_OMITSPARSESTATE.
 #
-# By default, __git_ps1 will compare HEAD to your SVN upstream if it can
-# find one, or @{upstream} otherwise.  Once you have set
-# GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
-# setting the bash.showUpstream config variable.
-#
 # If you would like to see more information about the identity of
 # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE
 # to one of these values:
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
                     ` (3 preceding siblings ...)
  2022-02-27 19:57   ` [PATCH v2 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
@ 2022-03-22 12:25   ` Ævar Arnfjörð Bjarmason
  2022-03-23 20:06     ` Junio C Hamano
  4 siblings, 1 reply; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-03-22 12:25 UTC (permalink / raw)
  To: Justin Donnelly via GitGitGadget; +Cc: git, Justin Donnelly


On Sun, Feb 27 2022, Justin Donnelly via GitGitGadget wrote:

> These patches are about the characters and words that can be configured to
> display in the PS1 prompt after the branch name. I've been unable to find a
> consistent terminology. I refer to them as follows: [short | long] [type]
> state indicator where short is for characters (e.g. ?), long is for words
> (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
> I'd be happy to change the commit messages to a different terminology if
> that's preferred.
>
> There are a few inconsistencies with the PS1 prompt upstream state indicator
> (GIT_PS1_SHOWUPSTREAM).
>
>  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
>    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
>    upstream state indicator appears adjacent to the branch name (e.g.
>    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
>    (main =)).
>  * If there are long state indicators (e.g. |SPARSE), a short upstream state
>    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
>    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
>    state indicators (e.g. (main +=|SPARSE)).
>  * The long upstream state indicator (e.g. GIT_PS1_SHOWUPSTREAM="verbose")
>    is separated from other (short or long) state indicators by a hard-coded
>    SP. Other long state indicators are separated by a hard-coded pipe (|).
>
> These patches are to make the upstream state indicators more consistent with
> other state indicators.
>
> ----------------------------------------------------------------------------
>
> Changes since v1:
>
>  * Added __git_ps1 examples and before/after tables to commit messages where
>    applicable. This should make it clearer what the behavior is for other
>    (not upstream) state indicators, and how the patches make the upstream
>    state indicator more consistent.
>  * Removed some extraneous information about long state indicators from
>    patch 2 commit message. This wasn't really helpful, and was a
>    distraction.

Since this was all in reponse to my review: I've looked this over again
and this all LGTM now:

Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators
  2022-03-22 12:25   ` [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
@ 2022-03-23 20:06     ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2022-03-23 20:06 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Justin Donnelly via GitGitGadget, git, Justin Donnelly

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Sun, Feb 27 2022, Justin Donnelly via GitGitGadget wrote:
>
>> These patches are about the characters and words that can be configured to
>> display in the PS1 prompt after the branch name. I've been unable to find a
>> consistent terminology. I refer to them as follows: [short | long] [type]
>> state indicator where short is for characters (e.g. ?), long is for words
>> (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
>> I'd be happy to change the commit messages to a different terminology if
>> that's preferred.
>>
>> There are a few inconsistencies with the PS1 prompt upstream state indicator
>> (GIT_PS1_SHOWUPSTREAM).
>>
>>  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
>>    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
>>    upstream state indicator appears adjacent to the branch name (e.g.
>>    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
>>    (main =)).
>>  * If there are long state indicators (e.g. |SPARSE), a short upstream state
>>    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
>>    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
>>    state indicators (e.g. (main +=|SPARSE)).
>>  * The long upstream state indicator (e.g. GIT_PS1_SHOWUPSTREAM="verbose")
>>    is separated from other (short or long) state indicators by a hard-coded
>>    SP. Other long state indicators are separated by a hard-coded pipe (|).
>>
>> These patches are to make the upstream state indicators more consistent with
>> other state indicators.
>>
>> ----------------------------------------------------------------------------
>>
>> Changes since v1:
>>
>>  * Added __git_ps1 examples and before/after tables to commit messages where
>>    applicable. This should make it clearer what the behavior is for other
>>    (not upstream) state indicators, and how the patches make the upstream
>>    state indicator more consistent.
>>  * Removed some extraneous information about long state indicators from
>>    patch 2 commit message. This wasn't really helpful, and was a
>>    distraction.
>
> Since this was all in reponse to my review: I've looked this over again
> and this all LGTM now:
>
> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Thanks, both.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-23 20:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 11:44 [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Justin Donnelly via GitGitGadget
2022-02-25 11:44 ` [PATCH 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
2022-02-25 11:44 ` [PATCH 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
2022-02-25 11:44 ` [PATCH 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
2022-02-25 11:44 ` [PATCH 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
2022-02-25 12:22 ` [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
2022-02-27  0:32   ` Justin Donnelly
2022-02-27 10:19     ` Ævar Arnfjörð Bjarmason
2022-02-27 19:57 ` [PATCH v2 " Justin Donnelly via GitGitGadget
2022-02-27 19:57   ` [PATCH v2 1/4] git-prompt: rename `upstream` to `upstream_type` Justin Donnelly via GitGitGadget
2022-02-27 19:57   ` [PATCH v2 2/4] git-prompt: make upstream state indicator location consistent Justin Donnelly via GitGitGadget
2022-02-27 19:57   ` [PATCH v2 3/4] git-prompt: make long upstream state indicator consistent Justin Donnelly via GitGitGadget
2022-02-27 19:57   ` [PATCH v2 4/4] git-prompt: put upstream comments together Justin Donnelly via GitGitGadget
2022-03-22 12:25   ` [PATCH v2 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators Ævar Arnfjörð Bjarmason
2022-03-23 20:06     ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.