Skip to content
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

vi-mode: fix smkx/rmkx by removing broken line-init/finish widgets #4191

Merged
merged 1 commit into from
Nov 29, 2015
Merged

vi-mode: fix smkx/rmkx by removing broken line-init/finish widgets #4191

merged 1 commit into from
Nov 29, 2015

Conversation

apjanke
Copy link
Contributor

@apjanke apjanke commented Jul 28, 2015

The zle-line-init and zle-line-finish definition changes introduced in #1533 were broken with
respect to smkx/rmkx because their "if" logic had fallthrough where it shouldn't,
so the mode was left in rmkx ("local" keypad mode) all the time. This breaks anything that uses terminfo or application ("transmit") mode escape sequences for cursor key bindings, including key bindings in some other plugins.

This PR just removes those widgets entirely, because they're now defined (correctly, I think) in lib/keybindings.zsh and not needed in plugins any more. (As long as the intent is to enable application cursor mode; see Caveats.)

Partially fixes #2735.

Caveats and Regressions

This fixes vi-mode so the terminal will be in "application cursor" aka "transmit" mode while editing the command line, instead of "local" mode. This is how it is when OMZ is loaded but the vi-mode plugin is not. This allows key bindings made with the portable terminfo support to work.

However, users that use vi-mode and have hardcoded keybindings that map the cursor key sequences that a terminal sends in "local" mode are currently relying in the current broken smkx/rmkx code to keep the keypad in "local cursor" mode. Those key bindings would break.

Since terminfo supports portability to different terminals, and OMZ as a whole and other plugins seem to be moving toward using them, we may want to do this despite the breakage. This also seems to be where the zsh community as a whole is going: the zsh mailing lists seem to endorse [sr]mkx. And Ubuntu is shipping with an /etc/zsh/zshrc that enables it by default.

Interaction with prompt overwriting and zle reset-prompt

I think the zle reset-prompt and zle -R were only needed on the zle-keymap-select, so they won't be missed from the other widgets (the lib/keybindings.zsh version doesn't use them). I've tested this out with the combination of vi-mode and per-directory-history (which introduced this initially in #1387), and it seems to be working for me. That is, with various themes I've tried, including two-line ones, it doesn't show the prompt-overwriting problem for me.

…ings

The zle-line-init and zle-line-finish definitions here were broken with
respect to smkx/rmkx because their "if" logic had fallthrough where it shouldn't,
so the mode was left in rmkx all the time. This just removes those widgets
entirely, because they're now defined (correctly) in lib/keybindings.zsh and
not needed in plugins.
@apjanke
Copy link
Contributor Author

apjanke commented Nov 18, 2015

Here's an example of why I think we should settle on using [sr]mkx and supporting terminfo. This would let us switch to more readable, mnemonic-based definitions for binding cursor and other navigation keys, like this.

omz_bindkey   kpp    up-line-or-history           # [PageUp] - Up a line of history
omz_bindkey   knp    down-line-or-history         # [PageDown] - Down a line of history
omz_bindkey   kcuu1  up-line-or-search            # start typing + [Up-Arrow] - fuzzy find history forward
omz_bindkey   kcud1  down-line-or-search          # start typing + [Down-Arrow] - fuzzy find history backward
omz_bindkey   khome  beginning-of-line            # [Home] - Go to beginning of line
omz_bindkey   kend   end-of-line                  # [End] - Go to end of line

I find that a lot more readable than hardcoded control sequences or the `if [[ -n "$terminfo{blah}" ]] ..." blocks. And using a function would give us the opportunity to add meaningful debugging output to help users identify what is setting their key bindings. And maybe even extend the terminfo mappings with support for things like alt-arrow keys and keypad-local-mode sequences.

mcornella added a commit that referenced this pull request Nov 29, 2015
vi-mode: fix smkx/rmkx by removing broken line-init/finish widgets
@mcornella mcornella merged commit 46824b3 into ohmyzsh:master Nov 29, 2015
nunogt pushed a commit to nunogt/oh-my-zsh that referenced this pull request Jan 25, 2016
vi-mode: fix smkx/rmkx by removing broken line-init/finish widgets
@apjanke apjanke deleted the vi-mode-fix-smkx branch November 4, 2018 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

history-substring-search no longer functions after latest update
2 participants