New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Improve compatibility with Zsh prompt theme system #3480
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Unfortunately doesn't let us interface with the zsh promptinit module, but if it works for Znap I'll take it.
One thing: can we modify the comment on line 27 (or add one above it) to explain why the naming scheme is needed? It departs from the naming schemes we use in most of the other init scripts, so I'm worried someone will try to change it back to preserve styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My previous comment was supposed to be a "Request Changes" but I guess I hit the wrong button.
src/init/starship.zsh
Outdated
@@ -24,7 +24,7 @@ else | |||
fi | |||
|
|||
# Will be run before every prompt draw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we modify this comment (or add one above it) to explain why the naming scheme is needed? It departs from the naming schemes we use in most of the other init scripts, so I'm worried someone will try to change it back to preserve styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marlonrichert Sorry for the multiple change requests---I'm afraid even after working with the GitHub interface with a while, I have a tendency to goober certain operations. |
Zsh's `promptinit` expects a theme's hook functions to be named `prompt_<theme>_<hook>`. See https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs look good and testing suggests that this works fine on my local machine. It's a 👍 from me.
Thank you for your contribution @marlonrichert and thanks for the review @chipbuster. |
[1]: starship#3480 (comment) [2]: https://zsh.sourceforge.io/Doc/Release/User-Contributions.html#Writing-Themes Signed-off-by: Coelacanthus <coelacanthus@outlook.com>
[1]: #3480 (comment) [2]: https://zsh.sourceforge.io/Doc/Release/User-Contributions.html#Writing-Themes Signed-off-by: Coelacanthus <coelacanthus@outlook.com>
Hi, I have updated to https://github.com/starship/starship/releases/tag/v1.3.0, |
@tshu-w What problems are you seeing? The only places I see |
@chipbuster @andytom When I install Starship 1.3.0, I still get the same output from (Click to expand)❯ starship init zsh --print-full-init
# ZSH has a quirk where `preexec` is only run if a command is actually run (i.e
# pressing ENTER at an empty command line will not cause preexec to fire). This
# can cause timing issues, as a user who presses "ENTER" without running a command
# will see the time to the start of the last command, which may be very large.
# To fix this, we create STARSHIP_START_TIME upon preexec() firing, and destroy it
# after drawing the prompt. This ensures that the timing for one command is only
# ever drawn once (for the prompt immediately after it is run).
zmodload zsh/parameter # Needed to access jobstates variable for STARSHIP_JOBS_COUNT
# Defines a function `__starship_get_time` that sets the time since epoch in millis in STARSHIP_CAPTURED_TIME.
if [[ $ZSH_VERSION == ([1-4]*) ]]; then
# ZSH <= 5; Does not have a built-in variable so we will rely on Starship's inbuilt time function.
__starship_get_time() {
STARSHIP_CAPTURED_TIME=$(/home/linuxbrew/.linuxbrew/Cellar/starship/1.2.1/bin/starship time)
}
else
zmodload zsh/datetime
zmodload zsh/mathfunc
__starship_get_time() {
(( STARSHIP_CAPTURED_TIME = int(rint(EPOCHREALTIME * 1000)) ))
}
fi
# Will be run before every prompt draw
starship_precmd() {
# Save the status, because commands in this pipeline will change $?
STARSHIP_CMD_STATUS=$? STARSHIP_PIPE_STATUS=(${pipestatus[@]})
# Compute cmd_duration, if we have a time to consume, otherwise clear the
# previous duration
if (( ${+STARSHIP_START_TIME} )); then
__starship_get_time && (( STARSHIP_DURATION = STARSHIP_CAPTURED_TIME - STARSHIP_START_TIME ))
unset STARSHIP_START_TIME
else
unset STARSHIP_DURATION
fi
# Use length of jobstates array as number of jobs. Expansion fails inside
# quotes so we set it here and then use the value later on.
STARSHIP_JOBS_COUNT=${#jobstates}
}
starship_preexec() {
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
}
# If precmd/preexec arrays are not already set, set them. If we don't do this,
# the code to detect whether starship_precmd is already in precmd_functions will
# fail because the array doesn't exist (and same for starship_preexec)
(( ! ${+precmd_functions} )) && precmd_functions=()
(( ! ${+preexec_functions} )) && preexec_functions=()
# If starship precmd/preexec functions are already hooked, don't double-hook them
# to avoid unnecessary performance degradation in nested shells
if [[ -z ${precmd_functions[(re)starship_precmd]} ]]; then
precmd_functions+=(starship_precmd)
fi
if [[ -z ${preexec_functions[(re)starship_preexec]} ]]; then
preexec_functions+=(starship_preexec)
fi
# Set up a function to redraw the prompt if the user switches vi modes
starship_zle-keymap-select() {
zle reset-prompt
}
## Check for existing keymap-select widget.
# zle-keymap-select is a special widget so it'll be "user:fnName" or nothing. Let's get fnName only.
__starship_preserved_zle_keymap_select=${widgets[zle-keymap-select]#user:}
if [[ -z $__starship_preserved_zle_keymap_select ]]; then
zle -N zle-keymap-select starship_zle-keymap-select;
else
# Define a wrapper fn to call the original widget fn and then Starship's.
starship_zle-keymap-select-wrapped() {
$__starship_preserved_zle_keymap_select "$@";
starship_zle-keymap-select "$@";
}
zle -N zle-keymap-select starship_zle-keymap-select-wrapped;
fi
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
export STARSHIP_SHELL="zsh"
# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.
VIRTUAL_ENV_DISABLE_PROMPT=1
setopt promptsubst
PROMPT='$(/home/linuxbrew/.linuxbrew/Cellar/starship/1.2.1/bin/starship prompt --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
RPROMPT='$(/home/linuxbrew/.linuxbrew/Cellar/starship/1.2.1/bin/starship prompt --right --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
PROMPT2="$(/home/linuxbrew/.linuxbrew/Cellar/starship/1.2.1/bin/starship prompt --continuation)" Is this cached somewhere? |
But in my satrship, output is correct: Details>>> starship init zsh --print-full-init
# ZSH has a quirk where `preexec` is only run if a command is actually run (i.e
# pressing ENTER at an empty command line will not cause preexec to fire). This
# can cause timing issues, as a user who presses "ENTER" without running a command
# will see the time to the start of the last command, which may be very large.
# To fix this, we create STARSHIP_START_TIME upon preexec() firing, and destroy it
# after drawing the prompt. This ensures that the timing for one command is only
# ever drawn once (for the prompt immediately after it is run).
zmodload zsh/parameter # Needed to access jobstates variable for STARSHIP_JOBS_COUNT
# Defines a function `__starship_get_time` that sets the time since epoch in millis in STARSHIP_CAPTURED_TIME.
if [[ $ZSH_VERSION == ([1-4]*) ]]; then
# ZSH <= 5; Does not have a built-in variable so we will rely on Starship's inbuilt time function.
__starship_get_time() {
STARSHIP_CAPTURED_TIME=$(/usr/bin/starship time)
}
else
zmodload zsh/datetime
zmodload zsh/mathfunc
__starship_get_time() {
(( STARSHIP_CAPTURED_TIME = int(rint(EPOCHREALTIME * 1000)) ))
}
fi
# The two functions below follow the naming convention `prompt_<theme>_<hook>`
# for compatibility with Zsh's prompt system. See
# https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155
# Runs before each new command line.
prompt_starship_precmd() {
# Save the status, because commands in this pipeline will change $?
STARSHIP_CMD_STATUS=$? STARSHIP_PIPE_STATUS=(${pipestatus[@]})
# Compute cmd_duration, if we have a time to consume, otherwise clear the
# previous duration
if (( ${+STARSHIP_START_TIME} )); then
__starship_get_time && (( STARSHIP_DURATION = STARSHIP_CAPTURED_TIME - STARSHIP_START_TIME ))
unset STARSHIP_START_TIME
else
unset STARSHIP_DURATION
fi
# Use length of jobstates array as number of jobs. Expansion fails inside
# quotes so we set it here and then use the value later on.
STARSHIP_JOBS_COUNT=${#jobstates}
}
# Runs after the user submits the command line, but before it is executed.
prompt_starship_preexec() {
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
}
# Add hook functions
autoload -Uz add-zsh-hook
add-zsh-hook precmd prompt_starship_precmd
add-zsh-hook preexec prompt_starship_preexec
# Set up a function to redraw the prompt if the user switches vi modes
starship_zle-keymap-select() {
zle reset-prompt
}
## Check for existing keymap-select widget.
# zle-keymap-select is a special widget so it'll be "user:fnName" or nothing. Let's get fnName only.
__starship_preserved_zle_keymap_select=${widgets[zle-keymap-select]#user:}
if [[ -z $__starship_preserved_zle_keymap_select ]]; then
zle -N zle-keymap-select starship_zle-keymap-select;
else
# Define a wrapper fn to call the original widget fn and then Starship's.
starship_zle-keymap-select-wrapped() {
$__starship_preserved_zle_keymap_select "$@";
starship_zle-keymap-select "$@";
}
zle -N zle-keymap-select starship_zle-keymap-select-wrapped;
fi
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
export STARSHIP_SHELL="zsh"
# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.
VIRTUAL_ENV_DISABLE_PROMPT=1
setopt promptsubst
PROMPT='$(/usr/bin/starship prompt --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
RPROMPT='$(/usr/bin/starship prompt --right --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
PROMPT2="$(/usr/bin/starship prompt --continuation)" I use archlinux |
Sorry I misunderstood, please ignore me. @marlonrichert Hi, my Click to expand# ZSH has a quirk where `preexec` is only run if a command is actually run (i.e
# pressing ENTER at an empty command line will not cause preexec to fire). This
# can cause timing issues, as a user who presses "ENTER" without running a command
# will see the time to the start of the last command, which may be very large.
# To fix this, we create STARSHIP_START_TIME upon preexec() firing, and destroy it
# after drawing the prompt. This ensures that the timing for one command is only
# ever drawn once (for the prompt immediately after it is run).
zmodload zsh/parameter # Needed to access jobstates variable for STARSHIP_JOBS_COUNT
# Defines a function `__starship_get_time` that sets the time since epoch in millis in STARSHIP_CAPTURED_TIME.
if [[ $ZSH_VERSION == ([1-4]*) ]]; then
# ZSH <= 5; Does not have a built-in variable so we will rely on Starship's inbuilt time function.
__starship_get_time() {
STARSHIP_CAPTURED_TIME=$(/usr/local/bin/starship time)
}
else
zmodload zsh/datetime
zmodload zsh/mathfunc
__starship_get_time() {
(( STARSHIP_CAPTURED_TIME = int(rint(EPOCHREALTIME * 1000)) ))
}
fi
# The two functions below follow the naming convention `prompt_<theme>_<hook>`
# for compatibility with Zsh's prompt system. See
# https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155
# Runs before each new command line.
prompt_starship_precmd() {
# Save the status, because commands in this pipeline will change $?
STARSHIP_CMD_STATUS=$? STARSHIP_PIPE_STATUS=(${pipestatus[@]})
# Compute cmd_duration, if we have a time to consume, otherwise clear the
# previous duration
if (( ${+STARSHIP_START_TIME} )); then
__starship_get_time && (( STARSHIP_DURATION = STARSHIP_CAPTURED_TIME - STARSHIP_START_TIME ))
unset STARSHIP_START_TIME
else
unset STARSHIP_DURATION
fi
# Use length of jobstates array as number of jobs. Expansion fails inside
# quotes so we set it here and then use the value later on.
STARSHIP_JOBS_COUNT=${#jobstates}
}
# Runs after the user submits the command line, but before it is executed.
prompt_starship_preexec() {
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
}
# Add hook functions
autoload -Uz add-zsh-hook
add-zsh-hook precmd prompt_starship_precmd
add-zsh-hook preexec prompt_starship_preexec
# Set up a function to redraw the prompt if the user switches vi modes
starship_zle-keymap-select() {
zle reset-prompt
}
## Check for existing keymap-select widget.
# zle-keymap-select is a special widget so it'll be "user:fnName" or nothing. Let's get fnName only.
__starship_preserved_zle_keymap_select=${widgets[zle-keymap-select]#user:}
if [[ -z $__starship_preserved_zle_keymap_select ]]; then
zle -N zle-keymap-select starship_zle-keymap-select;
else
# Define a wrapper fn to call the original widget fn and then Starship's.
starship_zle-keymap-select-wrapped() {
$__starship_preserved_zle_keymap_select "$@";
starship_zle-keymap-select "$@";
}
zle -N zle-keymap-select starship_zle-keymap-select-wrapped;
fi
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
export STARSHIP_SHELL="zsh"
# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.
VIRTUAL_ENV_DISABLE_PROMPT=1
setopt promptsubst
PROMPT='$(/usr/local/bin/starship prompt --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
RPROMPT='$(/usr/local/bin/starship prompt --right --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
PROMPT2="$(/usr/local/bin/starship prompt --continuation)"
``````shell
# ZSH has a quirk where `preexec` is only run if a command is actually run (i.e
# pressing ENTER at an empty command line will not cause preexec to fire). This
# can cause timing issues, as a user who presses "ENTER" without running a command
# will see the time to the start of the last command, which may be very large.
# To fix this, we create STARSHIP_START_TIME upon preexec() firing, and destroy it
# after drawing the prompt. This ensures that the timing for one command is only
# ever drawn once (for the prompt immediately after it is run).
zmodload zsh/parameter # Needed to access jobstates variable for STARSHIP_JOBS_COUNT
# Defines a function `__starship_get_time` that sets the time since epoch in millis in STARSHIP_CAPTURED_TIME.
if [[ $ZSH_VERSION == ([1-4]*) ]]; then
# ZSH <= 5; Does not have a built-in variable so we will rely on Starship's inbuilt time function.
__starship_get_time() {
STARSHIP_CAPTURED_TIME=$(/usr/local/bin/starship time)
}
else
zmodload zsh/datetime
zmodload zsh/mathfunc
__starship_get_time() {
(( STARSHIP_CAPTURED_TIME = int(rint(EPOCHREALTIME * 1000)) ))
}
fi
# The two functions below follow the naming convention `prompt_<theme>_<hook>`
# for compatibility with Zsh's prompt system. See
# https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155
# Runs before each new command line.
prompt_starship_precmd() {
# Save the status, because commands in this pipeline will change $?
STARSHIP_CMD_STATUS=$? STARSHIP_PIPE_STATUS=(${pipestatus[@]})
# Compute cmd_duration, if we have a time to consume, otherwise clear the
# previous duration
if (( ${+STARSHIP_START_TIME} )); then
__starship_get_time && (( STARSHIP_DURATION = STARSHIP_CAPTURED_TIME - STARSHIP_START_TIME ))
unset STARSHIP_START_TIME
else
unset STARSHIP_DURATION
fi
# Use length of jobstates array as number of jobs. Expansion fails inside
# quotes so we set it here and then use the value later on.
STARSHIP_JOBS_COUNT=${#jobstates}
}
# Runs after the user submits the command line, but before it is executed.
prompt_starship_preexec() {
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
}
# Add hook functions
autoload -Uz add-zsh-hook
add-zsh-hook precmd prompt_starship_precmd
add-zsh-hook preexec prompt_starship_preexec
# Set up a function to redraw the prompt if the user switches vi modes
starship_zle-keymap-select() {
zle reset-prompt
}
## Check for existing keymap-select widget.
# zle-keymap-select is a special widget so it'll be "user:fnName" or nothing. Let's get fnName only.
__starship_preserved_zle_keymap_select=${widgets[zle-keymap-select]#user:}
if [[ -z $__starship_preserved_zle_keymap_select ]]; then
zle -N zle-keymap-select starship_zle-keymap-select;
else
# Define a wrapper fn to call the original widget fn and then Starship's.
starship_zle-keymap-select-wrapped() {
$__starship_preserved_zle_keymap_select "$@";
starship_zle-keymap-select "$@";
}
zle -N zle-keymap-select starship_zle-keymap-select-wrapped;
fi
__starship_get_time && STARSHIP_START_TIME=$STARSHIP_CAPTURED_TIME
export STARSHIP_SHELL="zsh"
# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.
VIRTUAL_ENV_DISABLE_PROMPT=1
setopt promptsubst
PROMPT='$(/usr/local/bin/starship prompt --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
RPROMPT='$(/usr/local/bin/starship prompt --right --terminal-width="$COLUMNS" --keymap="${KEYMAP:-}" --status="$STARSHIP_CMD_STATUS" --pipestatus="${STARSHIP_PIPE_STATUS[*]}" --cmd-duration="${STARSHIP_DURATION:-}" --jobs="$STARSHIP_JOBS_COUNT")'
PROMPT2="$(/usr/local/bin/starship prompt --continuation)" |
@marlonrichert This is not cached. But the path to starship in your init seems to point to Anyway, I think I'll make a PR to make |
@davidkna Ah, my bad. I thought |
…p#3480) Zsh's `promptinit` expects a theme's hook functions to be named `prompt_<theme>_<hook>`. See https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155
…hip#3537) [1]: starship#3480 (comment) [2]: https://zsh.sourceforge.io/Doc/Release/User-Contributions.html#Writing-Themes Signed-off-by: Coelacanthus <coelacanthus@outlook.com>
Zsh's
promptinit
expects a theme's hook functions to be namedprompt_<theme>_<hook>
. See https://github.com/zsh-users/zsh/blob/2876c25a28b8052d6683027998cc118fc9b50157/Functions/Prompts/promptinit#L155Description
starship_precmd
toprompt_starship_precmd
.starship_preexec
toprompt_starship_preexec
.Motivation and Context
Adhering to this de facto standard makes it easier for
promptinit
and other Zsh software following these conventions (such as Znap'sprompt
command) to interact correctly with Starship.Screenshots (if appropriate):
How Has This Been Tested?
I started Zsh with the new output of
starship init zsh --print-full-init
and it works the same as the old one.Checklist:
This does not cause any changes in documentation or tests.