mbox series

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

Message ID pull.1750.v2.git.git.1723727653.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series git-prompt: support more shells v2 | expand

Message

Jean-Noël Avila via GitGitGadget Aug. 15, 2024, 1:14 p.m. UTC
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

Comments

Junio C Hamano Aug. 15, 2024, 6:48 p.m. UTC | #1
"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.
Patrick Steinhardt Aug. 16, 2024, 8:50 a.m. UTC | #2
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