Skip to content

Kill pure Vi key bindings, use hybrid behavior instead#10339

Closed
krobelus wants to merge 1 commit into
fish-shell:masterfrom
krobelus:vi-mode-is-hybrid
Closed

Kill pure Vi key bindings, use hybrid behavior instead#10339
krobelus wants to merge 1 commit into
fish-shell:masterfrom
krobelus:vi-mode-is-hybrid

Conversation

@krobelus

@krobelus krobelus commented Mar 2, 2024

Copy link
Copy Markdown
Contributor

I find pure Vi mode very unpleasant to use because it doesn't define
shortcuts like Control-n and Control-f. Judging by the number of references
to "fish_vi_key_bindings" in public github repos, it seems that many users
don't know about "fish_hybrid_key_bindings" (unless they prefer the other
for some unknown reason?).

As far as I'm concerned, hybrid bindings are strictly better than Vi ones.
Let's avoid this easy user error and make both use the hybrid behavior.

Hybrid mode is nice because it reduces the friction of learning Vi mode,
because most Emacs key work the same way. Similarly, it reduces friction
caused by some of our bugs and missing features in Vi mode, because one
can simply fall back to the Emacs binding.

I find pure Vi mode very unpleasant to use because it doesn't define
shortcuts like Control-n and Control-f. Judging by the number of references
to "fish_vi_key_bindings" in public github repos, it seems that many users
don't know about "fish_hybrid_key_bindings" (unless they prefer the other
for some unknown reason?).

As far as I'm concerned, hybrid bindings are strictly better than Vi ones.
Let's avoid this easy user error and make both use the hybrid behavior.

Hybrid mode is nice because it reduces the friction of learning Vi mode,
because most Emacs key work the same way. Similarly, it reduces friction
caused by some of our bugs and missing features in Vi mode, because one
can simply fall back to the Emacs binding.
@faho

faho commented Mar 2, 2024

Copy link
Copy Markdown
Member

I find pure Vi mode very unpleasant to use because it doesn't define
shortcuts like Control-n and Control-f

I just opened up an unconfigured neovim to check: Control-f does nothing sensible (appears to be unbound), and Control-n cycles through the completion candidates (so it's more like our TAB).

These two specifically are extremely fundamental emacs - the sort of binding it will teach you in the first five minutes of an emacs tutorial.

Judging by the number of references
to "fish_vi_key_bindings" in public github repos, it seems that many users
don't know about "fish_hybrid_key_bindings" (unless they prefer the other
for some unknown reason?).

I would generally assume that people use vi-bindings because they like the vi-style of working.

I'm not opposed to adding more things to the shared bindings, but I don't believe adding the core emacs movements to vi-mode is what vi-binding users want.

Of course we run into the vi-mode problem: None of us use vi-mode, and nobody who does tries to fix it at the level necessary.

(I have said before that I would be entirely fine to remove vi-mode from fish's core. Let someone maintain it as a plugin if they want)

@krobelus

krobelus commented Mar 3, 2024

Copy link
Copy Markdown
Contributor Author

These two specifically are extremely fundamental emacs - the sort of binding it will teach you in the first five minutes of an emacs tutorial.

One problem is that Vi insert mode does not have a good binding to accept autosuggestions (there's right arrow but that's not always easy).
This UI problem is unique to us; Vi doesn't have autosuggestions.
So we want to eventually come up with a good default binding.

Control-n also makes a lot of sense to me given that we already map Tab to complete.
And if there is a completion pager, it will move just like in Vi.

My guess is if we do some design work we'll end up with something that's close to the hybrid bindings.

I missed some subtle conflicts between hybrid and Vi bindings: \ef behaves differently.
We could resolve them in favor of Vi. I'm dogfooding Vi mode, if I stick to it, I'll eventually do that.

I saw that Vi mode already used to inherit from the default bindings but haven't researched that.

Of course we run into the vi-mode problem: None of us use vi-mode, and nobody who does tries to fix it at the level necessary.

(I have said before that I would be entirely fine to remove vi-mode from fish's core. Let someone maintain it as a plugin if they want)

Removing it is an option. It has definitely bitrotted.
I think getting rid of at least one of hybrid/vi would be good, to improve focus.

But an argument for keeping it is that this style of thing is popular in general; long term it might be nice to have Kakoune bindings.
Let's see if I start owning this.

@faho

faho commented Mar 3, 2024

Copy link
Copy Markdown
Member

I think getting rid of at least one of hybrid/vi would be good, to improve focus.

Hybrid is literally just "run emacs bindings, add vi on top". I do not believe there is really anything to work on for the hybrid bindings, so I do not see how that takes away focus.

In particular none of the issues open about vi-mode would be solved by this.

One problem is that Vi insert mode does not have a good binding to accept autosuggestions

It is insert mode. Insert-mode isn't really for moving around, if you interpret autosuggestions as "text after the real text" like we otherwise do it makes sense that you would have to switch to normal mode.

That being said: I would be fine with adding a thing to accept the autosuggestions in vi-insert mode. It could even be ctrl-n, which is already completion-y in vi.

But that's a far cry from "just do everything emacs-style too".

My guess is if we do some design work we'll end up with something that's close to the hybrid bindings.

I want to have someone work on the vi-bindings who is actually invested in vi-bindings.

If you are more interested in kakoune-bindings, I would be happy to have those, even if we dropped vi-bindings instead.

end

if test "$fish_key_bindings" = fish_vi_key_bindings
or test "$fish_key_bindings" = fish_hybrid_key_bindings

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we did merge this, this line should stay, because people will keep using their old hybrid_key_bindings.

Comment thread doc_src/interactive.rst
bind \cg beginning-of-line 'commandline -i "# "'
end
set -g fish_key_bindings fish_hybrid_key_bindings
set -g fish_key_bindings my_key_bindings

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not tell people to make a full function to change a single binding.

We still have people define fish_user_key_bindings because they read older docs or blog posts, and this adds to the confusion.

@krobelus

krobelus commented Mar 9, 2024

Copy link
Copy Markdown
Contributor Author

It seems that surprisingly many people -- who are otherwise sane -- use Vi mode so I'd say keep it for now.

That being said: I would be fine with adding a thing to accept the autosuggestions in vi-insert mode. It could even be ctrl-n, which is already completion-y in vi.

I like Control-N for accept-autosuggestion. I'll reduce this PR to just that.
This missing key binding is really the main pain point for me.

@krobelus krobelus closed this in 836ee93 Mar 9, 2024
@krobelus krobelus added this to the will-not-implement milestone Mar 9, 2024
@zanchey

zanchey commented Mar 9, 2024

Copy link
Copy Markdown
Member

Retargeting this so a release note gets added

@krobelus

krobelus commented Mar 9, 2024

Copy link
Copy Markdown
Contributor Author

I didn't add a release note because I'm not sure if I want to advertise Vi mode. I guess it's fine to add that though.

In general I don't feel good about accumulating (release note) work. If there's something left to do I'd rather not merge it.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants