jeffreytse / zsh-vi-mode

💻 A better and friendly vi(vim) mode plugin for ZSH.
MIT License
3.13k stars 109 forks source link

Only initialize unset command hooks arrays to empty arrays. #172

Closed gotgenes closed 2 years ago

gotgenes commented 2 years ago

This fixes an issue where zsh-vi-mode ignores the values for commands hooks arrays set before sourcing the plugin. For example, the documentation shows the following:

# Append a command directly
zvm_after_init_commands+=('[ -f ~/.fzf.zsh ] && source ~/.fzf.zsh')

but zvm_after_init_commands was always getting reset to the empty array upon sourcing this plugin.

gotgenes commented 2 years ago

@jeffreytse Is there anything I can do to help move this PR forward?

gotgenes commented 2 years ago

Yes, I can give you my scenario. Here Is how I'm setting up zsh-vi-mode..

I have two sets of keybindings I'd like to register. I define the functions to set the keybindings before sourcing zsh-vi-mode. Because sourcing zsh-vi-mode resets the hooks arrays to empty arrays, I can't directly configure these arrays as described in the docs.

Let's focus just on one of the two sets of keybindings so it is more clear what I mean.

Here is a set of insert-mode keybindings I want to register as a command to run after zsh-vi-mode init:

function set_insert_keybindings() {
    zvm_bindkey viins "^[[H" beginning-of-line
    zvm_bindkey viins "^[[F" end-of-line
    zvm_bindkey viins "^[[Z" reverse-menu-complete
    zvm_bindkey viins "^[[A" history-substring-search-up
    zvm_bindkey viins "^[[B" history-substring-search-down
}

Here is how the documentation describes registering the command:

zvm_after_init_commands+=(set_insert_keybindings)

Then I source zsh-vi-mode with

zinit light jeffreytse/zsh-vi-mode

Putting it all together, it would look like this:

function set_insert_keybindings() {
    zvm_bindkey viins "^[[H" beginning-of-line
    zvm_bindkey viins "^[[F" end-of-line
    zvm_bindkey viins "^[[Z" reverse-menu-complete
    zvm_bindkey viins "^[[A" history-substring-search-up
    zvm_bindkey viins "^[[B" history-substring-search-down
}
zvm_after_init_commands+=(set_insert_keybindings)
zinit light jeffreytse/zsh-vi-mode

When we source zsh-vi-mode, zvm_after_init_commands gets redefined as an empty array, and so my keybindings fail to get set.

You can see that right now I have to hack around this shortcoming by registering the hooks within another hook:

function zvm_before_init() {
    # zsh-vi-mode has set zvm_after_init_commands to an empty array at this point
    # but it has not yet invoked it, so we can modify it
    zvm_after_init_commands+=(set_insert_keybindings)
}

I'd rather set the hooks directly, not within another hook.

This PR fixes that issue by only defining the hooks arrays if they're not yet defined. They will no longer get overwritten.

gotgenes commented 2 years ago

I pushed up some changes to improve clarity. I hope they help.

gotgenes commented 2 years ago

If the plugin is already loaded, I would expect changing zvm_after_init_commands has no effect. I believe we should have already finished calling the commands in that array. What have I misunderstood?

gotgenes commented 2 years ago

@jeffreytse My apologies. I have updated the code to fit the 2-space indentation.