Stop refreshing previous prompt

[maximbaz]

This fixes https://github.com/sorin-ionescu/prezto/issues/667.

May be controversial, but I do think this is a bug, not a feature. There is no sense in treating the last prompt differently, and with the current behavior it is impossible to see when the last command started and how long it took to execute.

Before (pay attention to the time stamp on the right):

After:

[paulmelnikow]

I agree this should be fixed, but need to understand better before I can weigh in.

This line refreshes the prompt after a change in editor state, such as changing in and out of command mode, and between insert and overwrite modes. Does that seem right?

Is the clock continually refreshing, or does it only refresh just before the next command runs?

zle editor-info is invoked in a number of places in this file. Any idea which one is triggering the offending refresh?

[maximbaz]

Hey, thanks for looking into this pull request!

This line refreshes the prompt after a change in editor state, such as changing in and out of command mode, and between insert and overwrite modes. Does that seem right?

If the command mode is "edit command in editor", then it is not affected, the command that you enter in the editor is placed on a new line after you exit the editor, it is not replacing the previous prompt.

As for the changing the insert/overwrite mode, you are right! This is affected, with my change the current prompt is not refreshed.

Is the clock continually refreshing, or does it only refresh just before the next command runs?

The clock is not refreshing, the clock shows when the time when the prompt was drawn, i.e. when the previous command finished.

zle editor-info is invoked in a number of places in this file. Any idea which one is triggering the offending refresh?

I'll nail this down later today, as now I know that insert/overwrite mode is affected, will try to come up with a better fix. If you have ideas, please share.

[maximbaz]

@paulmelnikow this is the offending call: https://github.com/zsh-users/prezto/pull/17/files#diff-d47aedeec385fb4b85ecd505c23f9572R141

I'm honestly not sure if that call is even needed, i.e. if I can simply remove it, or the removal will break something. If I can't remove it, how should we proceed, parameterize editor-info somehow so that it doesn't execute zle reset-prompt in one specific condition, when it is coming from zle-line-finish?

[paulmelnikow]

I tracked down the commit that added the call to zle reset-prompt in zle-line-finish. It's some very old code.

I did some of my own testing. The call to editor-info in zle-line-finish is pointless, as leaving editing mode doesn't update $KEYMAP or $ZLE_STATE.

Let's remove it, and perhaps leave a comment behind with a link to this issue in case it resurfaces. It might be nice to provide a way to keep the old behavior. Right now the only way I see to do that is to overwrite zle-line-finish.

[paulmelnikow]

Ah, to make the functions easy to override, how about declaring them like this?

editor/init.zsh

function prezto_zle_line_finish {
  ...
}
zle -N zle-line-finish prezto_zle_line_finish

prompt_whatever_setup

function prompt_whatever_zle_line_finish {
  prezto_zle_line_finish
  zle editor-info
}

function prompt_whatever_setup {
  zle -N zle-line-finish prompt_whatever_zle_line_finish
}

[maximbaz]

Could you please elaborate a bit on the last suggestion? :slightly_smiling_face: I'm not sure I entirely understand what you propose.

[maximbaz]

I updated the PR with your previous suggestion to not call editor-info in zle-line-finish.

[paulmelnikow]

Sure. The idea is to let prompt developers keep the old behavior if they want. To do this, they need to define their own zle-line-finish widget that runs zle reset-prompt, and register it with zle -N zle-line-finish in their setup function.

The default implementation of zle-line-finish just turns off application mode in the terminal. To keep this behavior in place, they should invoke it from within their function. This is instead of copying and pasting it.

I'm suggesting changing the names of the hook functions from e.g. zle-line-finish to prezto_zle_line_finish and updating the calls to zle -N accordingly. While other code can invoke these functions with their current names, it feels safer if they're named in a way that is expected to be unique. I suggested e.g. prezto_zle_line_finish but prezto_editor_zle_line_finish seems better.

[maximbaz]

@paulmelnikow thanks for the explanation! I'm feeling a little uncomfortable to perform the renaming, I'm not familiar enough with the codebase and I can see that a widget with such name is referenced in other files, even in external repositories. Is it the same widget? Are we not risking to accidentally break other stuff? Maybe we should handle this separately?

❯  ag "zle-line-finish"
modules/syntax-highlighting/external/zsh-syntax-highlighting.zsh
269:  # Always wrap special zle-line-finish widget. This is needed to decide if the
272:  widgets_to_bind+=(zle-line-finish)

modules/syntax-highlighting/external/highlighters/cursor/cursor-highlighter.zsh
38:  [[ $WIDGET == zle-line-finish ]] || _zsh_highlight_cursor_moved
44:  [[ $WIDGET == zle-line-finish ]] && return

modules/syntax-highlighting/external/highlighters/main/test-data/path_prefix2.zsh
34:WIDGET=zle-line-finish

modules/syntax-highlighting/external/highlighters/main/main-highlighter.zsh
61:  [[ $WIDGET == zle-line-finish ]] || _zsh_highlight_buffer_modified
768:     [[ $WIDGET != zle-line-finish ]]; then

modules/syntax-highlighting/external/highlighters/brackets/test-data/cursor-matchingbracket-line-finish.zsh
30:WIDGET=zle-line-finish

modules/syntax-highlighting/external/highlighters/brackets/brackets-highlighter.zsh
43:  [[ $WIDGET == zle-line-finish ]] || _zsh_highlight_cursor_moved || _zsh_highlight_buffer_modified
89:  if [[ $WIDGET != zle-line-finish ]]; then

modules/editor/init.zsh
133:function zle-line-finish {
144:zle -N zle-line-finish

[paulmelnikow]

Good catch. I only searched within prezto and missed that.

Interesting. I see code that rebinds user-defined widgets. While I can't completely follow what it does, it doesn't seem to depend on the name of the function registered for the widget. That makes sense, since it couldn't assume any particular name.

I just tested syntax-highlighting, and it works fine, including the re-highlighting at the end of editing triggered by the line finish hook. So, I don't think the rename would break any of those functions.

That said, I'm fine with leaving the names alone. We can revisit if the need comes up.