git-prompt: support more shells v4 #1750
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
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>
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 d0583da (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>
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>
Welcome to GitGitGadgetHi @avih, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax: NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description): To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
|
/allow |
|
User avih is now allowed to use GitGitGadget. WARNING: avih has no public email address set on GitHub; |
|
/submit |
|
Error: Could not determine full name of avih |
|
/submit |
|
Submitted as pull.1750.git.git.1721762306.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget-git[bot]](https://avatars.githubusercontent.com/in/46807?s=60&v=4){kind=small}
| @@ -229,10 +243,10 @@ __git_ps1_show_upstream () | |||
| *) # diverged from upstream | |||
| upstream="|u+${count#* }-${count% *}" ;; | |||
| esac | |||
| if [[ -n "$count" && -n "$name" ]]; then | |||
| if [ -n "$count" ] && [ -n "$name" ]; then | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"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.
There was a problem hiding this comment.
Thank you for the quick reply.
I think we have something to say about local var="val" construct to help dash.
I was not aware of Documentation/CodingGuidelines, although I should have been, so thanks for pointing it out.
As far as I can tell, CodingGuidelines and my "portability guidelines" align completely for subjects which appear in both places, although each source also mentions few things which the other doesn't.
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.
Well, there's no bashism in it after this patchset, and the scope of non-standard allowance is pretty much identical as CodingGuidelines allows (posix, plus "local" with the same compatibility requirements), and which is also described my own "portability guidelines" commit message.
However, I didn't change style ("test" instead of [...], "then" in a new line, etc), because it wasn't broken as far as compatibility or correctness goes, and I considered it out of scope for this commit (but also wasn't aware of CodingGuidelines).
So the style in this file is the same as before, which means some divergence from CodingGuidelines.
But as far as everything else, I think it does adhere to CodingGuidelines.
Do you want me to do anything specific on this subject?
Shall I remove the "portability guidelines" part from the commit message?
There was a problem hiding this comment.
@avih unfortunately, PR comments are not bidirectional. Please follow this guidance to reply to the email.
There was a problem hiding this comment.
Oh.. oops...
Thanks for the heads up. I'll follow these links a bit later.
There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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?
| @@ -111,33 +111,46 @@ | |||
| __git_printf_supports_v= | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"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.
There was a problem hiding this comment.
I will not look at this series futher during the current development cycle that is about to conclude
Sure, I did not try to time this patchset. Whenever someone has the bandwidth to handle it, it's fine by me. We're not in a hurry.
(and I'd assume the same about the reply to my comments below)
Just forbid use of $'\octal' notation in the coding guidelines document, and implement just one variant.
Yeah. That's fair.
But I'd prefer to examine existing cases of $'...' at the git code base, if there are such, before deciding on a policy. I did not look at existing git scripts.
The two subjects I'd care about are performance and maintinability. I don't know if we want every script do this juggling of one printf and then extracting values from it, and even a single additional command substitution in a hot script can be non-negligible.
Depending on how the scripting ecosystem looks like, it might be most managable (maybe also most performant) to have a single file which sets C0 control chars values __git_001, __git_002 ... __git_037 either literally or however else we decide, then any script which needs control literals can dot it, or embed it, etc. The point is to not have every script reinventing the wheel.
I will update this commit to remove the fallback system (not right away, I prefer to wait for hoepfully more comments)
There was a problem hiding this comment.
@avih please send this reply as an email instead. GitGitGadget is not mirroring emails <-> PR comments bidirectionally. It only mirrors emails -> PR comments, not vice versa.
There was a problem hiding this comment.
@avih please send this reply as an email instead
Yes, I understood and replied to the 1st time you wrote it above. But I already replied to both messages when I saw it.
Thanks again.
There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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.There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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.
There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
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.
There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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?
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
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.
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): 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 |
|
User |
|
On the Git mailing list, avih wrote (reply to this): 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. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): 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.
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): 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. |
|
On the Git mailing list, avih wrote (reply to this): 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.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
avih
changed the title
|
/submit |
|
Submitted as pull.1750.v2.git.git.1723727653.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
|
||
| 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 | ||
| case "$key" in |
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"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.There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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?)
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
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.
| @@ -229,10 +243,10 @@ __git_ps1_show_upstream () | |||
| *) # diverged from upstream | |||
| upstream="|u+${count#* }-${count% *}" ;; | |||
| esac | |||
| if [[ -n "$count" && -n "$name" ]]; then | |||
| if [ -n "$count" ] && [ -n "$name" ]; then | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"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.There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
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?
There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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).
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "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. |
| @@ -137,7 +137,9 @@ __git_ps1_show_upstream () | |||
| upstream_type=svn+git # default upstream type is SVN if available, else git | |||
There was a problem hiding this comment.
On the Git mailing list, Patrick Steinhardt wrote (reply to this):
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
>
> There was a problem hiding this comment.
On the Git mailing list, avih wrote (reply to this):
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.There was a problem hiding this comment.
On the Git mailing list, Patrick Steinhardt wrote (reply to this):
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
|
User |
$'...' 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>
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>
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>
avih
changed the title
|
/submit |
|
Submitted as pull.1750.v4.git.git.1724118513.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "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. |
|
On the Git mailing list, avih wrote (reply to this): 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!
|
|
This patch series was integrated into seen via ca3252b. |
|
This patch series was integrated into seen via fff8c50. |
|
This patch series was integrated into seen via f1bd901. |
|
This patch series was integrated into next via 14fa411. |
|
There was a status update in the "Cooking" section about the branch The command line prompt support used to be littered with bash-isms, which has been corrected to work with more shells. Will merge to 'master'. source: <pull.1750.v4.git.git.1724118513.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via 3cd304c. |
|
This patch series was integrated into seen via 4045a5b. |
|
There was a status update in the "Cooking" section about the branch The command line prompt support used to be littered with bash-isms, which has been corrected to work with more shells. Will merge to 'master'. source: <pull.1750.v4.git.git.1724118513.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via d0ef1bb. |
|
This patch series was integrated into seen via 4acb02e. |
|
This patch series was integrated into seen via a9f47d2. |
|
There was a status update in the "Cooking" section about the branch The command line prompt support used to be littered with bash-isms, which has been corrected to work with more shells. Will merge to 'master'. source: <pull.1750.v4.git.git.1724118513.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via d19863b. |
|
This patch series was integrated into master via d19863b. |
|
This patch series was integrated into next via d19863b. |
|
Closed via d19863b. |
|
On the Git mailing list, avih wrote (reply to this): 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
cc: "Junio C. Hamano" gitster@pobox.com
cc: "brian m. carlson" sandals@crustytoothpaste.net
cc: Patrick Steinhardt ps@pks.im
cc: Eric Sunshine sunshine@sunshineco.com