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

the documented solution for hybrid key bindings does not work #3556

Closed
krader1961 opened this Issue Nov 15, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@krader1961
Contributor

krader1961 commented Nov 15, 2016

Per the discussion in issue #3068 the emacs specific bindings are no longer automatically included in the vi mode bindings. Recognizing that some people would still want to be able to create a hybrid of the default (aka emacs) and vi bindings we documented how to do so:

function hybrid_bindings --description "Vi-style bindings that inherit emacs-style bindings in all modes"
    for mode in default insert visual
        fish_default_key_bindings -M $mode
    end
    fish_vi_key_bindings --no-erase
end
set -g fish_key_bindings hybrid_bindings

Unfortunately that won't work. The reason is that the fish_vi_key_bindings function executes

set fish_key_bindings fish_vi_key_bindings

That triggers the fish C++ code to run the function a second time without the --no-erase flag (or any arguments for that matter).

The simplest solution is to also predicate that set on whether the function was not invoked with the --no-erase flag. That fixes the problem. Albeit with the side-effect that because $fish_key_bindings is no longer fish_vi_key_bindings you won't see the vi mode indicator; e.g., [I] or [N]. However, I suspect that most users of hybrid key bindings will view that as a feature.

@krader1961 krader1961 added the bug label Nov 15, 2016

@krader1961 krader1961 added this to the fish 2.5.0 milestone Nov 15, 2016

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 15, 2016

Contributor

Another side-effect of the simple solution I proposed above is that $fish_key_bindings will be set to hybrid_bindings which I think is what we want since it makes it clear where the bindings came from.

Contributor

krader1961 commented Nov 15, 2016

Another side-effect of the simple solution I proposed above is that $fish_key_bindings will be set to hybrid_bindings which I think is what we want since it makes it clear where the bindings came from.

@dead10ck

This comment has been minimized.

Show comment
Hide comment
@dead10ck

dead10ck Nov 15, 2016

Albeit with the side-effect that because $fish_key_bindings is no longer fish_vi_key_bindings you won't see the vi mode indicator; e.g., [I] or [N]. However, I suspect that most users of hybrid key bindings will view that as a feature.

I personally would see that as a bug. If I'm using vi mode, I would expect to see the vi mode indicator.

May I ask what the rationale was for removing key bindings from vi mode? #3068 did not really explain its rationale at all, other than "it shared too much." It worked pretty well for me before 2.4 was released.

dead10ck commented Nov 15, 2016

Albeit with the side-effect that because $fish_key_bindings is no longer fish_vi_key_bindings you won't see the vi mode indicator; e.g., [I] or [N]. However, I suspect that most users of hybrid key bindings will view that as a feature.

I personally would see that as a bug. If I'm using vi mode, I would expect to see the vi mode indicator.

May I ask what the rationale was for removing key bindings from vi mode? #3068 did not really explain its rationale at all, other than "it shared too much." It worked pretty well for me before 2.4 was released.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 15, 2016

Contributor

In fact, we should package the hybrid_bindings function with fish.

I personally would see that as a bug.

There has been talk about tying the vi mode indicator to something other than the value of $fish_key_bindings (this is done in share/functions/fish_mode_prompt.fish). It might make more sense to change the current logic to show [N] if using pure VI bindings and the mode is default; otherwise don't show the normal mode indicator. Otherwise base the other indicators on whether the mode is one of the VI modes: insert, visual or replace-one. Alternatively, just test whether $fish_key_bindings is either fish_vi_fish_vi_key_bindings or hybrid mode and leave the rest of the logic in the function as is.

Contributor

krader1961 commented Nov 15, 2016

In fact, we should package the hybrid_bindings function with fish.

I personally would see that as a bug.

There has been talk about tying the vi mode indicator to something other than the value of $fish_key_bindings (this is done in share/functions/fish_mode_prompt.fish). It might make more sense to change the current logic to show [N] if using pure VI bindings and the mode is default; otherwise don't show the normal mode indicator. Otherwise base the other indicators on whether the mode is one of the VI modes: insert, visual or replace-one. Alternatively, just test whether $fish_key_bindings is either fish_vi_fish_vi_key_bindings or hybrid mode and leave the rest of the logic in the function as is.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Nov 15, 2016

Member

I personally stopped using vi mode after 2.4.0 - but I think I was just very used to the old more-shared behavior. I'm not sure what's better. I'm not a vi power user.

Member

floam commented Nov 15, 2016

I personally stopped using vi mode after 2.4.0 - but I think I was just very used to the old more-shared behavior. I'm not sure what's better. I'm not a vi power user.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 15, 2016

Contributor

I stopped using vi mode many months ago because the inconsistencies with true Vi behavior was just too annoying. And I found that after creating a edit_command_buffer function to let me edit the current command line in a real editor and adding the following bindings I no longer needed the vi mode bindings to do things like delete-word. This makes it easier to just use a real editor for any non-trivial editing of the command line:

function fish_user_key_bindings
    bind \ee edit_command_buffer
    bind \ev edit_command_buffer
end

PR forthcoming to implement a working hybrid mode. TBD is whether we want the current fish_mode_prompt for the hybrid mode, no indicator, or a modified indicator to make it clearer that you're using hybrid bindings. My first draft of the PR will extend the current mode indicator behavior to the hybrid bindings.

Contributor

krader1961 commented Nov 15, 2016

I stopped using vi mode many months ago because the inconsistencies with true Vi behavior was just too annoying. And I found that after creating a edit_command_buffer function to let me edit the current command line in a real editor and adding the following bindings I no longer needed the vi mode bindings to do things like delete-word. This makes it easier to just use a real editor for any non-trivial editing of the command line:

function fish_user_key_bindings
    bind \ee edit_command_buffer
    bind \ev edit_command_buffer
end

PR forthcoming to implement a working hybrid mode. TBD is whether we want the current fish_mode_prompt for the hybrid mode, no indicator, or a modified indicator to make it clearer that you're using hybrid bindings. My first draft of the PR will extend the current mode indicator behavior to the hybrid bindings.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Nov 15, 2016

Contributor

May I ask what the rationale was for removing key bindings from vi mode?

The main problem, @dead10ck, is that hybrid mode causes characters not otherwise bound in "default" (i.e., vi normal) mode to self-insert. Which means that after pressing [escape] to switch from insert to normal mode pressing $ moves to the end of the line and pressing 0 moves to the start of the line but pressing 1 actually inserts a 1, etcetera.

Contributor

krader1961 commented Nov 15, 2016

May I ask what the rationale was for removing key bindings from vi mode?

The main problem, @dead10ck, is that hybrid mode causes characters not otherwise bound in "default" (i.e., vi normal) mode to self-insert. Which means that after pressing [escape] to switch from insert to normal mode pressing $ moves to the end of the line and pressing 0 moves to the start of the line but pressing 1 actually inserts a 1, etcetera.

krader1961 added a commit to krader1961/fish-shell that referenced this issue Nov 15, 2016

make it easy to get the legacy hybrid key bindings
The changes related to issue #3068 removed most of the emacs bindings
from vi mode. However, since fish 2.4.0 was released several people have
pointed out that the directions for reinstating the legacy hybrid key
bindings don't work. This change fixes that and makes it easier to use
the legacy hybrid bindings.

Fixes #3556

@krader1961 krader1961 self-assigned this Nov 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment