Update git-prompt.sh for Zsh Error #631
Conversation
Howdy! Problem If Zsh ever runs /etc/profile.d/git-prompts.sh, then zsh will also run git-completions.bash. git-completions.bash throws an Error when Zsh is detected. This Error echoes this message: "ERROR: this script is obsolete, please see git-completion.zsh" This Error breaks certain git completions and functionality. Solution Add condition to git-prompt.sh to avoid git-completion.bash if Zsh is detected. Add prompt that enables git prompts to work. Note: Git completions will still work for Bash and Zsh. Prompts with "__git_ps1" will still work for Bash and Zsh. Signed-off-by: Javier S. Orejarena <[email protected]>
Signed-off-by: Javier S. Orejarena <[email protected]>
Signed-off-by: Javier S. Orejarena <[email protected]>
Signed-off-by: Javier S. Orejarena <[email protected]>
Signed-off-by: Javier S. Orejarena <[email protected]>
|
/updpkgsums The workflow run was started. |
Signed-off-by: Matthias Aßhauer <[email protected]>
|
@JaviAir The build failures aren't due to issues in your script, but because the Feel free to force push additional changes over my commit, I'll try to look at this PR throughout the day and have the bot update the file again. |
|
@rimrul Thank you! Is there anything else I should do for this PR? |
There was a problem hiding this comment.
I have serious concerns about the shape of the commits. They reflect the (quite messy, and unhelpful to the reader) chronological development of the changes. I do not need to see every attempt you made to make this work (even though there are admittedly times when it is educative to describe in the commit message e.g. when an attempted approach failed to work unexpectedly and potential reviewers might be curious why it had not been attempted, when it had been).
A much more helpful way to represent the changes would be to squash them into combined commits as needed and then accompany them with stellar commit messages that closely follow the advice given in https://github.blog/2022-06-30-write-better-commits-build-better-projects/, focusing on:
| What you’re doing | Why you’re doing it | |
|---|---|---|
| High-level (strategic) | Intent (what does this accomplish?) | Context (why does the code do what it does now?) |
| Low-level (tactical) | Implementation (what did you do to accomplish your goal?) | Justification (why is this change being made?) |
One of the things I am missing in particular is a justification why this change should be accepted into Git for Windows when it does not even distribute a zsh, but only a Bash.
git-extra/git-prompt.sh
Outdated
| # newline=' | ||
| # ' |
There was a problem hiding this comment.
Does this need to be commented out? I would think that it doesn't work, then, it is used in the precmd below.
There was a problem hiding this comment.
Correct, it was initially not commented.
It was only commented to see if that was causing the build to fail earlier.
Thankfully rimrul helped explain what was going on.
I have now un-commented this. 👍
git-extra/git-prompt.sh
Outdated
| @@ -25,8 +29,18 @@ else | |||
| COMPLETION_PATH="$COMPLETION_PATH/share/git/completion" | |||
| if test -f "$COMPLETION_PATH/git-prompt.sh" | |||
| then | |||
| . "$COMPLETION_PATH/git-prompt.sh" | |||
| if [ ! "x${ZSH_VERSION}" = "x" ]; | |||
There was a problem hiding this comment.
Have a careful look at the code style used in this script, e.g. 3 lines up: It uses test, not [ ... ], the trailing ; is unwanted, and the condition dearly desires to be not so uncomplicated ;-)
| if [ ! "x${ZSH_VERSION}" = "x" ]; | |
| if test -n "$ZSH_VERSION" |
There was a problem hiding this comment.
I agree, this syntax is more legible. Applied suggested change. 👍
git-extra/git-prompt.sh
Outdated
| then | ||
| precmd() # Assembles git prompt that closely resembles the bash prompt. | ||
| { | ||
| pre="${newline}%F{green}%n@%m%f %F{magenta}${MSYSTEM:-"ZSH"}%f %F{yellow}%~%f %F{cyan}" | ||
| post="%f${newline}$ " | ||
| __git_ps1 "${pre}" "${post}" "(%s)" | ||
| } |
There was a problem hiding this comment.
That's different from what is recommended in Git's own contrib/completion/git-prompt.sh:
# ZSH: setopt PROMPT_SUBST ; PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
Would it not be preferable to follow that suggestion instead, or am I missing anything?
There was a problem hiding this comment.
A few reasons,
I tried that approach first (Line 16) however, it did not work when using it in this file.
It only worked when adding it to a zshrc file.
A couple lines below they themselves recommend a faster alternative that worked when using it in this file:
Faster alternative recommended in Git's own contrib/completion/git-prompt.sh
ZSH: precmd () { __git_ps1 "%n" ":%~$ " "|%s" }
git-extra/git-prompt.sh
Outdated
| post="%f${newline}$ " | ||
| __git_ps1 "${pre}" "${post}" "(%s)" | ||
| } | ||
| return # Avoids zsh git-completion.bash Error. |
There was a problem hiding this comment.
This file is sourced. What happens when you return in such a scenario? Especially to any other files in /etc/profile.d/ that are supposed to be sourced, too, not to mention the remainder of the file?
There was a problem hiding this comment.
I was under the impression the return would only end the current scope of the if block.
I have pushed a restructured if else block without the need for the return. 👍
git-extra/git-prompt.sh
Outdated
| __git_ps1 "${pre}" "${post}" "(%s)" | ||
| } | ||
| return # Avoids zsh git-completion.bash Error. | ||
| fi | ||
| . "$COMPLETION_PATH/git-completion.bash" |
There was a problem hiding this comment.
There is a git-completion.zsh. Why don't we use that with zsh?
There was a problem hiding this comment.
I tried that first but it gave even more errors and broke git completions completely.
Afterwards I discovered that Zsh comes with its own Git completions.
Signed-off-by: Javier S. Orejarena <[email protected]>
Howdy!
Problem
"ERROR: this script is obsolete, please see git-completion.zsh"Solution
Note:
Git completions will still work for Bash and Zsh.
Prompts with "__git_ps1" will still work for Bash and Zsh.