[PATCH 0/8] git-prompt: support more shells

[PATCH 0/8] git-prompt: support more shells

[Avi Halachmi via GitGitGadget]

Before this patchset only bash and zsh were supported.

After this patchset, the following shells work: bash, zsh, dash (since at
least 0.5.8), free/net bsd sh, busybox-ash, mksh, openbsd sh, pdksh(!),
Schily extended Bourne sh (bosh), yash.

The code should now be almost posix-compliant, with one big exception
("local" variables in functions) which is not simple to fix.

Shells which don't work, likely only due to missing "local": ksh93[u+m],
Schily minimal posix Bourne sh (pbosh), yash-posix-mode.

Most changes are trivial, like changing [[...]] to [...], with one exception
(git-prompt: don't use shell arrays) which changes few lines.

Tested with and without colors, with diversions from upstream, in git-svn
repos, and more, but I'm not considering it full coverage.

 * avih

Avi Halachmi (:avih) (8):
  git-prompt: use here-doc instead of here-string
  git-prompt: fix uninitialized variable
  git-prompt: don't use shell arrays
  git-prompt: replace [[...]] with standard code
  git-prompt: add some missing quotes
  git-prompt: add fallback for shells without $'...'
  git-prompt: ta-da! document usage in other shells
  git-prompt: support custom 0-width PS1 markers

 contrib/completion/git-prompt.sh | 196 +++++++++++++++++++++----------
 1 file changed, 131 insertions(+), 65 deletions(-)


base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1750%2Favih%2Fprompt-compat-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1750/avih/prompt-compat-v1
Pull-Request: https://github.com/git/git/pull/1750
-- 
gitgitgadget

[PATCH 1/8] git-prompt: use here-doc instead of here-string

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Here-documend is standard, and works in all shells.

Both here-string and here-doc add final newline, which is important
in this case, because $output is without final newline, but we do
want "read" to succeed on the last line as well.

Shells which support here-string:
- bash, zsh, mksh, ksh93, yash (non-posix-mode).

shells which don't, and got fixed:
- ash-derivatives (dash, free/net bsd sh, busybox-ash).
- pdksh, openbsd sh.
- All Schily Bourne shell variants.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5330e769a72..ebf2e30d684 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -137,7 +137,9 @@ __git_ps1_show_upstream ()
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
-	done <<< "$output"
+	done <<-OUTPUT
+		$output
+	OUTPUT
 
 	# parse configuration values
 	local option
-- 
gitgitgadget

[PATCH 2/8] git-prompt: fix uninitialized variable

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

First use is in the form:  local var; ...; var=$var$whatever...

If the variable was unset (as bash and others do after "local x"),
then it would error if set -u is in effect.

Also, many shells inherit the existing value after "local var"
without init, but in this case it's unlikely to have a prior value.

Now we initialize it.

(local var= is enough, but local var="" is the custom in this file)

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index ebf2e30d684..4cc2cf91bb6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,7 +116,7 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern count n
+	local svn_remote svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
-- 
gitgitgadget

[PATCH 3/8] git-prompt: don't use shell arrays

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Arrays only existed in the svn-upstream code, used to:
- Keep a list of svn remotes.
- Convert commit msg to array of words, extract the 2nd-to-last word.

Except bash/zsh, nearly all shells failed load on syntax errors here.

Now:
- The svn remotes are a list of newline-terminated values.
- The 2nd-to-last word is extracted using standard shell substrings.
- All shells can digest the svn-upstream code.

While using shell field splitting to extract the word is simple, and
doesn't even need non-standard code, e.g. set -- $(git log -1 ...),
it would have the same issues as the old array code: it depends on IFS
which we don't control, and it's subject to glob-expansion, e.g. if
the message happens to include * or **/* (as this commit message just
did), then the array could get huge. This was not great.

Now it uses standard shell substrings, and we know the exact delimiter
to expect, because it's the match from our grep just one line earlier.

The new word extraction code also fixes svn-upstream in zsh, because
previously it used arr[len-2], but because in zsh, unlike bash, array
subscripts are 1-based, it incorrectly extracted the 3rd-to-last word.
symptom: missing upstream status in a git-svn repo: u=, u+N-M, etc.

The breakage in zsh is surprising, because it was last touched by
  commit d0583da838 (prompt: fix show upstream with svn and zsh),
claiming to fix exactly that. However, it only mentions syntax fixes.
It's unclear if behavior was fixed too. But it was broken, now fixed.

Note LF=$'\n' and then using $LF instead of $'\n' few times.
A future commit will add fallback for shells without $'...', so this
would be the only line to touch instead of replacing every $'\n' .

Shells which could run the previous array code:
- bash

Shells which have arrays but were broken anyway:
- zsh: 1-based subscript
- ksh93: no "local" (the new code can't fix this part...)
- mksh, openbsd sh, pdksh: failed load on syntax error: "for ((...))".

More shells which Failed to load due to syntax error:
- dash, free/net bsd sh, busybox-ash, Schily Bourne shell, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 48 ++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4cc2cf91bb6..75c3a813fda 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,10 +116,10 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern="" count n
+	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
+	local LF=$'\n'
 
-	svn_remote=()
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
 	while read -r key value; do
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
+			svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
 			svn_url_pattern="$svn_url_pattern\\|$value"
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
@@ -156,25 +156,37 @@ __git_ps1_show_upstream ()
 	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)
-		local -a svn_upstream
-		svn_upstream=($(git log --first-parent -1 \
-					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
-		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-			svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
-			svn_upstream=${svn_upstream%@*}
-			local n_stop="${#svn_remote[@]}"
-			for ((n=1; n <= n_stop; n++)); do
-				svn_upstream=${svn_upstream#${svn_remote[$n]}}
-			done
+		# successful svn-upstream resolution:
+		# - get the list of configured svn-remotes ($svn_remotes set above)
+		# - get the last commit which seems from one of our svn-remotes
+		# - confirm that it is from one of the svn-remotes
+		# - use $GIT_SVN_ID if set, else "git-svn"
 
-			if [[ -z "$svn_upstream" ]]; then
+		# get upstream from "git-svn-id: UPSTRM@N HASH" in a commit message
+		# (git-svn uses essentially the same procedure internally)
+		local svn_upstream="$(
+			git log --first-parent -1 \
+				--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null
+		)"
+
+		if [ -n "$svn_upstream" ]; then
+			# extract the URI, assuming --grep matched the last line
+			svn_upstream=${svn_upstream##*$LF}  # last line
+			svn_upstream=${svn_upstream#*: }    # UPSTRM@N HASH
+			svn_upstream=${svn_upstream%@*}     # UPSTRM
+
+			case ${LF}${svn_remotes} in
+			*"${LF}${svn_upstream}${LF}"*)
+				# grep indeed matched the last line - it's our remote
 				# default branch name for checkouts with no layout:
 				upstream_type=${GIT_SVN_ID:-git-svn}
-			else
+				;;
+			*)
+				# the commit message includes one of our remotes, but
+				# it's not at the last line. is $svn_upstream junk?
 				upstream_type=${svn_upstream#/}
-			fi
+				;;
+			esac
 		elif [[ "svn+git" = "$upstream_type" ]]; then
 			upstream_type="@{upstream}"
 		fi
-- 
gitgitgadget

[PATCH 4/8] git-prompt: replace [[...]] with standard code

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The existing [[...]] tests were either already valid as standard [...]
tests, or only required minimal retouch:

Notes:

- [[...]] doesn't do field splitting and glob expansion, so $var
  or $(cmd...) don't need quoting, but [... does need quotes.

- [[ X == Y ]] when Y is a string is same as [ X = Y ], but if Y is
  a pattern, then we need:  case X in Y)... ; esac  .

- [[ ... && ... ]] was replaced with [ ... ] && [ ... ] .

- [[ -o <zsh-option> ]] requires [[...]], so put it in "eval" and only
  eval it in zsh, so other shells would not abort on syntax error
  (posix says [[ has unspecified results, shells allowed to reject it)

- ((x++)) was changed into x=$((x+1))  (yeah, not [[...]] ...)

Shells which accepted the previous forms:
- bash, zsh, ksh93, mksh, openbsd sh, pdksh.

Shells which didn't, and now can process it:
- dash, free/net bsd sh, busybox-ash, Schily Bourne sh, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 75c3a813fda..4781261f868 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -126,7 +126,7 @@ __git_ps1_show_upstream ()
 		case "$key" in
 		bash.showupstream)
 			GIT_PS1_SHOWUPSTREAM="$value"
-			if [[ -z "${GIT_PS1_SHOWUPSTREAM}" ]]; then
+			if [ -z "${GIT_PS1_SHOWUPSTREAM}" ]; then
 				p=""
 				return
 			fi
@@ -187,14 +187,14 @@ __git_ps1_show_upstream ()
 				upstream_type=${svn_upstream#/}
 				;;
 			esac
-		elif [[ "svn+git" = "$upstream_type" ]]; then
+		elif [ "svn+git" = "$upstream_type" ]; then
 			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
 
 	# Find how many commits we are ahead/behind our upstream
-	if [[ -z "$legacy" ]]; then
+	if [ -z "$legacy" ]; then
 		count="$(git rev-list --count --left-right \
 				"$upstream_type"...HEAD 2>/dev/null)"
 	else
@@ -206,8 +206,8 @@ __git_ps1_show_upstream ()
 			for commit in $commits
 			do
 				case "$commit" in
-				"<"*) ((behind++)) ;;
-				*)    ((ahead++))  ;;
+				"<"*) behind=$((behind+1)) ;;
+				*)    ahead=$((ahead+1))   ;;
 				esac
 			done
 			count="$behind	$ahead"
@@ -217,7 +217,7 @@ __git_ps1_show_upstream ()
 	fi
 
 	# calculate the result
-	if [[ -z "$verbose" ]]; then
+	if [ -z "$verbose" ]; then
 		case "$count" in
 		"") # no upstream
 			p="" ;;
@@ -243,7 +243,7 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
-		if [[ -n "$count" && -n "$name" ]]; then
+		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
@@ -265,7 +265,7 @@ __git_ps1_show_upstream ()
 # their own color.
 __git_ps1_colorize_gitstring ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
+	if [ -n "${ZSH_VERSION-}" ]; then
 		local c_red='%F{red}'
 		local c_green='%F{green}'
 		local c_lblue='%F{blue}'
@@ -417,7 +417,7 @@ __git_ps1 ()
 	# incorrect.)
 	#
 	local ps1_expanded=yes
-	[ -z "${ZSH_VERSION-}" ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+	[ -z "${ZSH_VERSION-}" ] || eval '[[ -o PROMPT_SUBST ]]' || ps1_expanded=no
 	[ -z "${BASH_VERSION-}" ] || shopt -q promptvars || ps1_expanded=no
 
 	local repo_info rev_parse_exit_code
@@ -502,11 +502,13 @@ __git_ps1 ()
 					return $exit
 				fi
 
-				if [[ $head == "ref: "* ]]; then
+				case $head in
+				"ref: "*)
 					head="${head#ref: }"
-				else
+					;;
+				*)
 					head=""
-				fi
+				esac
 				;;
 			*)
 				head="$(git symbolic-ref HEAD 2>/dev/null)"
@@ -542,8 +544,8 @@ __git_ps1 ()
 	fi
 
 	local conflict="" # state indicator for unresolved conflicts
-	if [[ "${GIT_PS1_SHOWCONFLICTSTATE-}" == "yes" ]] &&
-	   [[ $(git ls-files --unmerged 2>/dev/null) ]]; then
+	if [ "${GIT_PS1_SHOWCONFLICTSTATE-}" = "yes" ] &&
+	   [ "$(git ls-files --unmerged 2>/dev/null)" ]; then
 		conflict="|CONFLICT"
 	fi
 
-- 
gitgitgadget

[PATCH 5/8] git-prompt: add some missing quotes

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The issues which this commit fixes are unlikely to be broken
in real life, but the fixes improve correctness, and would prevent
bugs in some uncommon cases, such as weird IFS values.

Listing some portability guideline here for future reference.

I'm leaving it to someone else to decide whether to include
it in the file itself, place is as a new file, or not.

---------

The command "local" is non standard, but is allowed in this file:
- Quote initialization if it can expand (local x="$y"). See below.
- Don't assume initial value after "local x". Either initialize it
  (local x=..), or set before first use (local x;.. x=..; <use $x>).
  (between shells, "local x" can unset x, or inherit it, or do x= )

Other non-standard features beyond "local" are to be avoided.

Use the standard "test" - [...] instead of non-standard [[...]] .

--------

Quotes (some portability things, but mainly general correctness):

Quotes prevent tilde-expansion of some unquoted literal tildes (~).
If the expansion is undesirable, quotes would ensure that.
  Tilds expanded: a=~user:~/ ;  echo ~user ~/dir
  not expanded:   t="~"; a=${t}user  b=\~foo~;  echo "~user" $t/dir

But the main reason for quoting is to prevent IFS field splitting
(which also coalesces IFS chars) and glob expansion after parameter
expansion or command substitution.

In _command-arguments_, expanded/substituted values must be quoted:
  Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
  Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)

Still in _agumemts_, no need to quote non-expandable values:
  Good:                 local x=   y=yes;   echo OK
  OK, but not required: local x="" y="yes"; echo "OK"
But completely empty (NULL) arguments must be quoted:
  foo ""   is not the same as:   foo

Assignments in simple commands - with or without an actual command,
don't need quoting becase there's no IFS split or glob expansion:
  Good:   s=* a=$b c=$(cmd...)${x# foo }${y-   } [cmd ...]
  It's also OK to use double quotes, but not required.

This behavior (no IFS/glob) is called "assignment context", and
"local" does not behave with assignment context in some shells,
hence we require quotes when using "local" - for compatibility.

First value in 'case...' doesn't IFS-split/glob, doesn't need quotes:
  Good:       case  * $foo $(cmd...)  in ... ; esac
  identical:  case "* $foo $(cmd...)" in ... ; esac

Nested quotes in command substitution are fine, often necessary:
  Good: echo "$(foo... "$x" "$(bar ...)")"

Nested quotes in substring ops are legal, and sometimes needed
to prevent interpretation as a pattern, but not the most readable:
  Legal:  foo "${x#*"$y" }"

Nested quotes in "maybe other value" subst are invalid, unnecessary:
  Good:  local x="${y- }";   foo "${z:+ $a }"
  Bad:   local x="${y-" "}"; foo "${z:+" $a "}"
Outer/inner quotes in "maybe other value" have different use cases:
  "${x-$y}"  always one quoted arg: "$x" if x is set, else "$y".
  ${x+"$x"}  one quoted arg "$x" if x is set, else no arg at all.
  Unquoted $x is similar to the second case, but it would get split
  into few arguments if it includes any of the IFS chars.

Assignments don't need the outer quotes, and the braces delimit the
value, so nested quotes can be avoided, for readability:
  a=$(foo "$x")  a=${x#*"$y" }  c=${y- };  bar "$a" "$b" "$c"

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.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 4781261f868..5d7f236fe48 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -246,7 +246,7 @@ __git_ps1_show_upstream ()
 		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
+			if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
 				upstream="$upstream ${__git_ps1_upstream_name}"
@@ -278,12 +278,12 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue=$'\001\e[1;34m\002'
 		local c_clear=$'\001\e[0m\002'
 	fi
-	local bad_color=$c_red
-	local ok_color=$c_green
+	local bad_color="$c_red"
+	local ok_color="$c_green"
 	local flags_color="$c_lblue"
 
 	local branch_color=""
-	if [ $detached = no ]; then
+	if [ "$detached" = no ]; then
 		branch_color="$ok_color"
 	else
 		branch_color="$bad_color"
@@ -360,7 +360,7 @@ __git_sequencer_status ()
 __git_ps1 ()
 {
 	# preserve exit status
-	local exit=$?
+	local exit="$?"
 	local pcmode=no
 	local detached=no
 	local ps1pc_start='\u@\h:\w '
@@ -379,7 +379,7 @@ __git_ps1 ()
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
-		*)	return $exit
+		*)	return "$exit"
 		;;
 	esac
 
@@ -427,7 +427,7 @@ __git_ps1 ()
 	rev_parse_exit_code="$?"
 
 	if [ -z "$repo_info" ]; then
-		return $exit
+		return "$exit"
 	fi
 
 	local short_sha=""
@@ -449,7 +449,7 @@ __git_ps1 ()
 	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
 	   git check-ignore -q .
 	then
-		return $exit
+		return "$exit"
 	fi
 
 	local sparse=""
@@ -499,7 +499,7 @@ __git_ps1 ()
 			case "$ref_format" in
 			files)
 				if ! __git_eread "$g/HEAD" head; then
-					return $exit
+					return "$exit"
 				fi
 
 				case $head in
@@ -597,10 +597,10 @@ __git_ps1 ()
 		fi
 	fi
 
-	local z="${GIT_PS1_STATESEPARATOR-" "}"
+	local z="${GIT_PS1_STATESEPARATOR- }"
 
 	b=${b##refs/heads/}
-	if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
+	if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 		__git_ps1_branch_name=$b
 		b="\${__git_ps1_branch_name}"
 	fi
@@ -612,7 +612,7 @@ __git_ps1 ()
 	local f="$h$w$i$s$u$p"
 	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}${conflict}"
 
-	if [ $pcmode = yes ]; then
+	if [ "$pcmode" = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
 			gitstring=$(printf -- "$printf_format" "$gitstring")
 		else
@@ -623,5 +623,5 @@ __git_ps1 ()
 		printf -- "$printf_format" "$gitstring"
 	fi
 
-	return $exit
+	return "$exit"
 }
-- 
gitgitgadget

Re: [PATCH 5/8] git-prompt: add some missing quotes

[Junio C Hamano]

"Avi Halachmi (:avih) via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
>
> The issues which this commit fixes are unlikely to be broken
> in real life, but the fixes improve correctness, and would prevent
> bugs in some uncommon cases, such as weird IFS values.
>
> Listing some portability guideline here for future reference.
>
> I'm leaving it to someone else to decide whether to include
> it in the file itself, place is as a new file, or not.

Check Documentation/CodingGuidelines; I think we have something to
say about local var="val" construct to help dash.

We allowed liberal uses of bash-ism in this file, as it was
initially written for bash anyway.  If we were rewriting the prompt
scripts to be usable by other shells, great.  But then we'd want to
make sure it adheres to existing coding guidelines we have.

Re: [PATCH 5/8] git-prompt: add some missing quotes

[avih]

 On Tuesday, July 23, 2024 at 10:40:30 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:

Thanks for the quick reply, and aplogies for my delayed reply.

I replied at the github PR https://github.com/git/git/pull/1750
and didn't realize GitGitGadget doesn't forward it to the list.
Then I accidentally sent email with HTML. 3rd time the charm...

>> Listing some portability guideline here for future reference.
>>
>> I'm leaving it to someone else to decide whether to include
>> it in the file itself, place is as a new file, or not.

> Check Documentation/CodingGuidelines; I think we have something to
> say about local var="val" construct to help dash.

I wasn't aware of this file, but I should have searched for it before
posting. Thanks for the pointer.

As far as I can tell CodingGuidelines and my guideline align perfectly
on every subject which both mention, down to nuances like that quoted
initial value in "local", though each also has few subjects which the
other doesn't.

> ... If we were rewriting the prompt
> scripts to be usable by other shells, great.  But then we'd want to
> make sure it adheres to existing coding guidelines we have.

Not sure how many prompt scripts there are, but if you're referring
to the scripts at contrib/completion then only git-prompt.sh is
applicable in many shells and would gain by being portable. The
others are shell-specific, so I wouldn't think they need be portable.

As for git-prompt.sh, as far as I can tell, after this patchset, this
file adheres to CodingGuidelines completely as far as correctness and
compatibility go.

However, regardless of not being aware of CodingGuidelines, the goal
of this patchset was to improve compatibility and correctness, and I
wouldn't have chosen or felt comfortable to included style changes
("'then' in new line" can have also portability implications, though
not in the many shells which I tested).

So no change in terms of style, it still diverges from the guidelines.

Shall I add a commit which fixes style issues?

[PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

$'...' is new in POSIX (2024), and some shells support it in recent
versions, while others have had it for decades (bash, zsh, ksh93).

However, there are still enough shells which don't support it, and
it's cheap to provide a fallback for them, so let's do that instead
of dismissing it as "it's compliant".

shells where $'...' works:
- bash, zsh, ksh93, mksh, busybox-ash, dash master, free/net bsd sh.

shells where it doesn't work, but the new fallback works:
- all dash releases (up to 0.5.12), older versions of free/net bsd sh,
  openbsd sh, pdksh, all Schily Bourne sh variants, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 52 +++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5d7f236fe48..bbc16417ac9 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -111,6 +111,17 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
+__git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
+__git_LF=$'\n' __git_CRLF=$'\r\n'
+
+if [ $'\101' != A ]; then  # fallback for shells without $'...'
+   __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
+   __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
+   __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
+   __git_SOH=${__git_CRLF#??};   __git_CRLF=${__git_CRLF%?}
+   __git_LF=${__git_CRLF#?}
+fi
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -118,7 +129,7 @@ __git_ps1_show_upstream ()
 	local key value
 	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
-	local LF=$'\n'
+	local LF="$__git_LF"
 
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
@@ -271,12 +282,16 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue='%F{blue}'
 		local c_clear='%f'
 	else
-		# Using \001 and \002 around colors is necessary to prevent
-		# issues with command line editing/browsing/completion!
-		local c_red=$'\001\e[31m\002'
-		local c_green=$'\001\e[32m\002'
-		local c_lblue=$'\001\e[1;34m\002'
-		local c_clear=$'\001\e[0m\002'
+		# \001 (SOH) and \002 (STX) are 0-width substring markers
+		# which bash/readline identify while calculating the prompt
+		# on-screen width - to exclude 0-screen-width esc sequences.
+		local c_pre="${__git_SOH}${__git_ESC}["
+		local c_post="m${__git_STX}"
+
+		local c_red="${c_pre}31${c_post}"
+		local c_green="${c_pre}32${c_post}"
+		local c_lblue="${c_pre}1;34${c_post}"
+		local c_clear="${c_pre}0${c_post}"
 	fi
 	local bad_color="$c_red"
 	local ok_color="$c_green"
@@ -312,7 +327,7 @@ __git_ps1_colorize_gitstring ()
 # variable, in that order.
 __git_eread ()
 {
-	test -r "$1" && IFS=$'\r\n' read -r "$2" <"$1"
+	test -r "$1" && IFS=$__git_CRLF read -r "$2" <"$1"
 }
 
 # see if a cherry-pick or revert is in progress, if the user has committed a
@@ -430,19 +445,20 @@ __git_ps1 ()
 		return "$exit"
 	fi
 
+	local LF="$__git_LF"
 	local short_sha=""
 	if [ "$rev_parse_exit_code" = "0" ]; then
-		short_sha="${repo_info##*$'\n'}"
-		repo_info="${repo_info%$'\n'*}"
+		short_sha="${repo_info##*$LF}"
+		repo_info="${repo_info%$LF*}"
 	fi
-	local ref_format="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_worktree="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local bare_repo="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_gitdir="${repo_info##*$'\n'}"
-	local g="${repo_info%$'\n'*}"
+	local ref_format="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_worktree="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local bare_repo="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_gitdir="${repo_info##*$LF}"
+	local g="${repo_info%$LF*}"
 
 	if [ "true" = "$inside_worktree" ] &&
 	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED-}" ] &&
-- 
gitgitgadget

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Junio C Hamano]

"Avi Halachmi (:avih) via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
>
> $'...' is new in POSIX (2024), and some shells support it in recent
> versions, while others have had it for decades (bash, zsh, ksh93).

I will not look at this series futher during the current development
cycle that is about to conclude, but ...

> +__git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
> +__git_LF=$'\n' __git_CRLF=$'\r\n'
> +
> +if [ $'\101' != A ]; then  # fallback for shells without $'...'
> +   __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
> +   __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
> +   __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
> +   __git_SOH=${__git_CRLF#??};   __git_CRLF=${__git_CRLF%?}
> +   __git_LF=${__git_CRLF#?}
> +fi

... given that these are not used literally in-place but are always
referred to by their __git_BYTE names, if we are making this script
portable across shells to the same degree as other shell scripts
following our coding guidelines, I would prefer to see it done
without any "fallback".

$(printf '\r') would work with bash, zsh and ksh93, too, and one
time assignment to these variables is not going to be performance
critical.  Just forbid use of $'\octal' notation in the coding
guidelines document, and implement just one variant.

Perhaps we should spell more things out that you wrote in some of
your proposed log messages more explicitly.  I think these have been
rules we have followed (grep for them in *.sh files outside
contrib/) but I did not find mention in the guidelines document.

Thanks.

 Documentation/CodingGuidelines | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
index 1d92b2da03..bb058fcc87 100644
--- c/Documentation/CodingGuidelines
+++ w/Documentation/CodingGuidelines
@@ -107,6 +107,8 @@ For shell scripts specifically (not exhaustive):
 
  - We do not use Process Substitution <(list) or >(list).
 
+ - We do not use Dollar-Single-Quotes $'<octal>' notation.
+
  - Do not write control structures on a single line with semicolon.
    "then" should be on the next line for if statements, and "do"
    should be on the next line for "while" and "for".
@@ -140,7 +142,8 @@ For shell scripts specifically (not exhaustive):
 	sort >actual &&
 	...
 
- - We prefer "test" over "[ ... ]".
+ - We prefer "test" over "[ ... ]".  Never use "[[ ... ]]" unless in a
+   script only meant for bash.
 
  - We do not write the noiseword "function" in front of shell
    functions.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

On Tuesday, July 23, 2024 at 11:25:29 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:

> > $'...' is new in POSIX (2024), and some shells support it in recent
> > versions, while others have had it for decades (bash, zsh, ksh93).

> I will not look at this series futher during the current development
> cycle that is about to conclude, but ...

Thanks. I'm happy to continue whenever others have the bandwidth.

> > +__git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
> > +__git_LF=$'\n' __git_CRLF=$'\r\n'
> > +
> > +if [ $'\101' != A ]; then  # fallback for shells without $'...'
> > +  __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
> > +  __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
> > +  __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
> > +  __git_SOH=${__git_CRLF#??};  __git_CRLF=${__git_CRLF%?}
> > +  __git_LF=${__git_CRLF#?}
> > +fi

> ... I would prefer to see it done without any "fallback".

That's fair. I'll change it.

> $(printf '\r') would work with bash, zsh and ksh93, too, and one
> time assignment to these variables is not going to be performance
> critical.

Generally true, and also mostly non-generally as far as performace
goes, though personally I'd prefer to avoid command substitution in
this context if possible, as even a single one can have non-negligible
impact in (very) hot scripts.

But it would still be very small, and doesn't matter with this file.

> Just forbid use of $'\octal' notation in the coding
> guidelines document, and implement just one variant.

Agreed, and the CodingGuidelines patch LGTM.

However, off the top of my head I wouldn't know how this variant
should look like. This one printf and splitting it later is a bit
meh to be used in every script which needs control literals, but
I also don't have anything cleaner off the top of my head.

> Perhaps we should spell more things out that you wrote in some of
> your proposed log messages more explicitly.  I think these have been
> rules we have followed (grep for them in *.sh files outside
> contrib/) but I did not find mention in the guidelines document.

If I can help with this, let me know how.

> Thanks.

My pleasure.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

 On Wednesday, July 24, 2024 at 05:08:54 AM GMT+3, avih <avihpit@yahoo.com> wrote:
> On Tuesday, July 23, 2024 at 11:25:29 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
> > >
> > > +__git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
> > > +__git_LF=$'\n' __git_CRLF=$'\r\n'
> > > +
> > > +if [ $'\101' != A ]; then  # fallback for shells without $'...'
> > > +  __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
> > > +  __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
> > > +  __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
> > > +  __git_SOH=${__git_CRLF#??};  __git_CRLF=${__git_CRLF%?}
> > > +  __git_LF=${__git_CRLF#?}
> > > +fi
>
> > ... I would prefer to see it done without any "fallback".
> > Just forbid use of $'\octal' notation in the coding
> > guidelines document, and implement just one variant.
>
> ... off the top of my head I wouldn't know how this variant
> should look like. This one printf and splitting it later is a bit
> meh to be used in every script which needs control literals, but
> I also don't have anything cleaner off the top of my head.

I think I misinterpreted the scope of "variant". I thought it meant,
across scripts which need to use $'...', but now I realize it meant
between using $'...' and using the fallback in this patch.

So mainly as a general solution, but also applicable to this patch,
below is my best generalized solution so far, so that scripts don't
have to reinvent the wheel with this "string strip dance" above,
but I'm not too happy with it, mainly due to the gotcha that single
quotes in the value break the world (escape the "eval").

So unless others prefer this solution or have other ideas, I think
it's best to keep the existing "strip dance" which the patch already
has, but make it the only variant instead of being a fallback.

Generalized solution (without namespace-ification):

    # assign_as_fmt NAME=FMT ...  -->  NAME=$(printf FMT) ...
    # - works also if the output ends in newline
    # - NAME must be a valid var name
    # - FMT _must_ not include/output single quotes (escapes eval)
    # - best used like $'..':  x=$'\r\n'  ->  assign_as_fmt x='\r\n'
    #   (which also avoids accidental single quotes, but not '\047')
    assign_as_fmt () {
        # accumulate " NAME='FMT'" from each "NAME=FMT"
        fmt=  # ignore non-locality for now
        while [ "${1+x}" ]; do
            fmt="$fmt ${1%%=*}='${1#*=}'"
            shift
        done
        eval "$(printf "$fmt")"  # FMTs become values, eval'ed
    }

    assign_as_fmt \
        __git_SOH='\1' __git_STX='\2' __git_ESC='\33' \
        __git_LF='\n' __git_CRLF='\r\n'

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

 On Thursday, July 25, 2024 at 02:27:29 PM GMT+3, avih <avihpit@yahoo.com> wrote:
>
> So mainly as a general solution, but also applicable to this patch,
> below is my best generalized solution so far, so that scripts don't
> have to reinvent the wheel with this "string strip dance" above,
> but I'm not too happy with it, mainly due to the gotcha that single
> quotes in the value break the world (escape the "eval").

Pardon the noise.

To summarize the options to replace $'...' portably, like:

    __git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
    __git_LF=$'\n' __git_CRLF=$'\r\n'

The current patch has this, which is not fun and not scalable:

   __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
   __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
   __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
   __git_SOH=${__git_CRLF#??};   __git_CRLF=${__git_CRLF%?}
   __git_LF=${__git_CRLF#?}

If performance is not important, this works (with care about \n):

   __git_LF=$(printf "\nx"); __git_LF=${__git_LF%x}
   __git_STX=$(printf '\1')
   ...

A previous message suggested this, which has a beautiful API,
but it requires an additional non-tiny function, and it also
hides a great risk of escaping "eval":

    assign_as_fmt () {
        # hides the usage of "eval"
        ...
    }

    assign_as_fmt \
        __git_SOH='\1' __git_STX='\2' __git_ESC='\33' \
        __git_LF='\n' __git_CRLF='\r\n'

But then I figured there's another option, which is reasonably
readable, scalable, small without additional functions, but still
requires some care, though the risk is not hidden and easy to avoid:

It's basically what the function does, but without a function:
(double quotes required only if it ends in \n, or for uniformity)

    # doubel-check to ensure the printf output is valid shell input
    eval "$(printf '
        __git_SOH="\1" __git_STX="\2" __git_ESC="\33"
        __git_LF="\n" __git_CRLF="\r\n"
    ')"

I think it strikes the best balance between the options, both
for this patch, and possibly also as a general recomendation.

So unless there are objections or better suggestions, this is
what I currently prefer for this patch.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

>     eval "$(printf '
>         __git_SOH="\1" __git_STX="\2" __git_ESC="\33"
>         __git_LF="\n" __git_CRLF="\r\n"
>     ')"
>
> I think it strikes the best balance between the options, both
> for this patch, and possibly also as a general recomendation.
>
> So unless there are objections or better suggestions, this is
> what I currently prefer for this patch.

Modulo my superstition against \d and \dd, the above does look
very readable and hard to break.

Thanks.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

 On Thursday, July 25, 2024 at 07:19:56 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
> I think you are over-engineering this.  We do not have immediate
> need for such facility to be used by other scripts.  On the other
> hand, we know exactly what git-prompt wants to see available, and
> you already implemented them.

Yes. It was a misunderstanding on my part, and it was overengineered.

But because I initially misinterpreted your statement as
"disallow $'...' at the guidelines, and we need to find one variant
(form) to use", it got me thinking, because I wasn't very happy with
the existing form at the patch either.

And it did eventually lead to a solution we both liked. I'm OK taking
that road, but next time I should probably wait a bit more before
going pulic with half-baked ideas.

> So just losing "make assignment asuming $'blah' works, and then
> reassign based on what printf gave us" and always using the printf
> thing is what we want to see here.

Yes.

> Are you sure that everybody's implementation of printf(1) is happy
> with \d and \dd?  I am an old timer who learnt in a distant past to
> always spell octals as \ddd without omitting any leading 0-digit,
> because some was unhappy.

If I knew who "everybody" is, then maybe.

But "learned in the distant past" does carry weight, as do existing
practices. However, there seem to be almost no cases in non - /t/...
files, and most of them are in git-prompt.sh.

In test scripts though, it's a mixed bag. I think in decreasing order
of popularity:
- Always use \ddd form.
- Allow less than 3 if it begins with 0, like \01, and many \0 .
- Yes, \1 or \4 are fine (there are not many of those).

I'll use the \ddd form you because you prefer it and it does seem the
most popular (I think also outside of git codebase), and even if only
for being bullet-proof against following '0'-'7' chars at the string.

But back to the question of how much I'm sure, then I'm sure there
are exceptions, but I couldn't find one yet.

It works in all the shell-builtin-printf I have access to (~ 20),
as well as gnu /bin/printf. Obviously there are many common ancestors
there (esp. ash and pdksh), but they are still different codebases,
and others don't share code with those, like ksh, bash, Schily, yash.

As a cute data point, this printf statement, copy-pasted into a 1981
BSD 2.11 running on PDP11, prints the exact output as it does today:

    printf 'a="\1" b="\2" c="\33" d="\n" e="\r\n"' | od -c

https://skn.noip.me/pdp11/pdp11.html  ("boot RP1", user root, no pass)

On Thursday, July 25, 2024 at 07:24:08 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
> avih <avihpit@yahoo.com> writes:
> >    eval "$(printf '
> >        __git_SOH="\1" __git_STX="\2" __git_ESC="\33"
> >        __git_LF="\n" __git_CRLF="\r\n"
> >    ')"
>
> Modulo my superstition against \d and \dd, the above does look
> very readable and hard to break.

Thanks.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

> Generalized solution (without namespace-ification):

I think you are over-engineering this.  We do not have immediate
need for such facility to be used by other scripts.  On the other
hand, we know exactly what git-prompt wants to see available, and
you already implemented them.

So just losing "make assignment asuming $'blah' works, and then
reassign based on what printf gave us" and always using the printf
thing is what we want to see here.

>     assign_as_fmt \
>         __git_SOH='\1' __git_STX='\2' __git_ESC='\33' \
>         __git_LF='\n' __git_CRLF='\r\n'

Are you sure that everybody's implementation of printf(1) is happy
with \d and \dd?  I am an old timer who learnt in a distant past to
always spell octals as \ddd without omitting any leading 0-digit,
because some was unhappy.

Thanks.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

 On Tuesday, July 23, 2024 at 11:25:29 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
> > From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
> >
> > $'...' is new in POSIX (2024), and some shells support it in recent
> > versions, while others have had it for decades (bash, zsh, ksh93).
>
> I will not look at this series futher during the current development
> cycle that is about to conclude, but ...

Ping

Reminder: I'll update this part to not-use $'...' and without
fallback, but I'm currently waiting for comments on the other parts
as well before I update this patch.

- avih

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

>  On Tuesday, July 23, 2024 at 11:25:29 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>> > From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
>> >
>> > $'...' is new in POSIX (2024), and some shells support it in recent
>> > versions, while others have had it for decades (bash, zsh, ksh93).
>>
>> I will not look at this series futher during the current development
>> cycle that is about to conclude, but ...
>
> Ping
>
> Reminder: I'll update this part to not-use $'...' and without
> fallback, but I'm currently waiting for comments on the other parts
> as well before I update this patch.

Ping for others.  I do not recall having much other things to say on
the series.

Thanks.

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[avih]

 On Wednesday, August 14, 2024 at 10:32:19 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>> avih <avihpit@yahoo.com> writes:
>>>  On Tuesday, July 23, 2024 at 11:25:29 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>>> I will not look at this series futher during the current development
>>> cycle that is about to conclude, but ...
>>
>> Ping
>>
>> Reminder: I'll update this part to not-use $'...' and without
>> fallback, but I'm currently waiting for comments on the other parts
>> as well before I update this patch.
>
> Ping for others.  I do not recall having much other things to say on
> the series.

Not sure I understand.

Shall I ping the other 7 parts individually?

Or shall I go ahead and post the updated part 6/8 (and rebased
parts 7 and 8 trivially)

Slightly off topic, git-prompt.sh was not modified in master since
I submitted the series, so no need to rebase the series, right?

Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

>>> Ping
>>>
>>> Reminder: I'll update this part to not-use $'...' and without
>>> fallback, but I'm currently waiting for comments on the other parts
>>> as well before I update this patch.
>>
>> Ping for others.  I do not recall having much other things to say on
>> the series.
>
> Not sure I understand.
>
> Shall I ping the other 7 parts individually?

No, I pinged other folks, who are reading git@vger.kernel.org, to
give their reviews, because the prompt script is not exactly my area
of expertise.  I commented on it only because nobody else did, and
because I care about portability while keeping the complexity level
down.  Other aspects of the series are better reviewed by others,
not by me.

If you have updated series, resending the whole series with as v2
(e.g. [PATCH v2 1/8]..[PATCH v2 8/8], if there is no change in the
number of patches) would be good.

THanks.

[PATCH 7/8] git-prompt: ta-da! document usage in other shells

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

With one big exception, git-prompt.sh should now be both almost posix
compliant, and also compatible with most (posix-ish) shells.

That exception is the use of "local" vars in functions, which happens
extensively in the current code, and is not simple to replace with
posix compliant code (but also not impossible).

Luckily, almost all shells support "local" as used by the current
code, with the notable exception of ksh93[u+m], but also the Schily
minimal posix sh (pbosh), and yash in posix mode.

See assessment below that "local" is likely the only blocker in those.

So except mainly ksh93, git-prompt.sh now works in most shells:
- bash, zsh, dash since at least 0.5.8, free/net bsd sh, busybox-ash,
  mksh, openbsd sh, pdksh(!), Schily extended Bourne sh (bosh), yash.

which is quite nice.

As an anecdote, replacing the 1st line in __git_ps1() (local exit=$?)
with these 2 makes it work in all tested shells, even without "local":

  # handles only 0/1 args for simplicity. needs +5 LOC for any $#
  __git_e=$?; local exit="$__git_e" 2>/dev/null ||
    {(eval 'local() { export "$@"; }'; __git_ps1 "$@"); return "$__git_e"; }

Explanation:

  If the shell doesn't have the command "local", define our own
  function "local" which instead does plain (global) assignents.
  Then use __git_ps1 in a subshell to not clober the caller's vars.

  This happens to work because currently there are no name conflicts
  (shadow) at the code, initial value is not assumed (i.e. always
  doing either 'local x=...'  or 'local x;...  x=...'), and assigned
  initial values are quoted (local x="$y"), preventing word split and
  glob expansion (i.e. assignment context is not assumed).

  The last two (always init, quote values) seem to be enough to use
  "local" portably if supported, and otherwise shells indeed differ.

  Uses "eval", else shells with "local" may reject it during parsing.
  We don't need "export", but it's smaller than writing our own loop.

While cute, this approach is not really sustainable because all the
vars become global, which is hard to maintain without conflicts
(but hey, it currently has no conflicts - without even trying...).

However, regardless of being an anecdote, it provides some support to
the assessment that "local" is the only blocker in those shells.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 33 ++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index bbc16417ac9..5787eca28db 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -8,8 +8,8 @@
 # To enable:
 #
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
-#    2) Add the following line to your .bashrc/.zshrc:
-#        source ~/.git-prompt.sh
+#    2) Add the following line to your .bashrc/.zshrc/.profile:
+#        . ~/.git-prompt.sh   # dot path/to/this-file
 #    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
@@ -30,6 +30,8 @@
 #        Optionally, you can supply a third argument with a printf
 #        format string to finetune the output of the branch status
 #
+#    See notes below about compatibility with other shells.
+#
 # The repository status will be displayed only if you are currently in a
 # git repository. The %s token is the placeholder for the shown status.
 #
@@ -106,6 +108,33 @@
 # directory is set up to be ignored by git, then set
 # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
 # repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# Conpatibility with other shells (beyond bash/zsh):
+#
+#    We require posix-ish shell plus "local" support, which is most
+#    shells (even pdksh), but excluding ksh93 (because no "local").
+#
+#    Prompt integration might differ between shells, but the gist is
+#    to load it once on shell init with '. path/to/git-prompt.sh',
+#    set GIT_PS1* vars once as needed, and either place $(__git_ps1..)
+#    inside PS1 once (0/1 args), or, before each prompt is displayed,
+#    call __git_ps1 (2/3 args) which sets PS1 with the status embedded.
+#
+#    Many shells support the 1st method of command substitution,
+#    though some might need to first enable cmd substitution in PS1.
+#
+#    When using colors, each escape sequence is wrapped between byte
+#    values 1 and 2 (control chars SOH, STX, respectively), which are
+#    invisible at the output, but for bash/readline they mark 0-width
+#    strings (SGR color sequences) when calculating the on-screen
+#    prompt width, to maintain correct input editing at the prompt.
+#
+#    Currently there's no support for different markers, so if editing
+#    behaves weird when using colors in __git_ps1, then the solution
+#    is either to disable colors, or, in some shells which only care
+#    about the width of the last prompt line (e.g. busybox-ash),
+#    ensure the git output is not at the last line, maybe like so:
+#      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
 __git_printf_supports_v=
-- 
gitgitgadget

[PATCH 8/8] git-prompt: support custom 0-width PS1 markers

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

When using colors, the shell needs to identify 0-width substrings
in PS1 - such as color escape sequences - when calculating the
on-screen width of the prompt.

Until now, we used the form %F{<color>} in zsh - which it knows is
0-width, or otherwise use standard SGR esc sequences wrapped between
byte values 1 and 2 (SOH, STX) as 0-width start/end markers, which
bash/readline identify as such.

But now that more shells are supported, the standard SGR sequences
typically work, but the SOH/STX markers might not be identified.

This commit adds support for vars GIT_PS1_COLOR_{PRE,POST} which
set custom 0-width markers or disable the markers.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5787eca28db..60df5cb94fe 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -129,11 +129,16 @@
 #    strings (SGR color sequences) when calculating the on-screen
 #    prompt width, to maintain correct input editing at the prompt.
 #
-#    Currently there's no support for different markers, so if editing
-#    behaves weird when using colors in __git_ps1, then the solution
-#    is either to disable colors, or, in some shells which only care
-#    about the width of the last prompt line (e.g. busybox-ash),
-#    ensure the git output is not at the last line, maybe like so:
+#    To replace or disable the 0-width markers, set GIT_PS1_COLOR_PRE
+#    and GIT_PS1_COLOR_POST to other markers, or empty (nul) to not
+#    use markers. For instance, some shells support '\[' and '\]' as
+#    start/end markers in PS1 - when invoking __git_ps1 with 3/4 args,
+#    but it may or may not work in command substitution mode. YMMV.
+#
+#    If the shell doesn't support 0-width markers and editing behaves
+#    incorrectly when using colors in __git_ps1, then, other than
+#    disabling color, it might be solved using multi-line prompt,
+#    where the git status is not at the last line, e.g.:
 #      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
@@ -314,8 +319,8 @@ __git_ps1_colorize_gitstring ()
 		# \001 (SOH) and \002 (STX) are 0-width substring markers
 		# which bash/readline identify while calculating the prompt
 		# on-screen width - to exclude 0-screen-width esc sequences.
-		local c_pre="${__git_SOH}${__git_ESC}["
-		local c_post="m${__git_STX}"
+		local c_pre="${GIT_PS1_COLOR_PRE-$__git_SOH}${__git_ESC}["
+		local c_post="m${GIT_PS1_COLOR_POST-$__git_STX}"
 
 		local c_red="${c_pre}31${c_post}"
 		local c_green="${c_pre}32${c_post}"
-- 
gitgitgadget

Re: [PATCH 0/8] git-prompt: support more shells

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On 2024-07-23 at 19:18:18, Avi Halachmi via GitGitGadget wrote:
> Before this patchset only bash and zsh were supported.
> 
> After this patchset, the following shells work: bash, zsh, dash (since at
> least 0.5.8), free/net bsd sh, busybox-ash, mksh, openbsd sh, pdksh(!),
> Schily extended Bourne sh (bosh), yash.
> 
> The code should now be almost posix-compliant, with one big exception
> ("local" variables in functions) which is not simple to fix.

We explicitly allow `local` in our coding guidelines.  As a side note,
Debian requires it of all shells that can be used as `/bin/sh`.

> Shells which don't work, likely only due to missing "local": ksh93[u+m],
> Schily minimal posix Bourne sh (pbosh), yash-posix-mode.

ksh93u+m is planning on adding local in a future revision.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 0/8] git-prompt: support more shells

[avih]

 On Wednesday, July 24, 2024 at 01:50:16 AM GMT+3, brian m. carlson <sandals@crustytoothpaste.net> wrote:

> We explicitly allow `local` in our coding guidelines.

Yeah. I missed the guidelines initially, but I got to the same
conclusion with git-prompt.sh - to allow only "local" exception.

> As a side note,
> Debian requires it of all shells that can be used as `/bin/sh`.

I wasn't aware of that, but it does makes sense to me.
 
> ksh93u+m is planning on adding local in a future revision.

That's nice. I did try to check whether it's planned, and request if
it wasn't, but I didn't find the future plans (but also didn't try
too hard). Though I think they're still doing bug fixes for the
forseeable future, which is also great. Looking forward to it.

However, while they do have typeset (in non-posix 'function foo()...),
it has syntactic scope and not dynamic like with "local", so it
wouldn't be a trivial mapping to an existing functionality.

"local" is so useful, and with minimal application restrictions it's
already effectively portable with very few exceptions, but ksh93[u+m]
is indeed one of the notable ones.

I think was a missed opportinity that POSIX 2024 didn't include "local"
(I know it was discussed).

It is possible to implement the functionality compliantly and even
with reasonable syntax, no tricks, and very good performance, but it's
not the same as being officially supported.

Re: [PATCH 0/8] git-prompt: support more shells

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

>  On Wednesday, July 24, 2024 at 01:50:16 AM GMT+3, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>
>> We explicitly allow `local` in our coding guidelines.
>
> Yeah. I missed the guidelines initially, but I got to the same
> conclusion with git-prompt.sh - to allow only "local" exception.

It is a bit more nuanced than that, though.  Here is what we say:

 - Even though "local" is not part of POSIX, we make heavy use of it
   in our test suite.  We do not use it in scripted Porcelains, and
   hopefully nobody starts using "local" before all shells that matter
   support it (notably, ksh from AT&T Research does not support it yet).

For the purpose of git-prompt, I think it should be OK (without
"local", it is harder, if not impossible, to clobber end-user's
shell variable namespace with various temporaries we need to use
during prompt computation) to declare that we now support shells
other than bash and zsh as long as they are reasonably POSIX and
support "local" that is dynamic.

> That's nice. I did try to check whether it's planned, and request if
> it wasn't, but I didn't find the future plans (but also didn't try
> too hard). Though I think they're still doing bug fixes for the
> forseeable future, which is also great. Looking forward to it.

Do we know what kind of "local" is ksh93 adding?  The same as their
"typeset" that is not dynamic?  That is so different from what others
do and scripts expect to be all that useful, I am afraid.

Re: [PATCH 0/8] git-prompt: support more shells

[avih]

 On Wednesday, July 24, 2024 at 06:29:12 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>
> It is a bit more nuanced than that, though.  Here is what we say:
>
> - Even though "local" is not part of POSIX, we make heavy use of it
>   in our test suite.  We do not use it in scripted Porcelains, and
>   hopefully nobody starts using "local" before all shells that matter
>   support it (notably, ksh from AT&T Research does not support it yet).
>
> For the purpose of git-prompt, I think it should be OK ...
> to declare that we now support shells
> other than bash and zsh as long as they are reasonably POSIX and
> support "local" that is dynamic.

I have to admit I missed "in our test suite".
Right, so no "local" in Porcelains, but yes in the test suite.

But yes, agreed, because it supports so many more shells.
The commit "git-prompt: ta-da! document.." does document it.

> (without
> "local", it is harder, if not impossible, to clobber end-user's
> shell variable namespace with various temporaries we need to use
> during prompt computation)

It actually is technically possible with git-prompt.sh, with 1 LOC.
See the "anecdote" at end of the same "ta-da!" commit message which
does exactly that. Though for obvious reason we can't really do that.

> Do we know what kind of "local" is ksh93 adding?  The same as their
> "typeset" that is not dynamic?  That is so different from what others
> do and scripts expect to be all that useful, I am afraid.

I would think it has to be similar enough to other shells, or else it
creates a compatibility nightmare IMO. But that's a guess.

Somewhat off topic, so apologies if this shouldn't be here:

As for the Porcelains, I have to assume that it can be unpleasant
to maintain big scripts without "local". Would there be an interest
in adding a facility with the same semantics as "local", but posix
compliant (and also posix-ish shells), for use in Porcelains?

It's not a drop-in replacement, but the syntax is reasonable IMO:

    locals myfunc x y
    _myfunc () {
        echo "$? $1 $2"
        x=1 y=2
        return 33
    }

    x=x; unset y
    (exit 42) || myfunc foo bar
    echo "$? $x ${y-unset}"

Prints:
    42 foo bar
    33 x unset

"locals myfunc x y" creates a wrapper function "myfunc" which saves
the state of $x and $y, calls _myfunc "$@", then restores the state
(and propagates the initial and final $? appropriately). Recursion is
supported, the wrapper doesn't create additional variables, and no
subshells are used at the wrapper (also not at "locals").

The implementation of "locals" is small (10-20 LOC), but we can't
expect scripts to embed it, so it would need to be sourced (dot).

If there is interest in such thing, let me know, and I can submit
a patch (independent of this patchset) to adds such file which
can then be sourced by other scripts in order to use "locals".

Unrelated, and it might not mean much, but I did want to thank you
for maintaining git all those years.

Re: [PATCH 0/8] git-prompt: support more shells

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> For the purpose of git-prompt, I think it should be OK (without
> "local", it is harder, if not impossible, to clobber end-user's
> shell variable namespace with various temporaries we need to use
> during prompt computation) to declare that we now support shells
> other than bash and zsh as long as they are reasonably POSIX and
> support "local" that is dynamic.

"to clobber" -> "to avoid clobbering", sorry for the noise.

[PATCH v2 0/8] git-prompt: support more shells v2

[Avi Halachmi via GitGitGadget]

This addresses review comment on part 6/8 (git-prompt: add fallback for
shells without $'...') which requested to use one form for all shells
instead $'...' where supported and a fallback otherwise.

Parts 7/8 and 8/8 are rebased trivially on top of updated part 6/8.

Using the GitGitGadget github interface, so I don't know whether it will
send only the 3 updated patches, or all 8 as v2.

Potential followups which this series does not address:

 * A comment on part 6/8 suggested to add to CodingGuidelines some of the
   guidelines in the commit messages, without being specific, likely
   referring to part 5/8 (git-prompt: add some missing quotes).

 * The same comment to 6/8 posted a suggested patch to CodingGuidelines to
   disallow bashism [[...]], and disallow $'...' - which is comliant (POSIX
   2024) but not supported in all shells.

Avi Halachmi (:avih) (8):
  git-prompt: use here-doc instead of here-string
  git-prompt: fix uninitialized variable
  git-prompt: don't use shell arrays
  git-prompt: replace [[...]] with standard code
  git-prompt: add some missing quotes
  git-prompt: don't use shell $'...'
  git-prompt: ta-da! document usage in other shells
  git-prompt: support custom 0-width PS1 markers

 contrib/completion/git-prompt.sh | 191 ++++++++++++++++++++-----------
 1 file changed, 126 insertions(+), 65 deletions(-)


base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1750%2Favih%2Fprompt-compat-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1750/avih/prompt-compat-v2
Pull-Request: https://github.com/git/git/pull/1750

Range-diff vs v1:

 1:  9ce5ddadf0b = 1:  9ce5ddadf0b git-prompt: use here-doc instead of here-string
 2:  680ecb52404 = 2:  680ecb52404 git-prompt: fix uninitialized variable
 3:  7e994eae7bc = 3:  7e994eae7bc git-prompt: don't use shell arrays
 4:  232340902a1 = 4:  232340902a1 git-prompt: replace [[...]] with standard code
 5:  4f77b7eb7f1 = 5:  4f77b7eb7f1 git-prompt: add some missing quotes
 6:  1c1b58e20ca ! 6:  363b7015763 git-prompt: add fallback for shells without $'...'
     @@ Metadata
      Author: Avi Halachmi (:avih) <avihpit@yahoo.com>
      
       ## Commit message ##
     -    git-prompt: add fallback for shells without $'...'
     +    git-prompt: don't use shell $'...'
      
          $'...' is new in POSIX (2024), and some shells support it in recent
          versions, while others have had it for decades (bash, zsh, ksh93).
      
          However, there are still enough shells which don't support it, and
     -    it's cheap to provide a fallback for them, so let's do that instead
     -    of dismissing it as "it's compliant".
     +    it's cheap to use an alternative form which works in all shells,
     +    so let's do that instead of dismissing it as "it's compliant".
     +
     +    It was agreed to use one form rather than $'...' where supported and
     +    fallback otherwise.
      
          shells where $'...' works:
          - bash, zsh, ksh93, mksh, busybox-ash, dash master, free/net bsd sh.
     @@ contrib/completion/git-prompt.sh
       __git_printf_supports_v=
       printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
       
     -+__git_SOH=$'\1' __git_STX=$'\2' __git_ESC=$'\33'
     -+__git_LF=$'\n' __git_CRLF=$'\r\n'
     -+
     -+if [ $'\101' != A ]; then  # fallback for shells without $'...'
     -+   __git_CRLF=$(printf "\r\n\1\2\33")  # CR LF SOH STX ESC
     -+   __git_ESC=${__git_CRLF#????}; __git_CRLF=${__git_CRLF%?}
     -+   __git_STX=${__git_CRLF#???};  __git_CRLF=${__git_CRLF%?}
     -+   __git_SOH=${__git_CRLF#??};   __git_CRLF=${__git_CRLF%?}
     -+   __git_LF=${__git_CRLF#?}
     -+fi
     ++# like __git_SOH=$'\001' etc but works also in shells without $'...'
     ++eval "$(printf '
     ++	__git_SOH="\001" __git_STX="\002" __git_ESC="\033"
     ++	__git_LF="\n" __git_CRLF="\r\n"
     ++')"
      +
       # stores the divergence from upstream in $p
       # used by GIT_PS1_SHOWUPSTREAM
 7:  4a086ffc360 = 7:  4aa75cdb5dd git-prompt: ta-da! document usage in other shells
 8:  f241c3ae1e4 = 8:  e71ddcd2232 git-prompt: support custom 0-width PS1 markers

-- 
gitgitgadget

[PATCH v2 1/8] git-prompt: use here-doc instead of here-string

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Here-documend is standard, and works in all shells.

Both here-string and here-doc add final newline, which is important
in this case, because $output is without final newline, but we do
want "read" to succeed on the last line as well.

Shells which support here-string:
- bash, zsh, mksh, ksh93, yash (non-posix-mode).

shells which don't, and got fixed:
- ash-derivatives (dash, free/net bsd sh, busybox-ash).
- pdksh, openbsd sh.
- All Schily Bourne shell variants.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5330e769a72..ebf2e30d684 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -137,7 +137,9 @@ __git_ps1_show_upstream ()
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
-	done <<< "$output"
+	done <<-OUTPUT
+		$output
+	OUTPUT
 
 	# parse configuration values
 	local option
-- 
gitgitgadget

Re: [PATCH v2 1/8] git-prompt: use here-doc instead of here-string

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 01:14:06PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
> 
> Here-documend is standard, and works in all shells.
> 
> Both here-string and here-doc add final newline, which is important
> in this case, because $output is without final newline, but we do
> want "read" to succeed on the last line as well.
> 
> Shells which support here-string:
> - bash, zsh, mksh, ksh93, yash (non-posix-mode).
> 
> shells which don't, and got fixed:
> - ash-derivatives (dash, free/net bsd sh, busybox-ash).
> - pdksh, openbsd sh.
> - All Schily Bourne shell variants.
> 
> Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
> ---
>  contrib/completion/git-prompt.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 5330e769a72..ebf2e30d684 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -137,7 +137,9 @@ __git_ps1_show_upstream ()
>  			upstream_type=svn+git # default upstream type is SVN if available, else git
>  			;;
>  		esac
> -	done <<< "$output"
> +	done <<-OUTPUT
> +		$output
> +	OUTPUT

I was a bit sceptical at first whether this produces the correct output,
because I wasn't sure whether the first line might be indented while the
others wouldn't be. And that would only happen if we indented with
spaces, but when indenting with a tab it seems to work as expected.

Patrick

>  	# parse configuration values
>  	local option
> -- 
> gitgitgadget
> 
> 

Re: [PATCH v2 1/8] git-prompt: use here-doc instead of here-string

[avih]

 On Friday, August 16, 2024 at 11:50:12 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> On Thu, Aug 15, 2024 at 01:14:06PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
>> -    done <<< "$output"
>> +    done <<-OUTPUT
>> +        $output
>> +    OUTPUT
>
> I was a bit sceptical at first whether this produces the correct output,
> because I wasn't sure whether the first line might be indented while the
> others wouldn't be. And that would only happen if we indented with
> spaces, but when indenting with a tab it seems to work as expected.

That's what the "-" does in "<<-". It strips leading input tab chars
at the content and the last line, and was specified as such since the
first POSIX release in 1994:

  If the redirection symbol is <<−, all leading tab characters will
  be stripped from input lines and the line containing the trailing
  delimiter.

Re: [PATCH v2 1/8] git-prompt: use here-doc instead of here-string

[Patrick Steinhardt]

On Fri, Aug 16, 2024 at 09:37:15AM +0000, avih wrote:
>  On Friday, August 16, 2024 at 11:50:12 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> > On Thu, Aug 15, 2024 at 01:14:06PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> >> -    done <<< "$output"
> >> +    done <<-OUTPUT
> >> +        $output
> >> +    OUTPUT
> >
> > I was a bit sceptical at first whether this produces the correct output,
> > because I wasn't sure whether the first line might be indented while the
> > others wouldn't be. And that would only happen if we indented with
> > spaces, but when indenting with a tab it seems to work as expected.
> 
> That's what the "-" does in "<<-". It strips leading input tab chars
> at the content and the last line, and was specified as such since the
> first POSIX release in 1994:
> 
>   If the redirection symbol is <<−, all leading tab characters will
>   be stripped from input lines and the line containing the trailing
>   delimiter.

Oh, I know what `<<-` does. I just wasn't sure how it would behave when
"$output" expands to a multi-line string, where subsequent expanded
lines might or might not be indented.

Patrick

[PATCH v2 2/8] git-prompt: fix uninitialized variable

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

First use is in the form:  local var; ...; var=$var$whatever...

If the variable was unset (as bash and others do after "local x"),
then it would error if set -u is in effect.

Also, many shells inherit the existing value after "local var"
without init, but in this case it's unlikely to have a prior value.

Now we initialize it.

(local var= is enough, but local var="" is the custom in this file)

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index ebf2e30d684..4cc2cf91bb6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,7 +116,7 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern count n
+	local svn_remote svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
-- 
gitgitgadget

[PATCH v2 3/8] git-prompt: don't use shell arrays

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Arrays only existed in the svn-upstream code, used to:
- Keep a list of svn remotes.
- Convert commit msg to array of words, extract the 2nd-to-last word.

Except bash/zsh, nearly all shells failed load on syntax errors here.

Now:
- The svn remotes are a list of newline-terminated values.
- The 2nd-to-last word is extracted using standard shell substrings.
- All shells can digest the svn-upstream code.

While using shell field splitting to extract the word is simple, and
doesn't even need non-standard code, e.g. set -- $(git log -1 ...),
it would have the same issues as the old array code: it depends on IFS
which we don't control, and it's subject to glob-expansion, e.g. if
the message happens to include * or **/* (as this commit message just
did), then the array could get huge. This was not great.

Now it uses standard shell substrings, and we know the exact delimiter
to expect, because it's the match from our grep just one line earlier.

The new word extraction code also fixes svn-upstream in zsh, because
previously it used arr[len-2], but because in zsh, unlike bash, array
subscripts are 1-based, it incorrectly extracted the 3rd-to-last word.
symptom: missing upstream status in a git-svn repo: u=, u+N-M, etc.

The breakage in zsh is surprising, because it was last touched by
  commit d0583da838 (prompt: fix show upstream with svn and zsh),
claiming to fix exactly that. However, it only mentions syntax fixes.
It's unclear if behavior was fixed too. But it was broken, now fixed.

Note LF=$'\n' and then using $LF instead of $'\n' few times.
A future commit will add fallback for shells without $'...', so this
would be the only line to touch instead of replacing every $'\n' .

Shells which could run the previous array code:
- bash

Shells which have arrays but were broken anyway:
- zsh: 1-based subscript
- ksh93: no "local" (the new code can't fix this part...)
- mksh, openbsd sh, pdksh: failed load on syntax error: "for ((...))".

More shells which Failed to load due to syntax error:
- dash, free/net bsd sh, busybox-ash, Schily Bourne shell, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 48 ++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4cc2cf91bb6..75c3a813fda 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,10 +116,10 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern="" count n
+	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
+	local LF=$'\n'
 
-	svn_remote=()
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
 	while read -r key value; do
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
+			svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
 			svn_url_pattern="$svn_url_pattern\\|$value"
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
@@ -156,25 +156,37 @@ __git_ps1_show_upstream ()
 	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)
-		local -a svn_upstream
-		svn_upstream=($(git log --first-parent -1 \
-					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
-		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-			svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
-			svn_upstream=${svn_upstream%@*}
-			local n_stop="${#svn_remote[@]}"
-			for ((n=1; n <= n_stop; n++)); do
-				svn_upstream=${svn_upstream#${svn_remote[$n]}}
-			done
+		# successful svn-upstream resolution:
+		# - get the list of configured svn-remotes ($svn_remotes set above)
+		# - get the last commit which seems from one of our svn-remotes
+		# - confirm that it is from one of the svn-remotes
+		# - use $GIT_SVN_ID if set, else "git-svn"
 
-			if [[ -z "$svn_upstream" ]]; then
+		# get upstream from "git-svn-id: UPSTRM@N HASH" in a commit message
+		# (git-svn uses essentially the same procedure internally)
+		local svn_upstream="$(
+			git log --first-parent -1 \
+				--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null
+		)"
+
+		if [ -n "$svn_upstream" ]; then
+			# extract the URI, assuming --grep matched the last line
+			svn_upstream=${svn_upstream##*$LF}  # last line
+			svn_upstream=${svn_upstream#*: }    # UPSTRM@N HASH
+			svn_upstream=${svn_upstream%@*}     # UPSTRM
+
+			case ${LF}${svn_remotes} in
+			*"${LF}${svn_upstream}${LF}"*)
+				# grep indeed matched the last line - it's our remote
 				# default branch name for checkouts with no layout:
 				upstream_type=${GIT_SVN_ID:-git-svn}
-			else
+				;;
+			*)
+				# the commit message includes one of our remotes, but
+				# it's not at the last line. is $svn_upstream junk?
 				upstream_type=${svn_upstream#/}
-			fi
+				;;
+			esac
 		elif [[ "svn+git" = "$upstream_type" ]]; then
 			upstream_type="@{upstream}"
 		fi
-- 
gitgitgadget

Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 4cc2cf91bb6..75c3a813fda 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -116,10 +116,10 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
>  __git_ps1_show_upstream ()
>  {
>  	local key value
> -	local svn_remote svn_url_pattern="" count n
> +	local svn_remotes="" svn_url_pattern="" count n
>  	local upstream_type=git legacy="" verbose="" name=""
> +	local LF=$'\n'
>  
> -	svn_remote=()
>  	# get some config options from git-config
>  	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
>  	while read -r key value; do
> @@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
>  			fi
>  			;;
>  		svn-remote.*.url)
> -			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
> +			svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...

I was wondering whether this is something we want to quote, mostly
because I still have the failures of dash in mind when assigning values
with spaces to a `local` variable without quoting. I do not know whether
the same issues also apply to non-local variables though, probably not.

Patrick

Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

[avih]

 On Friday, August 16, 2024 at 11:50:14 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
>>
>> -            svn_remote[$((${#svn_remote[@]} + 1))]="$value"
>> +            svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
>
>
> I was wondering whether this is something we want to quote, mostly
> because I still have the failures of dash in mind when assigning values
> with spaces to a `local` variable without quoting. I do not know whether
> the same issues also apply to non-local variables though, probably not.

IFS field splitting and glob expansion strictly never happen and never
happened at the assignment part of a "simple command", since the first
version of POSIX in 1994, so quotes are not needed to avoid that.

See my guidelines at the commit message of part 5/8 (git-prompt: add
some missing quotes), and this always works: a=$b$(foo bar)${x##baz}

However, this does not answer the question of whether we want to use
quotes in assignments.

My general take is to use quotes only when required, and if one is
not sure, then use quotes, or read the spec to be sure, but do keep
in mind that some older shells are not always fully compliant with
the latest POSIX spec. But unquoted assignment is ubiquitous.

I'd say to not quote in assignments, except to avoid tilde expansion
(which can only happen with unquoted literal tilde at the input, but
not after expansion or substitution).

But if there's a strong precedence or preference to use quotes in
assignment, then I can change it.

avih

Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

[Patrick Steinhardt]

On Fri, Aug 16, 2024 at 09:53:36AM +0000, avih wrote:
>  On Friday, August 16, 2024 at 11:50:14 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> > On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> >>
> >> -            svn_remote[$((${#svn_remote[@]} + 1))]="$value"
> >> +            svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
> >
> >
> > I was wondering whether this is something we want to quote, mostly
> > because I still have the failures of dash in mind when assigning values
> > with spaces to a `local` variable without quoting. I do not know whether
> > the same issues also apply to non-local variables though, probably not.
> 
> IFS field splitting and glob expansion strictly never happen and never
> happened at the assignment part of a "simple command", since the first
> version of POSIX in 1994, so quotes are not needed to avoid that.

That's the theory, yes. But as said, we did hit bugs in similar areas in
dash where that wasn't properly honored, as Junio also pointed out on a
later patch. But that was in non-POSIX area anyway, as to the best of my
knowledge it only happens with `local` assignments.

Patrick

Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

[avih]

 On Friday, August 16, 2024 at 01:52:11 PM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
>On Fri, Aug 16, 2024 at 09:53:36AM +0000, avih wrote:
>>  On Friday, August 16, 2024 at 11:50:14 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
>> > On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
>> >>
>> >> -            svn_remote[$((${#svn_remote[@]} + 1))]="$value"
>> >> +            svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
>> >
>> >
>> > I was wondering whether this is something we want to quote, mostly
>> > because I still have the failures of dash in mind when assigning values
>> > with spaces to a `local` variable without quoting. I do not know whether
>> > the same issues also apply to non-local variables though, probably not.
>>
>> IFS field splitting and glob expansion strictly never happen and never
>> happened at the assignment part of a "simple command", since the first
>> version of POSIX in 1994, so quotes are not needed to avoid that.
>
> That's the theory, yes. But as said, we did hit bugs in similar areas in
> dash where that wasn't properly honored, as Junio also pointed out on a
> later patch. But that was in non-POSIX area anyway, as to the best of my
> knowledge it only happens with `local` assignments.

Yes. "local" is special, and not only because it's not POSIX.

The difference with "local" is that it takes assignment as arguments.

A "simple command" (posix term) is composed of optional assignment[s]
and optional command (and arguments).

The assigments part is never IFS-split or glob-expanded, while the
command and arguments part is (in words which include unquoted
expansion or substitution) and therefore needs quotes, e.g.:

foo=$x bar=$y echo a="$b" c="$d"

There are other commands (beyond "local") which take assignment[s]
as arguments, like "export", "readonly" and "command".

Before posix 2024, these commands also required quoting of the
arguments-assignments - just like "local" needed in dash.

But posix 2024 introduced the concept of a "declaration utility"
(which takes assignments as arguments, like export, readonly, etc),
and the concept of "assignment context" where IFS-split and glob
expansion don't happen - like the assignment part of a simple
command, but now also in the assignment arguments of declaration
utilities.

And indeed, new versions of shells now don't need quotes in export
etc, and shells now make "local" a declaration utility which
doesn't need quotes of the assignment args, including in dash.

However, the reason we do use quotes in local, export, etc, is
because many instances of shells which don't yet (or will ever)
support it still exist, so we quote for compatibility with those,
but still it's only needed in assignments which are arguments to
commands - not in the assignment part of a simple command.

I've also updated the wording a bit of my guidelines in part 5/8,
and I'll include it at the commit message of 5/8 v3.

Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

[Patrick Steinhardt]

On Fri, Aug 16, 2024 at 11:35:33AM +0000, avih wrote:
>  On Friday, August 16, 2024 at 01:52:11 PM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> >On Fri, Aug 16, 2024 at 09:53:36AM +0000, avih wrote:
> >>  On Friday, August 16, 2024 at 11:50:14 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> >> > On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> >> >>
> >> >> -            svn_remote[$((${#svn_remote[@]} + 1))]="$value"
> >> >> +            svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
> >> >
> >> >
> >> > I was wondering whether this is something we want to quote, mostly
> >> > because I still have the failures of dash in mind when assigning values
> >> > with spaces to a `local` variable without quoting. I do not know whether
> >> > the same issues also apply to non-local variables though, probably not.
> >>
> >> IFS field splitting and glob expansion strictly never happen and never
> >> happened at the assignment part of a "simple command", since the first
> >> version of POSIX in 1994, so quotes are not needed to avoid that.
> >
> > That's the theory, yes. But as said, we did hit bugs in similar areas in
> > dash where that wasn't properly honored, as Junio also pointed out on a
> > later patch. But that was in non-POSIX area anyway, as to the best of my
> > knowledge it only happens with `local` assignments.
> 
> Yes. "local" is special, and not only because it's not POSIX.
> 
> The difference with "local" is that it takes assignment as arguments.
> 
> A "simple command" (posix term) is composed of optional assignment[s]
> and optional command (and arguments).
> 
> The assigments part is never IFS-split or glob-expanded, while the
> command and arguments part is (in words which include unquoted
> expansion or substitution) and therefore needs quotes, e.g.:
> 
> foo=$x bar=$y echo a="$b" c="$d"
> 
> There are other commands (beyond "local") which take assignment[s]
> as arguments, like "export", "readonly" and "command".
> 
> Before posix 2024, these commands also required quoting of the
> arguments-assignments - just like "local" needed in dash.
> 
> But posix 2024 introduced the concept of a "declaration utility"
> (which takes assignments as arguments, like export, readonly, etc),
> and the concept of "assignment context" where IFS-split and glob
> expansion don't happen - like the assignment part of a simple
> command, but now also in the assignment arguments of declaration
> utilities.
> 
> And indeed, new versions of shells now don't need quotes in export
> etc, and shells now make "local" a declaration utility which
> doesn't need quotes of the assignment args, including in dash.
> 
> However, the reason we do use quotes in local, export, etc, is
> because many instances of shells which don't yet (or will ever)
> support it still exist, so we quote for compatibility with those,
> but still it's only needed in assignments which are arguments to
> commands - not in the assignment part of a simple command.
> 
> I've also updated the wording a bit of my guidelines in part 5/8,
> and I'll include it at the commit message of 5/8 v3.

Great, thanks for your thorough explanations!

Patrick

[PATCH v2 4/8] git-prompt: replace [[...]] with standard code

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The existing [[...]] tests were either already valid as standard [...]
tests, or only required minimal retouch:

Notes:

- [[...]] doesn't do field splitting and glob expansion, so $var
  or $(cmd...) don't need quoting, but [... does need quotes.

- [[ X == Y ]] when Y is a string is same as [ X = Y ], but if Y is
  a pattern, then we need:  case X in Y)... ; esac  .

- [[ ... && ... ]] was replaced with [ ... ] && [ ... ] .

- [[ -o <zsh-option> ]] requires [[...]], so put it in "eval" and only
  eval it in zsh, so other shells would not abort on syntax error
  (posix says [[ has unspecified results, shells allowed to reject it)

- ((x++)) was changed into x=$((x+1))  (yeah, not [[...]] ...)

Shells which accepted the previous forms:
- bash, zsh, ksh93, mksh, openbsd sh, pdksh.

Shells which didn't, and now can process it:
- dash, free/net bsd sh, busybox-ash, Schily Bourne sh, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 75c3a813fda..4781261f868 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -126,7 +126,7 @@ __git_ps1_show_upstream ()
 		case "$key" in
 		bash.showupstream)
 			GIT_PS1_SHOWUPSTREAM="$value"
-			if [[ -z "${GIT_PS1_SHOWUPSTREAM}" ]]; then
+			if [ -z "${GIT_PS1_SHOWUPSTREAM}" ]; then
 				p=""
 				return
 			fi
@@ -187,14 +187,14 @@ __git_ps1_show_upstream ()
 				upstream_type=${svn_upstream#/}
 				;;
 			esac
-		elif [[ "svn+git" = "$upstream_type" ]]; then
+		elif [ "svn+git" = "$upstream_type" ]; then
 			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
 
 	# Find how many commits we are ahead/behind our upstream
-	if [[ -z "$legacy" ]]; then
+	if [ -z "$legacy" ]; then
 		count="$(git rev-list --count --left-right \
 				"$upstream_type"...HEAD 2>/dev/null)"
 	else
@@ -206,8 +206,8 @@ __git_ps1_show_upstream ()
 			for commit in $commits
 			do
 				case "$commit" in
-				"<"*) ((behind++)) ;;
-				*)    ((ahead++))  ;;
+				"<"*) behind=$((behind+1)) ;;
+				*)    ahead=$((ahead+1))   ;;
 				esac
 			done
 			count="$behind	$ahead"
@@ -217,7 +217,7 @@ __git_ps1_show_upstream ()
 	fi
 
 	# calculate the result
-	if [[ -z "$verbose" ]]; then
+	if [ -z "$verbose" ]; then
 		case "$count" in
 		"") # no upstream
 			p="" ;;
@@ -243,7 +243,7 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
-		if [[ -n "$count" && -n "$name" ]]; then
+		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
@@ -265,7 +265,7 @@ __git_ps1_show_upstream ()
 # their own color.
 __git_ps1_colorize_gitstring ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
+	if [ -n "${ZSH_VERSION-}" ]; then
 		local c_red='%F{red}'
 		local c_green='%F{green}'
 		local c_lblue='%F{blue}'
@@ -417,7 +417,7 @@ __git_ps1 ()
 	# incorrect.)
 	#
 	local ps1_expanded=yes
-	[ -z "${ZSH_VERSION-}" ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+	[ -z "${ZSH_VERSION-}" ] || eval '[[ -o PROMPT_SUBST ]]' || ps1_expanded=no
 	[ -z "${BASH_VERSION-}" ] || shopt -q promptvars || ps1_expanded=no
 
 	local repo_info rev_parse_exit_code
@@ -502,11 +502,13 @@ __git_ps1 ()
 					return $exit
 				fi
 
-				if [[ $head == "ref: "* ]]; then
+				case $head in
+				"ref: "*)
 					head="${head#ref: }"
-				else
+					;;
+				*)
 					head=""
-				fi
+				esac
 				;;
 			*)
 				head="$(git symbolic-ref HEAD 2>/dev/null)"
@@ -542,8 +544,8 @@ __git_ps1 ()
 	fi
 
 	local conflict="" # state indicator for unresolved conflicts
-	if [[ "${GIT_PS1_SHOWCONFLICTSTATE-}" == "yes" ]] &&
-	   [[ $(git ls-files --unmerged 2>/dev/null) ]]; then
+	if [ "${GIT_PS1_SHOWCONFLICTSTATE-}" = "yes" ] &&
+	   [ "$(git ls-files --unmerged 2>/dev/null)" ]; then
 		conflict="|CONFLICT"
 	fi
 
-- 
gitgitgadget

Re: [PATCH v2 4/8] git-prompt: replace [[...]] with standard code

[Junio C Hamano]

"Avi Halachmi (:avih) via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
>
> The existing [[...]] tests were either already valid as standard [...]
> tests, or only required minimal retouch:

FWIW, our local coding guidelines to spell these with "test"
(without closing "]"), but this change certainly is a good first
step to get rid of non-portable "[[ ... ]]" construct.

Thanks.

Re: [PATCH v2 4/8] git-prompt: replace [[...]] with standard code

[avih]

 On Thursday, August 15, 2024 at 07:27:12 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>> From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>
>>
>> The existing [[...]] tests were either already valid as standard [...]
>> tests, or only required minimal retouch:
>
> FWIW, our local coding guidelines to spell these with "test"
> (without closing "]"), but this change certainly is a good first
> step to get rid of non-portable "[[ ... ]]" construct.

Right. I did see that, though only after I wrote the patch.

FWIW, the common form in this file was "[" (46 instances),
then "[[" (13 instances), and finally "test" (3 instances).

So I'd still think changing "[[" forms into "[" is the better choice
for this file in a compatibility-focused change, as it leaves the
file in a mostly consistent usage of "[" throughout.

There can come later another change to tighten adherence to the
guidelines.

But if you want to revise this commit and use "test" instead of "[[",
just let me know and I'll do that. I'd be fine with that.

In such case, should we also change the existing "[" at the file
to "test"? (in a new commit?)

Re: [PATCH v2 4/8] git-prompt: replace [[...]] with standard code

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

> FWIW, the common form in this file was "[" (46 instances),
> then "[[" (13 instances), and finally "test" (3 instances).

Yes, that came from the fact that this file has historically been
considered bash-only and our Bourne shell coding guidelines do not
apply.  

> So I'd still think changing "[[" forms into "[" is the better choice
> for this file in a compatibility-focused change, as it leaves the
> file in a mostly consistent usage of "[" throughout.

Absolutely.  We are in agreement (I said this is a good first step).
I do think making it more consistent after the dust settles (read:
not before the series graduates to 'master') would be a good idea,
though.

[PATCH v2 5/8] git-prompt: add some missing quotes

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The issues which this commit fixes are unlikely to be broken
in real life, but the fixes improve correctness, and would prevent
bugs in some uncommon cases, such as weird IFS values.

Listing some portability guideline here for future reference.

I'm leaving it to someone else to decide whether to include
it in the file itself, place is as a new file, or not.

---------

The command "local" is non standard, but is allowed in this file:
- Quote initialization if it can expand (local x="$y"). See below.
- Don't assume initial value after "local x". Either initialize it
  (local x=..), or set before first use (local x;.. x=..; <use $x>).
  (between shells, "local x" can unset x, or inherit it, or do x= )

Other non-standard features beyond "local" are to be avoided.

Use the standard "test" - [...] instead of non-standard [[...]] .

--------

Quotes (some portability things, but mainly general correctness):

Quotes prevent tilde-expansion of some unquoted literal tildes (~).
If the expansion is undesirable, quotes would ensure that.
  Tilds expanded: a=~user:~/ ;  echo ~user ~/dir
  not expanded:   t="~"; a=${t}user  b=\~foo~;  echo "~user" $t/dir

But the main reason for quoting is to prevent IFS field splitting
(which also coalesces IFS chars) and glob expansion after parameter
expansion or command substitution.

In _command-arguments_, expanded/substituted values must be quoted:
  Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
  Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)

Still in _agumemts_, no need to quote non-expandable values:
  Good:                 local x=   y=yes;   echo OK
  OK, but not required: local x="" y="yes"; echo "OK"
But completely empty (NULL) arguments must be quoted:
  foo ""   is not the same as:   foo

Assignments in simple commands - with or without an actual command,
don't need quoting becase there's no IFS split or glob expansion:
  Good:   s=* a=$b c=$(cmd...)${x# foo }${y-   } [cmd ...]
  It's also OK to use double quotes, but not required.

This behavior (no IFS/glob) is called "assignment context", and
"local" does not behave with assignment context in some shells,
hence we require quotes when using "local" - for compatibility.

First value in 'case...' doesn't IFS-split/glob, doesn't need quotes:
  Good:       case  * $foo $(cmd...)  in ... ; esac
  identical:  case "* $foo $(cmd...)" in ... ; esac

Nested quotes in command substitution are fine, often necessary:
  Good: echo "$(foo... "$x" "$(bar ...)")"

Nested quotes in substring ops are legal, and sometimes needed
to prevent interpretation as a pattern, but not the most readable:
  Legal:  foo "${x#*"$y" }"

Nested quotes in "maybe other value" subst are invalid, unnecessary:
  Good:  local x="${y- }";   foo "${z:+ $a }"
  Bad:   local x="${y-" "}"; foo "${z:+" $a "}"
Outer/inner quotes in "maybe other value" have different use cases:
  "${x-$y}"  always one quoted arg: "$x" if x is set, else "$y".
  ${x+"$x"}  one quoted arg "$x" if x is set, else no arg at all.
  Unquoted $x is similar to the second case, but it would get split
  into few arguments if it includes any of the IFS chars.

Assignments don't need the outer quotes, and the braces delimit the
value, so nested quotes can be avoided, for readability:
  a=$(foo "$x")  a=${x#*"$y" }  c=${y- };  bar "$a" "$b" "$c"

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.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 4781261f868..5d7f236fe48 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -246,7 +246,7 @@ __git_ps1_show_upstream ()
 		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
+			if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
 				upstream="$upstream ${__git_ps1_upstream_name}"
@@ -278,12 +278,12 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue=$'\001\e[1;34m\002'
 		local c_clear=$'\001\e[0m\002'
 	fi
-	local bad_color=$c_red
-	local ok_color=$c_green
+	local bad_color="$c_red"
+	local ok_color="$c_green"
 	local flags_color="$c_lblue"
 
 	local branch_color=""
-	if [ $detached = no ]; then
+	if [ "$detached" = no ]; then
 		branch_color="$ok_color"
 	else
 		branch_color="$bad_color"
@@ -360,7 +360,7 @@ __git_sequencer_status ()
 __git_ps1 ()
 {
 	# preserve exit status
-	local exit=$?
+	local exit="$?"
 	local pcmode=no
 	local detached=no
 	local ps1pc_start='\u@\h:\w '
@@ -379,7 +379,7 @@ __git_ps1 ()
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
-		*)	return $exit
+		*)	return "$exit"
 		;;
 	esac
 
@@ -427,7 +427,7 @@ __git_ps1 ()
 	rev_parse_exit_code="$?"
 
 	if [ -z "$repo_info" ]; then
-		return $exit
+		return "$exit"
 	fi
 
 	local short_sha=""
@@ -449,7 +449,7 @@ __git_ps1 ()
 	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
 	   git check-ignore -q .
 	then
-		return $exit
+		return "$exit"
 	fi
 
 	local sparse=""
@@ -499,7 +499,7 @@ __git_ps1 ()
 			case "$ref_format" in
 			files)
 				if ! __git_eread "$g/HEAD" head; then
-					return $exit
+					return "$exit"
 				fi
 
 				case $head in
@@ -597,10 +597,10 @@ __git_ps1 ()
 		fi
 	fi
 
-	local z="${GIT_PS1_STATESEPARATOR-" "}"
+	local z="${GIT_PS1_STATESEPARATOR- }"
 
 	b=${b##refs/heads/}
-	if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
+	if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 		__git_ps1_branch_name=$b
 		b="\${__git_ps1_branch_name}"
 	fi
@@ -612,7 +612,7 @@ __git_ps1 ()
 	local f="$h$w$i$s$u$p"
 	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}${conflict}"
 
-	if [ $pcmode = yes ]; then
+	if [ "$pcmode" = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
 			gitstring=$(printf -- "$printf_format" "$gitstring")
 		else
@@ -623,5 +623,5 @@ __git_ps1 ()
 		printf -- "$printf_format" "$gitstring"
 	fi
 
-	return $exit
+	return "$exit"
 }
-- 
gitgitgadget

Re: [PATCH v2 5/8] git-prompt: add some missing quotes

[Junio C Hamano]

"Avi Halachmi (:avih) via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> In _command-arguments_, expanded/substituted values must be quoted:
>   Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
>   Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)
>
> Still in _agumemts_, no need to quote non-expandable values:

arguments.

> -	local bad_color=$c_red
> -	local ok_color=$c_green
> +	local bad_color="$c_red"
> +	local ok_color="$c_green"
>  	local flags_color="$c_lblue"

Good.  I think we in the past was burned by some shells that want to
see these assignments with "local" always quoted.

>  	# preserve exit status
> -	local exit=$?
> +	local exit="$?"
>  	local pcmode=no

Well no matter what value $? has, it by definition has a few digits
without any $IFS funnies.  Does this really matter?  I'd imagine
that we would prefer to treat "$?" exactly the same way as "no".

> @@ -379,7 +379,7 @@ __git_ps1 ()
>  		;;
>  		0|1)	printf_format="${1:-$printf_format}"
>  		;;
> -		*)	return $exit
> +		*)	return "$exit"
>  		;;

Likewise.

Re: [PATCH v2 5/8] git-prompt: add some missing quotes

[avih]

 On Thursday, August 15, 2024 at 07:36:43 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>> > Still in _agumemts_, no need to quote non-expandable values:
>>
> arguments.

Thanks. Will fix in v3 (after more comments unless asked otherwise).

>> -    local bad_color=$c_red
>> +    local bad_color="$c_red"
>
> Good.  I think we in the past was burned by some shells that want to
> see these assignments with "local" always quoted.

Yes. After I reached the same conclusion I noticed it was also added
to CodingGuidelines not long ago at be34b510 (CodingGuidelines: quote
assigned value in 'local var=$val').

>>      # preserve exit status
>> -    local exit=$?
>> +    local exit="$?"
>
> Well no matter what value $? has, it by definition has a few digits
> without any $IFS funnies.  Does this really matter?  I'd imagine
> that we would prefer to treat "$?" exactly the same way as "no".
>
> -        *)    return $exit
> +        *)    return "$exit"
>
> Likewise.

Two things here:

1. It can matter, because we don't control IFS. __git_ps1 is
   a function which runs in the user's shell, so if the user did
   IFS=0123, then unquoted $? or $exit can get IFS-split.
   As the commit message notes, this is unlikely to fix things in
   practice, but it will fix things with weird IFS values.

2. In general, yes, $? is only needed as yes/no, and there's only
   one place which tests $? instead of using "&&" or "||" after
   a command in this file (rev_parse_exit_code="$?"). I didn't feel
   this needs any portability fix. It works.

   But with $exit, $? is not used as yes/no, but rather to preserve
   the exit status when __git_ps1 was entered. This is important if
   the user wants the shell's last command $? at the prompt, e.g.:

   PS1='\w$(__git_ps1)$(e=$?; [ "$e" = 0 ] || echo " E:$e") \$ '

   If __git_ps1 didn't exit with the same $? it saw on entry, then
   $e will be __git_ps1's exit code rather than the exit code of
   the last command which ran in the shell, so it should be the
   same value as before and not only yes/no.

Re: [PATCH v2 5/8] git-prompt: add some missing quotes

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

>> Well no matter what value $? has, it by definition has a few digits
>> without any $IFS funnies.  Does this really matter?  I'd imagine
>> that we would prefer to treat "$?" exactly the same way as "no".
> ...
> Two things here:
>
> 1. It can matter, because we don't control IFS. __git_ps1 is
>    a function which runs in the user's shell, so if the user did
>    IFS=0123, then unquoted $? or $exit can get IFS-split.

Fair enough.  My "we would prefer to treat $? exactly the same way
as no" still stands.  If the user did IFS=o, "no" would be broken.

>    As the commit message notes, this is unlikely to fix things in
>    practice, but it will fix things with weird IFS values.

Yes, so I'd prefer to see us being consistent.  If we quote "$?" to
protect ourselves from crazy folks who set insane values to $IFS, we
should quote "no" the same way, no?

Re: [PATCH v2 5/8] git-prompt: add some missing quotes

[avih]

 On Thursday, August 15, 2024 at 10:15:53 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:

> Fair enough.  My "we would prefer to treat $? exactly the same way
> as no" still stands.  If the user did IFS=o, "no" would be broken.
>
>>    As the commit message notes, this is unlikely to fix things in
>>    practice, but it will fix things with weird IFS values.
>
>
> Yes, so I'd prefer to see us being consistent.  If we quote "$?" to
> protect ourselves from crazy folks who set insane values to $IFS, we
> should quote "no" the same way, no?

OK, I see what you mean. But IFS-split doesn't happen on literal
shell input (i.e. script source). It only happens on parts which
get expanded with parameter or arithmetic expansion or command
substitution. To quote from POSIX (2024):

  After parameter expansion (2.6.2), command substitution (2.6.3),
  and arithmetic expansion (2.6.4), if the shell variable IFS
  (see 2.5.3 Shell Variables ) is set and its value is not empty,
  or if IFS is unset, the shell shall scan each field containing
  results of expansions and substitutions that did not occur in
  double-quotes for field splitting; zero, one or multiple fields
  can result.

So 'IFS=n; reply=no; echo no; local x=no' work regardless of IFS,
because there's no expansion, and IFS is not involved.

But 'IFS=n; reply=no; echo $reply; local x=$reply' will be affected
when unquoted $reply expands as argument to "echo" (or "local"), and
then gets split by "n", so it would echo "o" (" o" because reasons).
Fixed with quotes: IFS=n; reply=no; echo "$reply"; local x="$reply"

Both can even be adjacent: 'IFS=n reply=no; echo no $reply'
would echo "no  o" in all shells, because the literal `no' is
unaffected, but the expanded $reply is affacted.

So $? is the same as $reply in this regards - it expands to some
value, so IFS gets involved, so it needs quotes. But a literal `no'
works the same regardless if quoted or not.

Worth noting in our context is that zsh doesn't do IFS word-split
by default on parameter expansion like $x, but does split the output
of command substitution $(cmd....), and code in git-prompt.sh is
expected to work in zsh as well (like it always did).

[PATCH v2 6/8] git-prompt: don't use shell $'...'

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

$'...' is new in POSIX (2024), and some shells support it in recent
versions, while others have had it for decades (bash, zsh, ksh93).

However, there are still enough shells which don't support it, and
it's cheap to use an alternative form which works in all shells,
so let's do that instead of dismissing it as "it's compliant".

It was agreed to use one form rather than $'...' where supported and
fallback otherwise.

shells where $'...' works:
- bash, zsh, ksh93, mksh, busybox-ash, dash master, free/net bsd sh.

shells where it doesn't work, but the new fallback works:
- all dash releases (up to 0.5.12), older versions of free/net bsd sh,
  openbsd sh, pdksh, all Schily Bourne sh variants, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 47 ++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5d7f236fe48..c3dd38f847c 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -111,6 +111,12 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
+# like __git_SOH=$'\001' etc but works also in shells without $'...'
+eval "$(printf '
+	__git_SOH="\001" __git_STX="\002" __git_ESC="\033"
+	__git_LF="\n" __git_CRLF="\r\n"
+')"
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -118,7 +124,7 @@ __git_ps1_show_upstream ()
 	local key value
 	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
-	local LF=$'\n'
+	local LF="$__git_LF"
 
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
@@ -271,12 +277,16 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue='%F{blue}'
 		local c_clear='%f'
 	else
-		# Using \001 and \002 around colors is necessary to prevent
-		# issues with command line editing/browsing/completion!
-		local c_red=$'\001\e[31m\002'
-		local c_green=$'\001\e[32m\002'
-		local c_lblue=$'\001\e[1;34m\002'
-		local c_clear=$'\001\e[0m\002'
+		# \001 (SOH) and \002 (STX) are 0-width substring markers
+		# which bash/readline identify while calculating the prompt
+		# on-screen width - to exclude 0-screen-width esc sequences.
+		local c_pre="${__git_SOH}${__git_ESC}["
+		local c_post="m${__git_STX}"
+
+		local c_red="${c_pre}31${c_post}"
+		local c_green="${c_pre}32${c_post}"
+		local c_lblue="${c_pre}1;34${c_post}"
+		local c_clear="${c_pre}0${c_post}"
 	fi
 	local bad_color="$c_red"
 	local ok_color="$c_green"
@@ -312,7 +322,7 @@ __git_ps1_colorize_gitstring ()
 # variable, in that order.
 __git_eread ()
 {
-	test -r "$1" && IFS=$'\r\n' read -r "$2" <"$1"
+	test -r "$1" && IFS=$__git_CRLF read -r "$2" <"$1"
 }
 
 # see if a cherry-pick or revert is in progress, if the user has committed a
@@ -430,19 +440,20 @@ __git_ps1 ()
 		return "$exit"
 	fi
 
+	local LF="$__git_LF"
 	local short_sha=""
 	if [ "$rev_parse_exit_code" = "0" ]; then
-		short_sha="${repo_info##*$'\n'}"
-		repo_info="${repo_info%$'\n'*}"
+		short_sha="${repo_info##*$LF}"
+		repo_info="${repo_info%$LF*}"
 	fi
-	local ref_format="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_worktree="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local bare_repo="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_gitdir="${repo_info##*$'\n'}"
-	local g="${repo_info%$'\n'*}"
+	local ref_format="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_worktree="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local bare_repo="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_gitdir="${repo_info##*$LF}"
+	local g="${repo_info%$LF*}"
 
 	if [ "true" = "$inside_worktree" ] &&
 	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED-}" ] &&
-- 
gitgitgadget

[PATCH v2 7/8] git-prompt: ta-da! document usage in other shells

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

With one big exception, git-prompt.sh should now be both almost posix
compliant, and also compatible with most (posix-ish) shells.

That exception is the use of "local" vars in functions, which happens
extensively in the current code, and is not simple to replace with
posix compliant code (but also not impossible).

Luckily, almost all shells support "local" as used by the current
code, with the notable exception of ksh93[u+m], but also the Schily
minimal posix sh (pbosh), and yash in posix mode.

See assessment below that "local" is likely the only blocker in those.

So except mainly ksh93, git-prompt.sh now works in most shells:
- bash, zsh, dash since at least 0.5.8, free/net bsd sh, busybox-ash,
  mksh, openbsd sh, pdksh(!), Schily extended Bourne sh (bosh), yash.

which is quite nice.

As an anecdote, replacing the 1st line in __git_ps1() (local exit=$?)
with these 2 makes it work in all tested shells, even without "local":

  # handles only 0/1 args for simplicity. needs +5 LOC for any $#
  __git_e=$?; local exit="$__git_e" 2>/dev/null ||
    {(eval 'local() { export "$@"; }'; __git_ps1 "$@"); return "$__git_e"; }

Explanation:

  If the shell doesn't have the command "local", define our own
  function "local" which instead does plain (global) assignents.
  Then use __git_ps1 in a subshell to not clober the caller's vars.

  This happens to work because currently there are no name conflicts
  (shadow) at the code, initial value is not assumed (i.e. always
  doing either 'local x=...'  or 'local x;...  x=...'), and assigned
  initial values are quoted (local x="$y"), preventing word split and
  glob expansion (i.e. assignment context is not assumed).

  The last two (always init, quote values) seem to be enough to use
  "local" portably if supported, and otherwise shells indeed differ.

  Uses "eval", else shells with "local" may reject it during parsing.
  We don't need "export", but it's smaller than writing our own loop.

While cute, this approach is not really sustainable because all the
vars become global, which is hard to maintain without conflicts
(but hey, it currently has no conflicts - without even trying...).

However, regardless of being an anecdote, it provides some support to
the assessment that "local" is the only blocker in those shells.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 33 ++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c3dd38f847c..75f272daa21 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -8,8 +8,8 @@
 # To enable:
 #
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
-#    2) Add the following line to your .bashrc/.zshrc:
-#        source ~/.git-prompt.sh
+#    2) Add the following line to your .bashrc/.zshrc/.profile:
+#        . ~/.git-prompt.sh   # dot path/to/this-file
 #    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
@@ -30,6 +30,8 @@
 #        Optionally, you can supply a third argument with a printf
 #        format string to finetune the output of the branch status
 #
+#    See notes below about compatibility with other shells.
+#
 # The repository status will be displayed only if you are currently in a
 # git repository. The %s token is the placeholder for the shown status.
 #
@@ -106,6 +108,33 @@
 # directory is set up to be ignored by git, then set
 # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
 # repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# Conpatibility with other shells (beyond bash/zsh):
+#
+#    We require posix-ish shell plus "local" support, which is most
+#    shells (even pdksh), but excluding ksh93 (because no "local").
+#
+#    Prompt integration might differ between shells, but the gist is
+#    to load it once on shell init with '. path/to/git-prompt.sh',
+#    set GIT_PS1* vars once as needed, and either place $(__git_ps1..)
+#    inside PS1 once (0/1 args), or, before each prompt is displayed,
+#    call __git_ps1 (2/3 args) which sets PS1 with the status embedded.
+#
+#    Many shells support the 1st method of command substitution,
+#    though some might need to first enable cmd substitution in PS1.
+#
+#    When using colors, each escape sequence is wrapped between byte
+#    values 1 and 2 (control chars SOH, STX, respectively), which are
+#    invisible at the output, but for bash/readline they mark 0-width
+#    strings (SGR color sequences) when calculating the on-screen
+#    prompt width, to maintain correct input editing at the prompt.
+#
+#    Currently there's no support for different markers, so if editing
+#    behaves weird when using colors in __git_ps1, then the solution
+#    is either to disable colors, or, in some shells which only care
+#    about the width of the last prompt line (e.g. busybox-ash),
+#    ensure the git output is not at the last line, maybe like so:
+#      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
 __git_printf_supports_v=
-- 
gitgitgadget

Re: [PATCH v2 7/8] git-prompt: ta-da! document usage in other shells

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 01:14:12PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index c3dd38f847c..75f272daa21 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -8,8 +8,8 @@
>  # To enable:
>  #
>  #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
> -#    2) Add the following line to your .bashrc/.zshrc:
> -#        source ~/.git-prompt.sh
> +#    2) Add the following line to your .bashrc/.zshrc/.profile:
> +#        . ~/.git-prompt.sh   # dot path/to/this-file
>  #    3a) Change your PS1 to call __git_ps1 as
>  #        command-substitution:
>  #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> @@ -30,6 +30,8 @@
>  #        Optionally, you can supply a third argument with a printf
>  #        format string to finetune the output of the branch status
>  #
> +#    See notes below about compatibility with other shells.
> +#
>  # The repository status will be displayed only if you are currently in a
>  # git repository. The %s token is the placeholder for the shown status.
>  #
> @@ -106,6 +108,33 @@
>  # directory is set up to be ignored by git, then set
>  # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
>  # repository level by setting bash.hideIfPwdIgnored to "false".
> +#
> +# Conpatibility with other shells (beyond bash/zsh):

s/Conpatibility/Compatibility/

Patrick

Re: [PATCH v2 7/8] git-prompt: ta-da! document usage in other shells

[avih]

 On Friday, August 16, 2024 at 11:50:17 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
> On Thu, Aug 15, 2024 at 01:14:12PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
>>
>> +# Conpatibility with other shells (beyond bash/zsh):
>
> s/Conpatibility/Compatibility/

Thanks. Will be fixed in v3.

[PATCH v2 8/8] git-prompt: support custom 0-width PS1 markers

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

When using colors, the shell needs to identify 0-width substrings
in PS1 - such as color escape sequences - when calculating the
on-screen width of the prompt.

Until now, we used the form %F{<color>} in zsh - which it knows is
0-width, or otherwise use standard SGR esc sequences wrapped between
byte values 1 and 2 (SOH, STX) as 0-width start/end markers, which
bash/readline identify as such.

But now that more shells are supported, the standard SGR sequences
typically work, but the SOH/STX markers might not be identified.

This commit adds support for vars GIT_PS1_COLOR_{PRE,POST} which
set custom 0-width markers or disable the markers.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 75f272daa21..5c43981aa11 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -129,11 +129,16 @@
 #    strings (SGR color sequences) when calculating the on-screen
 #    prompt width, to maintain correct input editing at the prompt.
 #
-#    Currently there's no support for different markers, so if editing
-#    behaves weird when using colors in __git_ps1, then the solution
-#    is either to disable colors, or, in some shells which only care
-#    about the width of the last prompt line (e.g. busybox-ash),
-#    ensure the git output is not at the last line, maybe like so:
+#    To replace or disable the 0-width markers, set GIT_PS1_COLOR_PRE
+#    and GIT_PS1_COLOR_POST to other markers, or empty (nul) to not
+#    use markers. For instance, some shells support '\[' and '\]' as
+#    start/end markers in PS1 - when invoking __git_ps1 with 3/4 args,
+#    but it may or may not work in command substitution mode. YMMV.
+#
+#    If the shell doesn't support 0-width markers and editing behaves
+#    incorrectly when using colors in __git_ps1, then, other than
+#    disabling color, it might be solved using multi-line prompt,
+#    where the git status is not at the last line, e.g.:
 #      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
@@ -309,8 +314,8 @@ __git_ps1_colorize_gitstring ()
 		# \001 (SOH) and \002 (STX) are 0-width substring markers
 		# which bash/readline identify while calculating the prompt
 		# on-screen width - to exclude 0-screen-width esc sequences.
-		local c_pre="${__git_SOH}${__git_ESC}["
-		local c_post="m${__git_STX}"
+		local c_pre="${GIT_PS1_COLOR_PRE-$__git_SOH}${__git_ESC}["
+		local c_post="m${GIT_PS1_COLOR_POST-$__git_STX}"
 
 		local c_red="${c_pre}31${c_post}"
 		local c_green="${c_pre}32${c_post}"
-- 
gitgitgadget

Re: [PATCH v2 0/8] git-prompt: support more shells v2

[Junio C Hamano]

"Avi Halachmi via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This addresses review comment on part 6/8 (git-prompt: add fallback for
> shells without $'...') which requested to use one form for all shells
> instead $'...' where supported and a fallback otherwise.

I've read the series and they looked all sensible.  Will queue but
I'd appreciate a second set of eyes before marking it for 'next'.

Thanks.

Re: [PATCH v2 0/8] git-prompt: support more shells v2

[Patrick Steinhardt]

On Thu, Aug 15, 2024 at 11:48:23AM -0700, Junio C Hamano wrote:
> "Avi Halachmi via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > This addresses review comment on part 6/8 (git-prompt: add fallback for
> > shells without $'...') which requested to use one form for all shells
> > instead $'...' where supported and a fallback otherwise.
> 
> I've read the series and they looked all sensible.  Will queue but
> I'd appreciate a second set of eyes before marking it for 'next'.

I did have a look, but honestly I wouldn't consider that to be a
qualified review. POSIX shell tends to get borderline unreadable, so I
don't want to claim to understand everything I've read.

In any case, I didn't spot anything grave.

Patrick

[PATCH v3 0/8] git-prompt: support more shells v3

[Avi Halachmi via GitGitGadget]

This addresses review comments on part 5/8 v2 (git-prompt: add some missing
quotes) to fix typo in the commit message "aguments" into "arguments", but
which was used to reword it a bit so that it's more accurate.

Also addresses review comment on part 7/8 v2 (git-prompt: ta-da! document
usage in other shells) and fix typo "Conpatibility".

Avi Halachmi (:avih) (8):
  git-prompt: use here-doc instead of here-string
  git-prompt: fix uninitialized variable
  git-prompt: don't use shell arrays
  git-prompt: replace [[...]] with standard code
  git-prompt: add some missing quotes
  git-prompt: don't use shell $'...'
  git-prompt: ta-da! document usage in other shells
  git-prompt: support custom 0-width PS1 markers

 contrib/completion/git-prompt.sh | 191 ++++++++++++++++++++-----------
 1 file changed, 126 insertions(+), 65 deletions(-)


base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1750%2Favih%2Fprompt-compat-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1750/avih/prompt-compat-v3
Pull-Request: https://github.com/git/git/pull/1750

Range-diff vs v2:

 1:  9ce5ddadf0b = 1:  9ce5ddadf0b git-prompt: use here-doc instead of here-string
 2:  680ecb52404 = 2:  680ecb52404 git-prompt: fix uninitialized variable
 3:  7e994eae7bc = 3:  7e994eae7bc git-prompt: don't use shell arrays
 4:  232340902a1 = 4:  232340902a1 git-prompt: replace [[...]] with standard code
 5:  4f77b7eb7f1 ! 5:  3a41ad889cc git-prompt: add some missing quotes
     @@ Commit message
            not expanded:   t="~"; a=${t}user  b=\~foo~;  echo "~user" $t/dir
      
          But the main reason for quoting is to prevent IFS field splitting
     -    (which also coalesces IFS chars) and glob expansion after parameter
     -    expansion or command substitution.
     +    (which also coalesces IFS chars) and glob expansion in parts which
     +    contain parameter/arithmetic expansion or command substitution.
      
     -    In _command-arguments_, expanded/substituted values must be quoted:
     +    "Simple command" (POSIX term) is assignment[s] and/or command [args].
     +    Examples:
     +      foo=bar         # one assignment
     +      foo=$bar x=y    # two assignments
     +      foo bar         # command, no assignments
     +      x=123 foo bar   # one assignment and a command
     +
     +    The assignments part is not IFS-split or glob-expanded.
     +
     +    The command+args part does get IFS field split and glob expanded,
     +    but only at unquoted expanded/substituted parts.
     +
     +    In the command+args part, expanded/substituted values must be quoted.
     +    (the commands here are "[" and "local"):
            Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
            Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)
      
     -    Still in _agumemts_, no need to quote non-expandable values:
     +    The arguments to "local" do look like assignments, but they're not
     +    the assignment part of a simple command. they're at the command part.
     +
     +    Still at the command part, no need to quote non-expandable values:
            Good:                 local x=   y=yes;   echo OK
            OK, but not required: local x="" y="yes"; echo "OK"
          But completely empty (NULL) arguments must be quoted:
     @@ Commit message
          "local" does not behave with assignment context in some shells,
          hence we require quotes when using "local" - for compatibility.
      
     -    First value in 'case...' doesn't IFS-split/glob, doesn't need quotes:
     +    The value between 'case' and 'in' doesn't IFS-split/glob-expand:
            Good:       case  * $foo $(cmd...)  in ... ; esac
            identical:  case "* $foo $(cmd...)" in ... ; esac
      
 6:  363b7015763 = 6:  e735a1696a0 git-prompt: don't use shell $'...'
 7:  4aa75cdb5dd ! 7:  e70440e669a git-prompt: ta-da! document usage in other shells
     @@ contrib/completion/git-prompt.sh
       # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
       # repository level by setting bash.hideIfPwdIgnored to "false".
      +#
     -+# Conpatibility with other shells (beyond bash/zsh):
     ++# Compatibility with other shells (beyond bash/zsh):
      +#
      +#    We require posix-ish shell plus "local" support, which is most
      +#    shells (even pdksh), but excluding ksh93 (because no "local").
 8:  e71ddcd2232 = 8:  633e71a01d3 git-prompt: support custom 0-width PS1 markers

-- 
gitgitgadget

[PATCH v3 1/8] git-prompt: use here-doc instead of here-string

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Here-documend is standard, and works in all shells.

Both here-string and here-doc add final newline, which is important
in this case, because $output is without final newline, but we do
want "read" to succeed on the last line as well.

Shells which support here-string:
- bash, zsh, mksh, ksh93, yash (non-posix-mode).

shells which don't, and got fixed:
- ash-derivatives (dash, free/net bsd sh, busybox-ash).
- pdksh, openbsd sh.
- All Schily Bourne shell variants.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5330e769a72..ebf2e30d684 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -137,7 +137,9 @@ __git_ps1_show_upstream ()
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
-	done <<< "$output"
+	done <<-OUTPUT
+		$output
+	OUTPUT
 
 	# parse configuration values
 	local option
-- 
gitgitgadget

[PATCH v3 2/8] git-prompt: fix uninitialized variable

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

First use is in the form:  local var; ...; var=$var$whatever...

If the variable was unset (as bash and others do after "local x"),
then it would error if set -u is in effect.

Also, many shells inherit the existing value after "local var"
without init, but in this case it's unlikely to have a prior value.

Now we initialize it.

(local var= is enough, but local var="" is the custom in this file)

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index ebf2e30d684..4cc2cf91bb6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,7 +116,7 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern count n
+	local svn_remote svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
-- 
gitgitgadget

[PATCH v3 3/8] git-prompt: don't use shell arrays

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Arrays only existed in the svn-upstream code, used to:
- Keep a list of svn remotes.
- Convert commit msg to array of words, extract the 2nd-to-last word.

Except bash/zsh, nearly all shells failed load on syntax errors here.

Now:
- The svn remotes are a list of newline-terminated values.
- The 2nd-to-last word is extracted using standard shell substrings.
- All shells can digest the svn-upstream code.

While using shell field splitting to extract the word is simple, and
doesn't even need non-standard code, e.g. set -- $(git log -1 ...),
it would have the same issues as the old array code: it depends on IFS
which we don't control, and it's subject to glob-expansion, e.g. if
the message happens to include * or **/* (as this commit message just
did), then the array could get huge. This was not great.

Now it uses standard shell substrings, and we know the exact delimiter
to expect, because it's the match from our grep just one line earlier.

The new word extraction code also fixes svn-upstream in zsh, because
previously it used arr[len-2], but because in zsh, unlike bash, array
subscripts are 1-based, it incorrectly extracted the 3rd-to-last word.
symptom: missing upstream status in a git-svn repo: u=, u+N-M, etc.

The breakage in zsh is surprising, because it was last touched by
  commit d0583da838 (prompt: fix show upstream with svn and zsh),
claiming to fix exactly that. However, it only mentions syntax fixes.
It's unclear if behavior was fixed too. But it was broken, now fixed.

Note LF=$'\n' and then using $LF instead of $'\n' few times.
A future commit will add fallback for shells without $'...', so this
would be the only line to touch instead of replacing every $'\n' .

Shells which could run the previous array code:
- bash

Shells which have arrays but were broken anyway:
- zsh: 1-based subscript
- ksh93: no "local" (the new code can't fix this part...)
- mksh, openbsd sh, pdksh: failed load on syntax error: "for ((...))".

More shells which Failed to load due to syntax error:
- dash, free/net bsd sh, busybox-ash, Schily Bourne shell, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 48 ++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4cc2cf91bb6..75c3a813fda 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,10 +116,10 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern="" count n
+	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
+	local LF=$'\n'
 
-	svn_remote=()
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
 	while read -r key value; do
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
+			svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
 			svn_url_pattern="$svn_url_pattern\\|$value"
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
@@ -156,25 +156,37 @@ __git_ps1_show_upstream ()
 	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)
-		local -a svn_upstream
-		svn_upstream=($(git log --first-parent -1 \
-					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
-		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-			svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
-			svn_upstream=${svn_upstream%@*}
-			local n_stop="${#svn_remote[@]}"
-			for ((n=1; n <= n_stop; n++)); do
-				svn_upstream=${svn_upstream#${svn_remote[$n]}}
-			done
+		# successful svn-upstream resolution:
+		# - get the list of configured svn-remotes ($svn_remotes set above)
+		# - get the last commit which seems from one of our svn-remotes
+		# - confirm that it is from one of the svn-remotes
+		# - use $GIT_SVN_ID if set, else "git-svn"
 
-			if [[ -z "$svn_upstream" ]]; then
+		# get upstream from "git-svn-id: UPSTRM@N HASH" in a commit message
+		# (git-svn uses essentially the same procedure internally)
+		local svn_upstream="$(
+			git log --first-parent -1 \
+				--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null
+		)"
+
+		if [ -n "$svn_upstream" ]; then
+			# extract the URI, assuming --grep matched the last line
+			svn_upstream=${svn_upstream##*$LF}  # last line
+			svn_upstream=${svn_upstream#*: }    # UPSTRM@N HASH
+			svn_upstream=${svn_upstream%@*}     # UPSTRM
+
+			case ${LF}${svn_remotes} in
+			*"${LF}${svn_upstream}${LF}"*)
+				# grep indeed matched the last line - it's our remote
 				# default branch name for checkouts with no layout:
 				upstream_type=${GIT_SVN_ID:-git-svn}
-			else
+				;;
+			*)
+				# the commit message includes one of our remotes, but
+				# it's not at the last line. is $svn_upstream junk?
 				upstream_type=${svn_upstream#/}
-			fi
+				;;
+			esac
 		elif [[ "svn+git" = "$upstream_type" ]]; then
 			upstream_type="@{upstream}"
 		fi
-- 
gitgitgadget

[PATCH v3 4/8] git-prompt: replace [[...]] with standard code

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The existing [[...]] tests were either already valid as standard [...]
tests, or only required minimal retouch:

Notes:

- [[...]] doesn't do field splitting and glob expansion, so $var
  or $(cmd...) don't need quoting, but [... does need quotes.

- [[ X == Y ]] when Y is a string is same as [ X = Y ], but if Y is
  a pattern, then we need:  case X in Y)... ; esac  .

- [[ ... && ... ]] was replaced with [ ... ] && [ ... ] .

- [[ -o <zsh-option> ]] requires [[...]], so put it in "eval" and only
  eval it in zsh, so other shells would not abort on syntax error
  (posix says [[ has unspecified results, shells allowed to reject it)

- ((x++)) was changed into x=$((x+1))  (yeah, not [[...]] ...)

Shells which accepted the previous forms:
- bash, zsh, ksh93, mksh, openbsd sh, pdksh.

Shells which didn't, and now can process it:
- dash, free/net bsd sh, busybox-ash, Schily Bourne sh, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 75c3a813fda..4781261f868 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -126,7 +126,7 @@ __git_ps1_show_upstream ()
 		case "$key" in
 		bash.showupstream)
 			GIT_PS1_SHOWUPSTREAM="$value"
-			if [[ -z "${GIT_PS1_SHOWUPSTREAM}" ]]; then
+			if [ -z "${GIT_PS1_SHOWUPSTREAM}" ]; then
 				p=""
 				return
 			fi
@@ -187,14 +187,14 @@ __git_ps1_show_upstream ()
 				upstream_type=${svn_upstream#/}
 				;;
 			esac
-		elif [[ "svn+git" = "$upstream_type" ]]; then
+		elif [ "svn+git" = "$upstream_type" ]; then
 			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
 
 	# Find how many commits we are ahead/behind our upstream
-	if [[ -z "$legacy" ]]; then
+	if [ -z "$legacy" ]; then
 		count="$(git rev-list --count --left-right \
 				"$upstream_type"...HEAD 2>/dev/null)"
 	else
@@ -206,8 +206,8 @@ __git_ps1_show_upstream ()
 			for commit in $commits
 			do
 				case "$commit" in
-				"<"*) ((behind++)) ;;
-				*)    ((ahead++))  ;;
+				"<"*) behind=$((behind+1)) ;;
+				*)    ahead=$((ahead+1))   ;;
 				esac
 			done
 			count="$behind	$ahead"
@@ -217,7 +217,7 @@ __git_ps1_show_upstream ()
 	fi
 
 	# calculate the result
-	if [[ -z "$verbose" ]]; then
+	if [ -z "$verbose" ]; then
 		case "$count" in
 		"") # no upstream
 			p="" ;;
@@ -243,7 +243,7 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
-		if [[ -n "$count" && -n "$name" ]]; then
+		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
@@ -265,7 +265,7 @@ __git_ps1_show_upstream ()
 # their own color.
 __git_ps1_colorize_gitstring ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
+	if [ -n "${ZSH_VERSION-}" ]; then
 		local c_red='%F{red}'
 		local c_green='%F{green}'
 		local c_lblue='%F{blue}'
@@ -417,7 +417,7 @@ __git_ps1 ()
 	# incorrect.)
 	#
 	local ps1_expanded=yes
-	[ -z "${ZSH_VERSION-}" ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+	[ -z "${ZSH_VERSION-}" ] || eval '[[ -o PROMPT_SUBST ]]' || ps1_expanded=no
 	[ -z "${BASH_VERSION-}" ] || shopt -q promptvars || ps1_expanded=no
 
 	local repo_info rev_parse_exit_code
@@ -502,11 +502,13 @@ __git_ps1 ()
 					return $exit
 				fi
 
-				if [[ $head == "ref: "* ]]; then
+				case $head in
+				"ref: "*)
 					head="${head#ref: }"
-				else
+					;;
+				*)
 					head=""
-				fi
+				esac
 				;;
 			*)
 				head="$(git symbolic-ref HEAD 2>/dev/null)"
@@ -542,8 +544,8 @@ __git_ps1 ()
 	fi
 
 	local conflict="" # state indicator for unresolved conflicts
-	if [[ "${GIT_PS1_SHOWCONFLICTSTATE-}" == "yes" ]] &&
-	   [[ $(git ls-files --unmerged 2>/dev/null) ]]; then
+	if [ "${GIT_PS1_SHOWCONFLICTSTATE-}" = "yes" ] &&
+	   [ "$(git ls-files --unmerged 2>/dev/null)" ]; then
 		conflict="|CONFLICT"
 	fi
 
-- 
gitgitgadget

[PATCH v3 5/8] git-prompt: add some missing quotes

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The issues which this commit fixes are unlikely to be broken
in real life, but the fixes improve correctness, and would prevent
bugs in some uncommon cases, such as weird IFS values.

Listing some portability guideline here for future reference.

I'm leaving it to someone else to decide whether to include
it in the file itself, place is as a new file, or not.

---------

The command "local" is non standard, but is allowed in this file:
- Quote initialization if it can expand (local x="$y"). See below.
- Don't assume initial value after "local x". Either initialize it
  (local x=..), or set before first use (local x;.. x=..; <use $x>).
  (between shells, "local x" can unset x, or inherit it, or do x= )

Other non-standard features beyond "local" are to be avoided.

Use the standard "test" - [...] instead of non-standard [[...]] .

--------

Quotes (some portability things, but mainly general correctness):

Quotes prevent tilde-expansion of some unquoted literal tildes (~).
If the expansion is undesirable, quotes would ensure that.
  Tilds expanded: a=~user:~/ ;  echo ~user ~/dir
  not expanded:   t="~"; a=${t}user  b=\~foo~;  echo "~user" $t/dir

But the main reason for quoting is to prevent IFS field splitting
(which also coalesces IFS chars) and glob expansion in parts which
contain parameter/arithmetic expansion or command substitution.

"Simple command" (POSIX term) is assignment[s] and/or command [args].
Examples:
  foo=bar         # one assignment
  foo=$bar x=y    # two assignments
  foo bar         # command, no assignments
  x=123 foo bar   # one assignment and a command

The assignments part is not IFS-split or glob-expanded.

The command+args part does get IFS field split and glob expanded,
but only at unquoted expanded/substituted parts.

In the command+args part, expanded/substituted values must be quoted.
(the commands here are "[" and "local"):
  Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
  Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)

The arguments to "local" do look like assignments, but they're not
the assignment part of a simple command. they're at the command part.

Still at the command part, no need to quote non-expandable values:
  Good:                 local x=   y=yes;   echo OK
  OK, but not required: local x="" y="yes"; echo "OK"
But completely empty (NULL) arguments must be quoted:
  foo ""   is not the same as:   foo

Assignments in simple commands - with or without an actual command,
don't need quoting becase there's no IFS split or glob expansion:
  Good:   s=* a=$b c=$(cmd...)${x# foo }${y-   } [cmd ...]
  It's also OK to use double quotes, but not required.

This behavior (no IFS/glob) is called "assignment context", and
"local" does not behave with assignment context in some shells,
hence we require quotes when using "local" - for compatibility.

The value between 'case' and 'in' doesn't IFS-split/glob-expand:
  Good:       case  * $foo $(cmd...)  in ... ; esac
  identical:  case "* $foo $(cmd...)" in ... ; esac

Nested quotes in command substitution are fine, often necessary:
  Good: echo "$(foo... "$x" "$(bar ...)")"

Nested quotes in substring ops are legal, and sometimes needed
to prevent interpretation as a pattern, but not the most readable:
  Legal:  foo "${x#*"$y" }"

Nested quotes in "maybe other value" subst are invalid, unnecessary:
  Good:  local x="${y- }";   foo "${z:+ $a }"
  Bad:   local x="${y-" "}"; foo "${z:+" $a "}"
Outer/inner quotes in "maybe other value" have different use cases:
  "${x-$y}"  always one quoted arg: "$x" if x is set, else "$y".
  ${x+"$x"}  one quoted arg "$x" if x is set, else no arg at all.
  Unquoted $x is similar to the second case, but it would get split
  into few arguments if it includes any of the IFS chars.

Assignments don't need the outer quotes, and the braces delimit the
value, so nested quotes can be avoided, for readability:
  a=$(foo "$x")  a=${x#*"$y" }  c=${y- };  bar "$a" "$b" "$c"

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.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 4781261f868..5d7f236fe48 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -246,7 +246,7 @@ __git_ps1_show_upstream ()
 		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
+			if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
 				upstream="$upstream ${__git_ps1_upstream_name}"
@@ -278,12 +278,12 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue=$'\001\e[1;34m\002'
 		local c_clear=$'\001\e[0m\002'
 	fi
-	local bad_color=$c_red
-	local ok_color=$c_green
+	local bad_color="$c_red"
+	local ok_color="$c_green"
 	local flags_color="$c_lblue"
 
 	local branch_color=""
-	if [ $detached = no ]; then
+	if [ "$detached" = no ]; then
 		branch_color="$ok_color"
 	else
 		branch_color="$bad_color"
@@ -360,7 +360,7 @@ __git_sequencer_status ()
 __git_ps1 ()
 {
 	# preserve exit status
-	local exit=$?
+	local exit="$?"
 	local pcmode=no
 	local detached=no
 	local ps1pc_start='\u@\h:\w '
@@ -379,7 +379,7 @@ __git_ps1 ()
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
-		*)	return $exit
+		*)	return "$exit"
 		;;
 	esac
 
@@ -427,7 +427,7 @@ __git_ps1 ()
 	rev_parse_exit_code="$?"
 
 	if [ -z "$repo_info" ]; then
-		return $exit
+		return "$exit"
 	fi
 
 	local short_sha=""
@@ -449,7 +449,7 @@ __git_ps1 ()
 	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
 	   git check-ignore -q .
 	then
-		return $exit
+		return "$exit"
 	fi
 
 	local sparse=""
@@ -499,7 +499,7 @@ __git_ps1 ()
 			case "$ref_format" in
 			files)
 				if ! __git_eread "$g/HEAD" head; then
-					return $exit
+					return "$exit"
 				fi
 
 				case $head in
@@ -597,10 +597,10 @@ __git_ps1 ()
 		fi
 	fi
 
-	local z="${GIT_PS1_STATESEPARATOR-" "}"
+	local z="${GIT_PS1_STATESEPARATOR- }"
 
 	b=${b##refs/heads/}
-	if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
+	if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 		__git_ps1_branch_name=$b
 		b="\${__git_ps1_branch_name}"
 	fi
@@ -612,7 +612,7 @@ __git_ps1 ()
 	local f="$h$w$i$s$u$p"
 	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}${conflict}"
 
-	if [ $pcmode = yes ]; then
+	if [ "$pcmode" = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
 			gitstring=$(printf -- "$printf_format" "$gitstring")
 		else
@@ -623,5 +623,5 @@ __git_ps1 ()
 		printf -- "$printf_format" "$gitstring"
 	fi
 
-	return $exit
+	return "$exit"
 }
-- 
gitgitgadget

Re: [PATCH v3 5/8] git-prompt: add some missing quotes

[Eric Sunshine]

On Sat, Aug 17, 2024 at 5:26 AM Avi Halachmi (:avih) via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The issues which this commit fixes are unlikely to be broken
> in real life, but the fixes improve correctness, and would prevent
> bugs in some uncommon cases, such as weird IFS values.
>
> Listing some portability guideline here for future reference.

s/guideline/guidelines/

> I'm leaving it to someone else to decide whether to include
> it in the file itself, place is as a new file, or not.

perhaps: s/is as/it as/

> "Simple command" (POSIX term) is assignment[s] and/or command [args].
> Examples:
>   foo=bar         # one assignment
>   foo=$bar x=y    # two assignments
>   foo bar         # command, no assignments
>   x=123 foo bar   # one assignment and a command
>
> The assignments part is not IFS-split or glob-expanded.
>
> The command+args part does get IFS field split and glob expanded,
> but only at unquoted expanded/substituted parts.
>
> In the command+args part, expanded/substituted values must be quoted.
> (the commands here are "[" and "local"):
>   Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
>   Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)

This new explanation in v3 is a helpful addition.

> The arguments to "local" do look like assignments, but they're not
> the assignment part of a simple command. they're at the command part.

either: s/they're/They're/
or: s/. they're/; they're/

I doubt that any of the above extremely minor commit message botches
is worth a reroll.

Re: [PATCH v3 5/8] git-prompt: add some missing quotes

[avih]

 On Saturday, August 17, 2024 at 12:38:23 PM GMT+3, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Aug 17, 2024 at 5:26 AM Avi Halachmi (:avih) via GitGitGadget
>> Listing some portability guideline here for future reference.
>
>> s/guideline/guidelines/

Right.

>> I'm leaving it to someone else to decide whether to include
>> it in the file itself, place is as a new file, or not.
>
> perhaps: s/is as/it as/

Indeed.

>> "Simple command" (POSIX term) is assignment[s] and/or command [args].
>> Examples:
>> ...
>
> This new explanation in v3 is a helpful addition.

Thanks.

>> The arguments to "local" do look like assignments, but they're not
>> the assignment part of a simple command. they're at the command part.
>
> either: s/they're/They're/
> or: s/. they're/; they're/

Indeed. Thanks for the comments.

> I doubt that any of the above extremely minor commit message botches
> is worth a reroll.

I'm not the one to judge that, but I'm OK to push as many new versions
as deemed required/preferred. For now I'm waiting for someone to say
whether I should do that or not... and if yes - when.

I was trying to wait few days for more comments on v2 (perhaps
like yours), but I noticed that v2 was already was just integrated
into "seen", so I posted v3 to address the existing comments on v2.

This is my 1st patch to "git", so still finding my feet with the
procedures.

avih

Re: [PATCH v3 5/8] git-prompt: add some missing quotes

[Junio C Hamano]

avih <avihpit@yahoo.com> writes:

> I was trying to wait few days for more comments on v2 (perhaps
> like yours), but I noticed that v2 was already was just integrated
> into "seen", so I posted v3 to address the existing comments on v2.

Please consider that it is just like being on the list and nothing
else to be in "seen".  It merely is another place some patches I've
"seen" are published, to help those of you who find "git fetch &&
git log -pW origin/master..origin/topic" a more convenient way to
review the changes.  This is outlined in the note I send out
occasionally.

    https://lore.kernel.org/git/xmqqmslewwpo.fsf@gitster.g/

If you think that v2 needs a few more days' exposure to receive more
feedback from reviewers, and that v3 might be incomplete before
waiting for their feedback, just saying so as a response to the
"What's cooking" message is a very effective way to make sure I'll
wait for an updated iteration.  Such a comment on individual topics
is *not* limited to the author of the topic, e.g.

  https://lore.kernel.org/git/owlyil264yew.fsf@fine.c.googlers.com/

is an example (but the particular example was sent a bit too
late---such a "wait, don't merge, an update is coming" needs to come
before the topic is merged down to 'next'.

  https://lore.kernel.org/git/13f08ce5-f036-f769-1ba9-7d47b572af28@gmail.com/

is an example of a reviewer sending a "stop, I have an improvement
to suggest on this one I'll send soon" on a topic by somebody else.

  https://lore.kernel.org/git/Zr2_4sNu56_frqlr@tanuki/

is an example of the author saying "this topic of mine should be
complete already", which is a bit less usual and takes some finesse
to avoid sounding too self-promoting (this particular example does
so well).

Re: [PATCH v3 5/8] git-prompt: add some missing quotes

[avih]

 On Saturday, August 17, 2024 at 07:28:44 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
> avih <avihpit@yahoo.com> writes:
>> I was trying to wait few days for more comments on v2 (perhaps
>> like yours), but I noticed that v2 was already was just integrated
>> into "seen", so I posted v3 to address the existing comments on v2.
>
> Please consider that it is just like being on the list and nothing
> else to be in "seen".  It merely is another place some patches I've
> "seen" are published, to help those of you who find "git fetch &&
> git log -pW origin/master..origin/topic" a more convenient way to
> review the changes.  This is outlined in the note I send out
> occasionally.
>
>     https://lore.kernel.org/git/xmqqmslewwpo.fsf@gitster.g/

Thanks for the time and info. That's a useful intro, and I now do
see it says this:

  until a topic is merged to "next", updates to it is expected
  by replacing the patch(es) in the topic with an improved version

> If you think that v2 needs a few more days' exposure to receive more
> feedback from reviewers, and that v3 might be incomplete before
> waiting for their feedback, just saying so as a response to the
> "What's cooking" message is a very effective way to make sure I'll
> wait for an updated iteration.  Such a comment on individual topics
> is *not* limited to the author of the topic, e.g.
>
>   https://lore.kernel.org/git/owlyil264yew.fsf@fine.c.googlers.com/
>
> is an example ...

Thanks. I replied few times that requested changes will be included
in v3, so I thought it's apparent that v3 is sill to come, but in
retrospect, when you replied to [PATCH v2 0/8]:

> I've read the series and they looked all sensible.  Will queue but
> I'd appreciate a second set of eyes before marking it for 'next'.

Then I should have replied with something along those lines:

  Please wait for v3 with requested non-critical changes (typos in
  comment and commit message) before moving it to "next", if at all.

or even sending the same message once I noticed it was merged into
"seen", instead of posting v3 out of "panic" that it boarded the
train without all the requested changes applied, yes?

If yes, then here's heads-up that there's still another non-critical
requested change to arrive in v4 (commit message wording), which I'll
send in few days, to allow for further comments to arrive.

Because resending the whole series (is that "reroll"?) for every
minor typo or wording change feels noisy and inappropriate to me.

Thanks again for the time and info.

avih

[PATCH v3 6/8] git-prompt: don't use shell $'...'

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

$'...' is new in POSIX (2024), and some shells support it in recent
versions, while others have had it for decades (bash, zsh, ksh93).

However, there are still enough shells which don't support it, and
it's cheap to use an alternative form which works in all shells,
so let's do that instead of dismissing it as "it's compliant".

It was agreed to use one form rather than $'...' where supported and
fallback otherwise.

shells where $'...' works:
- bash, zsh, ksh93, mksh, busybox-ash, dash master, free/net bsd sh.

shells where it doesn't work, but the new fallback works:
- all dash releases (up to 0.5.12), older versions of free/net bsd sh,
  openbsd sh, pdksh, all Schily Bourne sh variants, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 47 ++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5d7f236fe48..c3dd38f847c 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -111,6 +111,12 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
+# like __git_SOH=$'\001' etc but works also in shells without $'...'
+eval "$(printf '
+	__git_SOH="\001" __git_STX="\002" __git_ESC="\033"
+	__git_LF="\n" __git_CRLF="\r\n"
+')"
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -118,7 +124,7 @@ __git_ps1_show_upstream ()
 	local key value
 	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
-	local LF=$'\n'
+	local LF="$__git_LF"
 
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
@@ -271,12 +277,16 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue='%F{blue}'
 		local c_clear='%f'
 	else
-		# Using \001 and \002 around colors is necessary to prevent
-		# issues with command line editing/browsing/completion!
-		local c_red=$'\001\e[31m\002'
-		local c_green=$'\001\e[32m\002'
-		local c_lblue=$'\001\e[1;34m\002'
-		local c_clear=$'\001\e[0m\002'
+		# \001 (SOH) and \002 (STX) are 0-width substring markers
+		# which bash/readline identify while calculating the prompt
+		# on-screen width - to exclude 0-screen-width esc sequences.
+		local c_pre="${__git_SOH}${__git_ESC}["
+		local c_post="m${__git_STX}"
+
+		local c_red="${c_pre}31${c_post}"
+		local c_green="${c_pre}32${c_post}"
+		local c_lblue="${c_pre}1;34${c_post}"
+		local c_clear="${c_pre}0${c_post}"
 	fi
 	local bad_color="$c_red"
 	local ok_color="$c_green"
@@ -312,7 +322,7 @@ __git_ps1_colorize_gitstring ()
 # variable, in that order.
 __git_eread ()
 {
-	test -r "$1" && IFS=$'\r\n' read -r "$2" <"$1"
+	test -r "$1" && IFS=$__git_CRLF read -r "$2" <"$1"
 }
 
 # see if a cherry-pick or revert is in progress, if the user has committed a
@@ -430,19 +440,20 @@ __git_ps1 ()
 		return "$exit"
 	fi
 
+	local LF="$__git_LF"
 	local short_sha=""
 	if [ "$rev_parse_exit_code" = "0" ]; then
-		short_sha="${repo_info##*$'\n'}"
-		repo_info="${repo_info%$'\n'*}"
+		short_sha="${repo_info##*$LF}"
+		repo_info="${repo_info%$LF*}"
 	fi
-	local ref_format="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_worktree="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local bare_repo="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_gitdir="${repo_info##*$'\n'}"
-	local g="${repo_info%$'\n'*}"
+	local ref_format="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_worktree="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local bare_repo="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_gitdir="${repo_info##*$LF}"
+	local g="${repo_info%$LF*}"
 
 	if [ "true" = "$inside_worktree" ] &&
 	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED-}" ] &&
-- 
gitgitgadget

[PATCH v3 7/8] git-prompt: ta-da! document usage in other shells

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

With one big exception, git-prompt.sh should now be both almost posix
compliant, and also compatible with most (posix-ish) shells.

That exception is the use of "local" vars in functions, which happens
extensively in the current code, and is not simple to replace with
posix compliant code (but also not impossible).

Luckily, almost all shells support "local" as used by the current
code, with the notable exception of ksh93[u+m], but also the Schily
minimal posix sh (pbosh), and yash in posix mode.

See assessment below that "local" is likely the only blocker in those.

So except mainly ksh93, git-prompt.sh now works in most shells:
- bash, zsh, dash since at least 0.5.8, free/net bsd sh, busybox-ash,
  mksh, openbsd sh, pdksh(!), Schily extended Bourne sh (bosh), yash.

which is quite nice.

As an anecdote, replacing the 1st line in __git_ps1() (local exit=$?)
with these 2 makes it work in all tested shells, even without "local":

  # handles only 0/1 args for simplicity. needs +5 LOC for any $#
  __git_e=$?; local exit="$__git_e" 2>/dev/null ||
    {(eval 'local() { export "$@"; }'; __git_ps1 "$@"); return "$__git_e"; }

Explanation:

  If the shell doesn't have the command "local", define our own
  function "local" which instead does plain (global) assignents.
  Then use __git_ps1 in a subshell to not clober the caller's vars.

  This happens to work because currently there are no name conflicts
  (shadow) at the code, initial value is not assumed (i.e. always
  doing either 'local x=...'  or 'local x;...  x=...'), and assigned
  initial values are quoted (local x="$y"), preventing word split and
  glob expansion (i.e. assignment context is not assumed).

  The last two (always init, quote values) seem to be enough to use
  "local" portably if supported, and otherwise shells indeed differ.

  Uses "eval", else shells with "local" may reject it during parsing.
  We don't need "export", but it's smaller than writing our own loop.

While cute, this approach is not really sustainable because all the
vars become global, which is hard to maintain without conflicts
(but hey, it currently has no conflicts - without even trying...).

However, regardless of being an anecdote, it provides some support to
the assessment that "local" is the only blocker in those shells.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 33 ++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c3dd38f847c..6be2f1dd901 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -8,8 +8,8 @@
 # To enable:
 #
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
-#    2) Add the following line to your .bashrc/.zshrc:
-#        source ~/.git-prompt.sh
+#    2) Add the following line to your .bashrc/.zshrc/.profile:
+#        . ~/.git-prompt.sh   # dot path/to/this-file
 #    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
@@ -30,6 +30,8 @@
 #        Optionally, you can supply a third argument with a printf
 #        format string to finetune the output of the branch status
 #
+#    See notes below about compatibility with other shells.
+#
 # The repository status will be displayed only if you are currently in a
 # git repository. The %s token is the placeholder for the shown status.
 #
@@ -106,6 +108,33 @@
 # directory is set up to be ignored by git, then set
 # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
 # repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# Compatibility with other shells (beyond bash/zsh):
+#
+#    We require posix-ish shell plus "local" support, which is most
+#    shells (even pdksh), but excluding ksh93 (because no "local").
+#
+#    Prompt integration might differ between shells, but the gist is
+#    to load it once on shell init with '. path/to/git-prompt.sh',
+#    set GIT_PS1* vars once as needed, and either place $(__git_ps1..)
+#    inside PS1 once (0/1 args), or, before each prompt is displayed,
+#    call __git_ps1 (2/3 args) which sets PS1 with the status embedded.
+#
+#    Many shells support the 1st method of command substitution,
+#    though some might need to first enable cmd substitution in PS1.
+#
+#    When using colors, each escape sequence is wrapped between byte
+#    values 1 and 2 (control chars SOH, STX, respectively), which are
+#    invisible at the output, but for bash/readline they mark 0-width
+#    strings (SGR color sequences) when calculating the on-screen
+#    prompt width, to maintain correct input editing at the prompt.
+#
+#    Currently there's no support for different markers, so if editing
+#    behaves weird when using colors in __git_ps1, then the solution
+#    is either to disable colors, or, in some shells which only care
+#    about the width of the last prompt line (e.g. busybox-ash),
+#    ensure the git output is not at the last line, maybe like so:
+#      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
 __git_printf_supports_v=
-- 
gitgitgadget

[PATCH v3 8/8] git-prompt: support custom 0-width PS1 markers

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

When using colors, the shell needs to identify 0-width substrings
in PS1 - such as color escape sequences - when calculating the
on-screen width of the prompt.

Until now, we used the form %F{<color>} in zsh - which it knows is
0-width, or otherwise use standard SGR esc sequences wrapped between
byte values 1 and 2 (SOH, STX) as 0-width start/end markers, which
bash/readline identify as such.

But now that more shells are supported, the standard SGR sequences
typically work, but the SOH/STX markers might not be identified.

This commit adds support for vars GIT_PS1_COLOR_{PRE,POST} which
set custom 0-width markers or disable the markers.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 6be2f1dd901..6186c474ba7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -129,11 +129,16 @@
 #    strings (SGR color sequences) when calculating the on-screen
 #    prompt width, to maintain correct input editing at the prompt.
 #
-#    Currently there's no support for different markers, so if editing
-#    behaves weird when using colors in __git_ps1, then the solution
-#    is either to disable colors, or, in some shells which only care
-#    about the width of the last prompt line (e.g. busybox-ash),
-#    ensure the git output is not at the last line, maybe like so:
+#    To replace or disable the 0-width markers, set GIT_PS1_COLOR_PRE
+#    and GIT_PS1_COLOR_POST to other markers, or empty (nul) to not
+#    use markers. For instance, some shells support '\[' and '\]' as
+#    start/end markers in PS1 - when invoking __git_ps1 with 3/4 args,
+#    but it may or may not work in command substitution mode. YMMV.
+#
+#    If the shell doesn't support 0-width markers and editing behaves
+#    incorrectly when using colors in __git_ps1, then, other than
+#    disabling color, it might be solved using multi-line prompt,
+#    where the git status is not at the last line, e.g.:
 #      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
@@ -309,8 +314,8 @@ __git_ps1_colorize_gitstring ()
 		# \001 (SOH) and \002 (STX) are 0-width substring markers
 		# which bash/readline identify while calculating the prompt
 		# on-screen width - to exclude 0-screen-width esc sequences.
-		local c_pre="${__git_SOH}${__git_ESC}["
-		local c_post="m${__git_STX}"
+		local c_pre="${GIT_PS1_COLOR_PRE-$__git_SOH}${__git_ESC}["
+		local c_post="m${GIT_PS1_COLOR_POST-$__git_STX}"
 
 		local c_red="${c_pre}31${c_post}"
 		local c_green="${c_pre}32${c_post}"
-- 
gitgitgadget

[PATCH v4 0/8] git-prompt: support more shells v4

[Avi Halachmi via GitGitGadget]

This addresses review comments on part 5/8 v3 (git-prompt: add some missing
quotes) to fix minor wording issues at the commit message.

Hopefully this is the last wording fixup.

Avi Halachmi (:avih) (8):
  git-prompt: use here-doc instead of here-string
  git-prompt: fix uninitialized variable
  git-prompt: don't use shell arrays
  git-prompt: replace [[...]] with standard code
  git-prompt: add some missing quotes
  git-prompt: don't use shell $'...'
  git-prompt: ta-da! document usage in other shells
  git-prompt: support custom 0-width PS1 markers

 contrib/completion/git-prompt.sh | 191 ++++++++++++++++++++-----------
 1 file changed, 126 insertions(+), 65 deletions(-)


base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1750%2Favih%2Fprompt-compat-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1750/avih/prompt-compat-v4
Pull-Request: https://github.com/git/git/pull/1750

Range-diff vs v3:

 1:  9ce5ddadf0b = 1:  9ce5ddadf0b git-prompt: use here-doc instead of here-string
 2:  680ecb52404 = 2:  680ecb52404 git-prompt: fix uninitialized variable
 3:  7e994eae7bc = 3:  7e994eae7bc git-prompt: don't use shell arrays
 4:  232340902a1 = 4:  232340902a1 git-prompt: replace [[...]] with standard code
 5:  3a41ad889cc ! 5:  18ff70db6b3 git-prompt: add some missing quotes
     @@ Commit message
          in real life, but the fixes improve correctness, and would prevent
          bugs in some uncommon cases, such as weird IFS values.
      
     -    Listing some portability guideline here for future reference.
     +    Listing some portability guidelines here for future reference.
      
          I'm leaving it to someone else to decide whether to include
     -    it in the file itself, place is as a new file, or not.
     +    it in the file itself, place it as a new file, or not.
      
          ---------
      
     @@ Commit message
            Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)
      
          The arguments to "local" do look like assignments, but they're not
     -    the assignment part of a simple command. they're at the command part.
     +    the assignment part of a simple command; they're at the command part.
      
          Still at the command part, no need to quote non-expandable values:
            Good:                 local x=   y=yes;   echo OK
 6:  e735a1696a0 = 6:  48aa31feedb git-prompt: don't use shell $'...'
 7:  e70440e669a = 7:  cd20b830b24 git-prompt: ta-da! document usage in other shells
 8:  633e71a01d3 = 8:  cb705d5fc8e git-prompt: support custom 0-width PS1 markers

-- 
gitgitgadget

[PATCH v4 1/8] git-prompt: use here-doc instead of here-string

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Here-documend is standard, and works in all shells.

Both here-string and here-doc add final newline, which is important
in this case, because $output is without final newline, but we do
want "read" to succeed on the last line as well.

Shells which support here-string:
- bash, zsh, mksh, ksh93, yash (non-posix-mode).

shells which don't, and got fixed:
- ash-derivatives (dash, free/net bsd sh, busybox-ash).
- pdksh, openbsd sh.
- All Schily Bourne shell variants.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5330e769a72..ebf2e30d684 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -137,7 +137,9 @@ __git_ps1_show_upstream ()
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
 		esac
-	done <<< "$output"
+	done <<-OUTPUT
+		$output
+	OUTPUT
 
 	# parse configuration values
 	local option
-- 
gitgitgadget

[PATCH v4 2/8] git-prompt: fix uninitialized variable

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

First use is in the form:  local var; ...; var=$var$whatever...

If the variable was unset (as bash and others do after "local x"),
then it would error if set -u is in effect.

Also, many shells inherit the existing value after "local var"
without init, but in this case it's unlikely to have a prior value.

Now we initialize it.

(local var= is enough, but local var="" is the custom in this file)

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index ebf2e30d684..4cc2cf91bb6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,7 +116,7 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern count n
+	local svn_remote svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
 
 	svn_remote=()
-- 
gitgitgadget

[PATCH v4 3/8] git-prompt: don't use shell arrays

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

Arrays only existed in the svn-upstream code, used to:
- Keep a list of svn remotes.
- Convert commit msg to array of words, extract the 2nd-to-last word.

Except bash/zsh, nearly all shells failed load on syntax errors here.

Now:
- The svn remotes are a list of newline-terminated values.
- The 2nd-to-last word is extracted using standard shell substrings.
- All shells can digest the svn-upstream code.

While using shell field splitting to extract the word is simple, and
doesn't even need non-standard code, e.g. set -- $(git log -1 ...),
it would have the same issues as the old array code: it depends on IFS
which we don't control, and it's subject to glob-expansion, e.g. if
the message happens to include * or **/* (as this commit message just
did), then the array could get huge. This was not great.

Now it uses standard shell substrings, and we know the exact delimiter
to expect, because it's the match from our grep just one line earlier.

The new word extraction code also fixes svn-upstream in zsh, because
previously it used arr[len-2], but because in zsh, unlike bash, array
subscripts are 1-based, it incorrectly extracted the 3rd-to-last word.
symptom: missing upstream status in a git-svn repo: u=, u+N-M, etc.

The breakage in zsh is surprising, because it was last touched by
  commit d0583da838 (prompt: fix show upstream with svn and zsh),
claiming to fix exactly that. However, it only mentions syntax fixes.
It's unclear if behavior was fixed too. But it was broken, now fixed.

Note LF=$'\n' and then using $LF instead of $'\n' few times.
A future commit will add fallback for shells without $'...', so this
would be the only line to touch instead of replacing every $'\n' .

Shells which could run the previous array code:
- bash

Shells which have arrays but were broken anyway:
- zsh: 1-based subscript
- ksh93: no "local" (the new code can't fix this part...)
- mksh, openbsd sh, pdksh: failed load on syntax error: "for ((...))".

More shells which Failed to load due to syntax error:
- dash, free/net bsd sh, busybox-ash, Schily Bourne shell, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 48 ++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4cc2cf91bb6..75c3a813fda 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -116,10 +116,10 @@ printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 __git_ps1_show_upstream ()
 {
 	local key value
-	local svn_remote svn_url_pattern="" count n
+	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
+	local LF=$'\n'
 
-	svn_remote=()
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
 	while read -r key value; do
@@ -132,7 +132,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
+			svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
 			svn_url_pattern="$svn_url_pattern\\|$value"
 			upstream_type=svn+git # default upstream type is SVN if available, else git
 			;;
@@ -156,25 +156,37 @@ __git_ps1_show_upstream ()
 	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)
-		local -a svn_upstream
-		svn_upstream=($(git log --first-parent -1 \
-					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
-		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-			svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
-			svn_upstream=${svn_upstream%@*}
-			local n_stop="${#svn_remote[@]}"
-			for ((n=1; n <= n_stop; n++)); do
-				svn_upstream=${svn_upstream#${svn_remote[$n]}}
-			done
+		# successful svn-upstream resolution:
+		# - get the list of configured svn-remotes ($svn_remotes set above)
+		# - get the last commit which seems from one of our svn-remotes
+		# - confirm that it is from one of the svn-remotes
+		# - use $GIT_SVN_ID if set, else "git-svn"
 
-			if [[ -z "$svn_upstream" ]]; then
+		# get upstream from "git-svn-id: UPSTRM@N HASH" in a commit message
+		# (git-svn uses essentially the same procedure internally)
+		local svn_upstream="$(
+			git log --first-parent -1 \
+				--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null
+		)"
+
+		if [ -n "$svn_upstream" ]; then
+			# extract the URI, assuming --grep matched the last line
+			svn_upstream=${svn_upstream##*$LF}  # last line
+			svn_upstream=${svn_upstream#*: }    # UPSTRM@N HASH
+			svn_upstream=${svn_upstream%@*}     # UPSTRM
+
+			case ${LF}${svn_remotes} in
+			*"${LF}${svn_upstream}${LF}"*)
+				# grep indeed matched the last line - it's our remote
 				# default branch name for checkouts with no layout:
 				upstream_type=${GIT_SVN_ID:-git-svn}
-			else
+				;;
+			*)
+				# the commit message includes one of our remotes, but
+				# it's not at the last line. is $svn_upstream junk?
 				upstream_type=${svn_upstream#/}
-			fi
+				;;
+			esac
 		elif [[ "svn+git" = "$upstream_type" ]]; then
 			upstream_type="@{upstream}"
 		fi
-- 
gitgitgadget

[PATCH v4 4/8] git-prompt: replace [[...]] with standard code

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The existing [[...]] tests were either already valid as standard [...]
tests, or only required minimal retouch:

Notes:

- [[...]] doesn't do field splitting and glob expansion, so $var
  or $(cmd...) don't need quoting, but [... does need quotes.

- [[ X == Y ]] when Y is a string is same as [ X = Y ], but if Y is
  a pattern, then we need:  case X in Y)... ; esac  .

- [[ ... && ... ]] was replaced with [ ... ] && [ ... ] .

- [[ -o <zsh-option> ]] requires [[...]], so put it in "eval" and only
  eval it in zsh, so other shells would not abort on syntax error
  (posix says [[ has unspecified results, shells allowed to reject it)

- ((x++)) was changed into x=$((x+1))  (yeah, not [[...]] ...)

Shells which accepted the previous forms:
- bash, zsh, ksh93, mksh, openbsd sh, pdksh.

Shells which didn't, and now can process it:
- dash, free/net bsd sh, busybox-ash, Schily Bourne sh, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 75c3a813fda..4781261f868 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -126,7 +126,7 @@ __git_ps1_show_upstream ()
 		case "$key" in
 		bash.showupstream)
 			GIT_PS1_SHOWUPSTREAM="$value"
-			if [[ -z "${GIT_PS1_SHOWUPSTREAM}" ]]; then
+			if [ -z "${GIT_PS1_SHOWUPSTREAM}" ]; then
 				p=""
 				return
 			fi
@@ -187,14 +187,14 @@ __git_ps1_show_upstream ()
 				upstream_type=${svn_upstream#/}
 				;;
 			esac
-		elif [[ "svn+git" = "$upstream_type" ]]; then
+		elif [ "svn+git" = "$upstream_type" ]; then
 			upstream_type="@{upstream}"
 		fi
 		;;
 	esac
 
 	# Find how many commits we are ahead/behind our upstream
-	if [[ -z "$legacy" ]]; then
+	if [ -z "$legacy" ]; then
 		count="$(git rev-list --count --left-right \
 				"$upstream_type"...HEAD 2>/dev/null)"
 	else
@@ -206,8 +206,8 @@ __git_ps1_show_upstream ()
 			for commit in $commits
 			do
 				case "$commit" in
-				"<"*) ((behind++)) ;;
-				*)    ((ahead++))  ;;
+				"<"*) behind=$((behind+1)) ;;
+				*)    ahead=$((ahead+1))   ;;
 				esac
 			done
 			count="$behind	$ahead"
@@ -217,7 +217,7 @@ __git_ps1_show_upstream ()
 	fi
 
 	# calculate the result
-	if [[ -z "$verbose" ]]; then
+	if [ -z "$verbose" ]; then
 		case "$count" in
 		"") # no upstream
 			p="" ;;
@@ -243,7 +243,7 @@ __git_ps1_show_upstream ()
 		*)	    # diverged from upstream
 			upstream="|u+${count#*	}-${count%	*}" ;;
 		esac
-		if [[ -n "$count" && -n "$name" ]]; then
+		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
@@ -265,7 +265,7 @@ __git_ps1_show_upstream ()
 # their own color.
 __git_ps1_colorize_gitstring ()
 {
-	if [[ -n ${ZSH_VERSION-} ]]; then
+	if [ -n "${ZSH_VERSION-}" ]; then
 		local c_red='%F{red}'
 		local c_green='%F{green}'
 		local c_lblue='%F{blue}'
@@ -417,7 +417,7 @@ __git_ps1 ()
 	# incorrect.)
 	#
 	local ps1_expanded=yes
-	[ -z "${ZSH_VERSION-}" ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+	[ -z "${ZSH_VERSION-}" ] || eval '[[ -o PROMPT_SUBST ]]' || ps1_expanded=no
 	[ -z "${BASH_VERSION-}" ] || shopt -q promptvars || ps1_expanded=no
 
 	local repo_info rev_parse_exit_code
@@ -502,11 +502,13 @@ __git_ps1 ()
 					return $exit
 				fi
 
-				if [[ $head == "ref: "* ]]; then
+				case $head in
+				"ref: "*)
 					head="${head#ref: }"
-				else
+					;;
+				*)
 					head=""
-				fi
+				esac
 				;;
 			*)
 				head="$(git symbolic-ref HEAD 2>/dev/null)"
@@ -542,8 +544,8 @@ __git_ps1 ()
 	fi
 
 	local conflict="" # state indicator for unresolved conflicts
-	if [[ "${GIT_PS1_SHOWCONFLICTSTATE-}" == "yes" ]] &&
-	   [[ $(git ls-files --unmerged 2>/dev/null) ]]; then
+	if [ "${GIT_PS1_SHOWCONFLICTSTATE-}" = "yes" ] &&
+	   [ "$(git ls-files --unmerged 2>/dev/null)" ]; then
 		conflict="|CONFLICT"
 	fi
 
-- 
gitgitgadget

[PATCH v4 5/8] git-prompt: add some missing quotes

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

The issues which this commit fixes are unlikely to be broken
in real life, but the fixes improve correctness, and would prevent
bugs in some uncommon cases, such as weird IFS values.

Listing some portability guidelines here for future reference.

I'm leaving it to someone else to decide whether to include
it in the file itself, place it as a new file, or not.

---------

The command "local" is non standard, but is allowed in this file:
- Quote initialization if it can expand (local x="$y"). See below.
- Don't assume initial value after "local x". Either initialize it
  (local x=..), or set before first use (local x;.. x=..; <use $x>).
  (between shells, "local x" can unset x, or inherit it, or do x= )

Other non-standard features beyond "local" are to be avoided.

Use the standard "test" - [...] instead of non-standard [[...]] .

--------

Quotes (some portability things, but mainly general correctness):

Quotes prevent tilde-expansion of some unquoted literal tildes (~).
If the expansion is undesirable, quotes would ensure that.
  Tilds expanded: a=~user:~/ ;  echo ~user ~/dir
  not expanded:   t="~"; a=${t}user  b=\~foo~;  echo "~user" $t/dir

But the main reason for quoting is to prevent IFS field splitting
(which also coalesces IFS chars) and glob expansion in parts which
contain parameter/arithmetic expansion or command substitution.

"Simple command" (POSIX term) is assignment[s] and/or command [args].
Examples:
  foo=bar         # one assignment
  foo=$bar x=y    # two assignments
  foo bar         # command, no assignments
  x=123 foo bar   # one assignment and a command

The assignments part is not IFS-split or glob-expanded.

The command+args part does get IFS field split and glob expanded,
but only at unquoted expanded/substituted parts.

In the command+args part, expanded/substituted values must be quoted.
(the commands here are "[" and "local"):
  Good: [ "$mode" = yes ]; local s="*" x="$y" e="$?" z="$(cmd ...)"
  Bad:  [ $mode = yes ];   local s=*   x=$y   e=$?   z=$(cmd...)

The arguments to "local" do look like assignments, but they're not
the assignment part of a simple command; they're at the command part.

Still at the command part, no need to quote non-expandable values:
  Good:                 local x=   y=yes;   echo OK
  OK, but not required: local x="" y="yes"; echo "OK"
But completely empty (NULL) arguments must be quoted:
  foo ""   is not the same as:   foo

Assignments in simple commands - with or without an actual command,
don't need quoting becase there's no IFS split or glob expansion:
  Good:   s=* a=$b c=$(cmd...)${x# foo }${y-   } [cmd ...]
  It's also OK to use double quotes, but not required.

This behavior (no IFS/glob) is called "assignment context", and
"local" does not behave with assignment context in some shells,
hence we require quotes when using "local" - for compatibility.

The value between 'case' and 'in' doesn't IFS-split/glob-expand:
  Good:       case  * $foo $(cmd...)  in ... ; esac
  identical:  case "* $foo $(cmd...)" in ... ; esac

Nested quotes in command substitution are fine, often necessary:
  Good: echo "$(foo... "$x" "$(bar ...)")"

Nested quotes in substring ops are legal, and sometimes needed
to prevent interpretation as a pattern, but not the most readable:
  Legal:  foo "${x#*"$y" }"

Nested quotes in "maybe other value" subst are invalid, unnecessary:
  Good:  local x="${y- }";   foo "${z:+ $a }"
  Bad:   local x="${y-" "}"; foo "${z:+" $a "}"
Outer/inner quotes in "maybe other value" have different use cases:
  "${x-$y}"  always one quoted arg: "$x" if x is set, else "$y".
  ${x+"$x"}  one quoted arg "$x" if x is set, else no arg at all.
  Unquoted $x is similar to the second case, but it would get split
  into few arguments if it includes any of the IFS chars.

Assignments don't need the outer quotes, and the braces delimit the
value, so nested quotes can be avoided, for readability:
  a=$(foo "$x")  a=${x#*"$y" }  c=${y- };  bar "$a" "$b" "$c"

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.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 4781261f868..5d7f236fe48 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -246,7 +246,7 @@ __git_ps1_show_upstream ()
 		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
+			if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 				upstream="$upstream \${__git_ps1_upstream_name}"
 			else
 				upstream="$upstream ${__git_ps1_upstream_name}"
@@ -278,12 +278,12 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue=$'\001\e[1;34m\002'
 		local c_clear=$'\001\e[0m\002'
 	fi
-	local bad_color=$c_red
-	local ok_color=$c_green
+	local bad_color="$c_red"
+	local ok_color="$c_green"
 	local flags_color="$c_lblue"
 
 	local branch_color=""
-	if [ $detached = no ]; then
+	if [ "$detached" = no ]; then
 		branch_color="$ok_color"
 	else
 		branch_color="$bad_color"
@@ -360,7 +360,7 @@ __git_sequencer_status ()
 __git_ps1 ()
 {
 	# preserve exit status
-	local exit=$?
+	local exit="$?"
 	local pcmode=no
 	local detached=no
 	local ps1pc_start='\u@\h:\w '
@@ -379,7 +379,7 @@ __git_ps1 ()
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
-		*)	return $exit
+		*)	return "$exit"
 		;;
 	esac
 
@@ -427,7 +427,7 @@ __git_ps1 ()
 	rev_parse_exit_code="$?"
 
 	if [ -z "$repo_info" ]; then
-		return $exit
+		return "$exit"
 	fi
 
 	local short_sha=""
@@ -449,7 +449,7 @@ __git_ps1 ()
 	   [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
 	   git check-ignore -q .
 	then
-		return $exit
+		return "$exit"
 	fi
 
 	local sparse=""
@@ -499,7 +499,7 @@ __git_ps1 ()
 			case "$ref_format" in
 			files)
 				if ! __git_eread "$g/HEAD" head; then
-					return $exit
+					return "$exit"
 				fi
 
 				case $head in
@@ -597,10 +597,10 @@ __git_ps1 ()
 		fi
 	fi
 
-	local z="${GIT_PS1_STATESEPARATOR-" "}"
+	local z="${GIT_PS1_STATESEPARATOR- }"
 
 	b=${b##refs/heads/}
-	if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
+	if [ "$pcmode" = yes ] && [ "$ps1_expanded" = yes ]; then
 		__git_ps1_branch_name=$b
 		b="\${__git_ps1_branch_name}"
 	fi
@@ -612,7 +612,7 @@ __git_ps1 ()
 	local f="$h$w$i$s$u$p"
 	local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}${conflict}"
 
-	if [ $pcmode = yes ]; then
+	if [ "$pcmode" = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then
 			gitstring=$(printf -- "$printf_format" "$gitstring")
 		else
@@ -623,5 +623,5 @@ __git_ps1 ()
 		printf -- "$printf_format" "$gitstring"
 	fi
 
-	return $exit
+	return "$exit"
 }
-- 
gitgitgadget

[PATCH v4 6/8] git-prompt: don't use shell $'...'

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

$'...' is new in POSIX (2024), and some shells support it in recent
versions, while others have had it for decades (bash, zsh, ksh93).

However, there are still enough shells which don't support it, and
it's cheap to use an alternative form which works in all shells,
so let's do that instead of dismissing it as "it's compliant".

It was agreed to use one form rather than $'...' where supported and
fallback otherwise.

shells where $'...' works:
- bash, zsh, ksh93, mksh, busybox-ash, dash master, free/net bsd sh.

shells where it doesn't work, but the new fallback works:
- all dash releases (up to 0.5.12), older versions of free/net bsd sh,
  openbsd sh, pdksh, all Schily Bourne sh variants, yash.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 47 ++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 5d7f236fe48..c3dd38f847c 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -111,6 +111,12 @@
 __git_printf_supports_v=
 printf -v __git_printf_supports_v -- '%s' yes >/dev/null 2>&1
 
+# like __git_SOH=$'\001' etc but works also in shells without $'...'
+eval "$(printf '
+	__git_SOH="\001" __git_STX="\002" __git_ESC="\033"
+	__git_LF="\n" __git_CRLF="\r\n"
+')"
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
@@ -118,7 +124,7 @@ __git_ps1_show_upstream ()
 	local key value
 	local svn_remotes="" svn_url_pattern="" count n
 	local upstream_type=git legacy="" verbose="" name=""
-	local LF=$'\n'
+	local LF="$__git_LF"
 
 	# get some config options from git-config
 	local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
@@ -271,12 +277,16 @@ __git_ps1_colorize_gitstring ()
 		local c_lblue='%F{blue}'
 		local c_clear='%f'
 	else
-		# Using \001 and \002 around colors is necessary to prevent
-		# issues with command line editing/browsing/completion!
-		local c_red=$'\001\e[31m\002'
-		local c_green=$'\001\e[32m\002'
-		local c_lblue=$'\001\e[1;34m\002'
-		local c_clear=$'\001\e[0m\002'
+		# \001 (SOH) and \002 (STX) are 0-width substring markers
+		# which bash/readline identify while calculating the prompt
+		# on-screen width - to exclude 0-screen-width esc sequences.
+		local c_pre="${__git_SOH}${__git_ESC}["
+		local c_post="m${__git_STX}"
+
+		local c_red="${c_pre}31${c_post}"
+		local c_green="${c_pre}32${c_post}"
+		local c_lblue="${c_pre}1;34${c_post}"
+		local c_clear="${c_pre}0${c_post}"
 	fi
 	local bad_color="$c_red"
 	local ok_color="$c_green"
@@ -312,7 +322,7 @@ __git_ps1_colorize_gitstring ()
 # variable, in that order.
 __git_eread ()
 {
-	test -r "$1" && IFS=$'\r\n' read -r "$2" <"$1"
+	test -r "$1" && IFS=$__git_CRLF read -r "$2" <"$1"
 }
 
 # see if a cherry-pick or revert is in progress, if the user has committed a
@@ -430,19 +440,20 @@ __git_ps1 ()
 		return "$exit"
 	fi
 
+	local LF="$__git_LF"
 	local short_sha=""
 	if [ "$rev_parse_exit_code" = "0" ]; then
-		short_sha="${repo_info##*$'\n'}"
-		repo_info="${repo_info%$'\n'*}"
+		short_sha="${repo_info##*$LF}"
+		repo_info="${repo_info%$LF*}"
 	fi
-	local ref_format="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_worktree="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local bare_repo="${repo_info##*$'\n'}"
-	repo_info="${repo_info%$'\n'*}"
-	local inside_gitdir="${repo_info##*$'\n'}"
-	local g="${repo_info%$'\n'*}"
+	local ref_format="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_worktree="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local bare_repo="${repo_info##*$LF}"
+	repo_info="${repo_info%$LF*}"
+	local inside_gitdir="${repo_info##*$LF}"
+	local g="${repo_info%$LF*}"
 
 	if [ "true" = "$inside_worktree" ] &&
 	   [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED-}" ] &&
-- 
gitgitgadget

[PATCH v4 7/8] git-prompt: ta-da! document usage in other shells

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

With one big exception, git-prompt.sh should now be both almost posix
compliant, and also compatible with most (posix-ish) shells.

That exception is the use of "local" vars in functions, which happens
extensively in the current code, and is not simple to replace with
posix compliant code (but also not impossible).

Luckily, almost all shells support "local" as used by the current
code, with the notable exception of ksh93[u+m], but also the Schily
minimal posix sh (pbosh), and yash in posix mode.

See assessment below that "local" is likely the only blocker in those.

So except mainly ksh93, git-prompt.sh now works in most shells:
- bash, zsh, dash since at least 0.5.8, free/net bsd sh, busybox-ash,
  mksh, openbsd sh, pdksh(!), Schily extended Bourne sh (bosh), yash.

which is quite nice.

As an anecdote, replacing the 1st line in __git_ps1() (local exit=$?)
with these 2 makes it work in all tested shells, even without "local":

  # handles only 0/1 args for simplicity. needs +5 LOC for any $#
  __git_e=$?; local exit="$__git_e" 2>/dev/null ||
    {(eval 'local() { export "$@"; }'; __git_ps1 "$@"); return "$__git_e"; }

Explanation:

  If the shell doesn't have the command "local", define our own
  function "local" which instead does plain (global) assignents.
  Then use __git_ps1 in a subshell to not clober the caller's vars.

  This happens to work because currently there are no name conflicts
  (shadow) at the code, initial value is not assumed (i.e. always
  doing either 'local x=...'  or 'local x;...  x=...'), and assigned
  initial values are quoted (local x="$y"), preventing word split and
  glob expansion (i.e. assignment context is not assumed).

  The last two (always init, quote values) seem to be enough to use
  "local" portably if supported, and otherwise shells indeed differ.

  Uses "eval", else shells with "local" may reject it during parsing.
  We don't need "export", but it's smaller than writing our own loop.

While cute, this approach is not really sustainable because all the
vars become global, which is hard to maintain without conflicts
(but hey, it currently has no conflicts - without even trying...).

However, regardless of being an anecdote, it provides some support to
the assessment that "local" is the only blocker in those shells.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 33 ++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c3dd38f847c..6be2f1dd901 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -8,8 +8,8 @@
 # To enable:
 #
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
-#    2) Add the following line to your .bashrc/.zshrc:
-#        source ~/.git-prompt.sh
+#    2) Add the following line to your .bashrc/.zshrc/.profile:
+#        . ~/.git-prompt.sh   # dot path/to/this-file
 #    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
@@ -30,6 +30,8 @@
 #        Optionally, you can supply a third argument with a printf
 #        format string to finetune the output of the branch status
 #
+#    See notes below about compatibility with other shells.
+#
 # The repository status will be displayed only if you are currently in a
 # git repository. The %s token is the placeholder for the shown status.
 #
@@ -106,6 +108,33 @@
 # directory is set up to be ignored by git, then set
 # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
 # repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# Compatibility with other shells (beyond bash/zsh):
+#
+#    We require posix-ish shell plus "local" support, which is most
+#    shells (even pdksh), but excluding ksh93 (because no "local").
+#
+#    Prompt integration might differ between shells, but the gist is
+#    to load it once on shell init with '. path/to/git-prompt.sh',
+#    set GIT_PS1* vars once as needed, and either place $(__git_ps1..)
+#    inside PS1 once (0/1 args), or, before each prompt is displayed,
+#    call __git_ps1 (2/3 args) which sets PS1 with the status embedded.
+#
+#    Many shells support the 1st method of command substitution,
+#    though some might need to first enable cmd substitution in PS1.
+#
+#    When using colors, each escape sequence is wrapped between byte
+#    values 1 and 2 (control chars SOH, STX, respectively), which are
+#    invisible at the output, but for bash/readline they mark 0-width
+#    strings (SGR color sequences) when calculating the on-screen
+#    prompt width, to maintain correct input editing at the prompt.
+#
+#    Currently there's no support for different markers, so if editing
+#    behaves weird when using colors in __git_ps1, then the solution
+#    is either to disable colors, or, in some shells which only care
+#    about the width of the last prompt line (e.g. busybox-ash),
+#    ensure the git output is not at the last line, maybe like so:
+#      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
 __git_printf_supports_v=
-- 
gitgitgadget

[PATCH v4 8/8] git-prompt: support custom 0-width PS1 markers

[Avi Halachmi (:avih) via GitGitGadget]

From: "Avi Halachmi (:avih)" <avihpit@yahoo.com>

When using colors, the shell needs to identify 0-width substrings
in PS1 - such as color escape sequences - when calculating the
on-screen width of the prompt.

Until now, we used the form %F{<color>} in zsh - which it knows is
0-width, or otherwise use standard SGR esc sequences wrapped between
byte values 1 and 2 (SOH, STX) as 0-width start/end markers, which
bash/readline identify as such.

But now that more shells are supported, the standard SGR sequences
typically work, but the SOH/STX markers might not be identified.

This commit adds support for vars GIT_PS1_COLOR_{PRE,POST} which
set custom 0-width markers or disable the markers.

Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
---
 contrib/completion/git-prompt.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 6be2f1dd901..6186c474ba7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -129,11 +129,16 @@
 #    strings (SGR color sequences) when calculating the on-screen
 #    prompt width, to maintain correct input editing at the prompt.
 #
-#    Currently there's no support for different markers, so if editing
-#    behaves weird when using colors in __git_ps1, then the solution
-#    is either to disable colors, or, in some shells which only care
-#    about the width of the last prompt line (e.g. busybox-ash),
-#    ensure the git output is not at the last line, maybe like so:
+#    To replace or disable the 0-width markers, set GIT_PS1_COLOR_PRE
+#    and GIT_PS1_COLOR_POST to other markers, or empty (nul) to not
+#    use markers. For instance, some shells support '\[' and '\]' as
+#    start/end markers in PS1 - when invoking __git_ps1 with 3/4 args,
+#    but it may or may not work in command substitution mode. YMMV.
+#
+#    If the shell doesn't support 0-width markers and editing behaves
+#    incorrectly when using colors in __git_ps1, then, other than
+#    disabling color, it might be solved using multi-line prompt,
+#    where the git status is not at the last line, e.g.:
 #      PS1='\n\w \u@\h$(__git_ps1 " (%s)")\n\$ '
 
 # check whether printf supports -v
@@ -309,8 +314,8 @@ __git_ps1_colorize_gitstring ()
 		# \001 (SOH) and \002 (STX) are 0-width substring markers
 		# which bash/readline identify while calculating the prompt
 		# on-screen width - to exclude 0-screen-width esc sequences.
-		local c_pre="${__git_SOH}${__git_ESC}["
-		local c_post="m${__git_STX}"
+		local c_pre="${GIT_PS1_COLOR_PRE-$__git_SOH}${__git_ESC}["
+		local c_post="m${GIT_PS1_COLOR_POST-$__git_STX}"
 
 		local c_red="${c_pre}31${c_post}"
 		local c_green="${c_pre}32${c_post}"
-- 
gitgitgadget

Re: [PATCH v4 0/8] git-prompt: support more shells v4

[Junio C Hamano]

"Avi Halachmi via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This addresses review comments on part 5/8 v3 (git-prompt: add some missing
> quotes) to fix minor wording issues at the commit message.

Good.  This exactly matches what has been queued in 'seen', as I've
fixed these typoes locally while queueing.

> Hopefully this is the last wording fixup.

;-)  Let me mark the topic for 'next' in a few days, then.

Thanks.

Re: [PATCH v4 0/8] git-prompt: support more shells v4

[avih]

 On Tuesday, August 20, 2024 at 06:32:55 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
"Avi Halachmi via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> This addresses review comments on part 5/8 v3 (git-prompt: add some missing
>> quotes) to fix minor wording issues at the commit message.
>
> Good.  This exactly matches what has been queued in 'seen', as I've
> fixed these typoes locally while queueing.

Ouch... Indeed I didn't check whether "seen" includes those fixups.
You're testing me ;)

>> Hopefully this is the last wording fixup.
>
> ;-)  Let me mark the topic for 'next' in a few days, then.

Thanks!

Re: [PATCH v4 0/8] git-prompt: support more shells v4

[avih]

 Thanks for merging the git-prompt portability improvements into
master, and for coordinating the development of git all those years!

I probably won't be following the git mailing list closely, but do
feel free to email or CC me with any question or other comments,
either specically about git-prompt, or anything else you think I
might be able to help with (I'm guessing mainly shell-related).

Best regards,

Avi Halachmi