Re: [PATCH 6/8] git-prompt: add fallback for shells without $'...'
- From
avih <avihpit@yahoo.com>
- Date
- Jul 24, 2024, 02:08 UTC
- Message-ID
- <1542063589.2363688.1721786934049@mail.yahoo.com>
- In-Reply-To
- <xmqqy15rzwi5.fsf@gitster.g>
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.
Show 10 quoted lines
> > +__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.