On Mon, Feb 22, 2021 at 10:29:52AM -0800, Bart Schaefer wrote: > On Mon, Jan 25, 2021 at 12:17 AM Arseny Maslennikov wrote: > > > > The promptinit framework fails to apply cleanup commands of the current > > theme on any theme change other than "prompt restore", as well as when > > invoking setup functions of a certain $theme as an implementation detail > > of `prompt -[hp] $theme'. > > Thanks, I did have a chance to look at this. > > > We fix it in the following way, hopefully without breaking compatibility: > > * Rename zstyle `cleanup' on the context `:prompt-theme' to `restore' > > everywhere but in prompt_cleanup(). It is only used as a restore > > mechanism now. > > * Ensure prompt_cleanup() continues to store its command list in the > > `cleanup' style. > > * Clean up before theme switch at the end of set_prompt(). > > These all seem fine, conceptually. > > > * Prepend every use of prompt_*_setup (which might modify the shell > > state in ways that require cleanup) with a cleanup run. > > Have you confirmed that this is a no-op when there has not been a > previous installation of a theme? I double-checked right now; in that case, with the patch applied as posted, it is a no-op when the command 'prompt -p' is invoked, but not when 'prompt -h' is — the trap does not run. Thanks for the good catch! The following diff on top of my patch fixes it: --->8--- diff --git a/Functions/Prompts/promptinit b/Functions/Prompts/promptinit index ab2b50857..5c7dcc5a1 100644 --- a/Functions/Prompts/promptinit +++ b/Functions/Prompts/promptinit @@ -105,9 +105,11 @@ Use prompt -h for help on specific themes.' local +h PS1=$PS1 PS2=$PS2 PS3=$PS3 PS4=$PS4 RPS1=$RPS1 RPS2=$RPS2 local +h PROMPT=$PROMPT RPROMPT=$RPROMPT RPROMPT2=$RPROMPT2 PSVAR=$PSVAR local -a precmd_functions preexec_functions + local theme_reset='' else - trap 'zstyle -t :prompt-theme cleanup; prompt_${prompt_theme[1]}_setup "${(@)prompt_theme[2,-1]}"' 0 + local theme_reset='prompt_${prompt_theme[1]}_setup "${(@)prompt_theme[2,-1]}"' fi + trap 'zstyle -t :prompt-theme cleanup;'"${theme_reset:+ $theme_reset}" 0 ;; esac case "$opt" in -------- > > > * Adjust `prompt restore' to do both parts of the newly split restore > > mechanism, cleanup first. > > Also fine. Just a couple of nit-picky things about the diff itself: > > > diff --git a/Functions/Prompts/promptinit b/Functions/Prompts/promptinit > > index 5e42ebdd3..1c6d27ad7 100644 > > --- a/Functions/Prompts/promptinit > > +++ b/Functions/Prompts/promptinit > > @@ -123,6 +125,7 @@ Use prompt -h for help on specific themes.' > > # The next line is a bit ugly. It (perhaps unnecessarily) > > # runs the prompt theme setup function to ensure that if > > # the theme has a _help function that it's been autoloaded. > > + zstyle -t :prompt-theme cleanup > > prompt_$2_setup > > fi > > if functions prompt_$2_help >/dev/null; then > > Placement of the added zstyle call makes the comment incorrect, move > it above the comment? Will do. > > > @@ -179,28 +182,41 @@ Use prompt -h for help on specific themes.' > > + # Will add the hook this time. > > + prompt_cleanup "$@" > > This is logically correct, but I don't think it adds much in terms of > maintainability to make this a recursive call; there are only 2 > commands executed in that event. Ok; I'll have to refactor this again. :)