-
-
Notifications
You must be signed in to change notification settings - Fork 135
Only initialize unset command hooks arrays to empty arrays. #172
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
Only initialize unset command hooks arrays to empty arrays. #172
Conversation
@jeffreytse Is there anything I can do to help move this PR forward? |
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.
Hi @gotgenes
Thanks for your contribution. I am trying to understand what scenario this change can be used to, one of the scenario I can find is we need to set these commands when we change initial mode to ZVM_INIT_MODE=sourcing
, as maybe some internal commands will be added to these aspects in the future and keep the order fixed. : )
Thanks & Regards
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:
Here is how the documentation describes registering the command:
Then I source zsh-vi-mode with
Putting it all together, it would look like this:
When we source zsh-vi-mode, You can see that right now I have to hack around this shortcoming by registering the hooks within another hook:
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. |
I pushed up some changes to improve clarity. I hope they help. |
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.
Hi @gotgenes
I can understand what you mean, I still have a question, why you don't want to put them after loading zsh-vi-mode
.
zinit light jeffreytse/zsh-vi-mode
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)
Thanks & Regards
If the plugin is already loaded, I would expect changing |
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.
Hi @gotgenes
Currently, I am going to merge your PR, as the indent is 2 spaces, could you format it by the standard?
Thanks & Regards
fbca865
to
176dbfa
Compare
@jeffreytse My apologies. I have updated the code to fit the 2-space indentation. |
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.
That's great, thanks for your contribution, and keep going on! : )
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:
but
zvm_after_init_commands
was always getting reset to the empty array upon sourcing this plugin.