| Message ID | 232340902a1feeafe526528eb88b8d0814d11545.1723727653.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fe445a1026562dfeb4fa1b0ee93bdfe3b7422251 |
| Headers | show |
| Series | git-prompt: support more shells v2 | expand |
"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.
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?)
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.
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