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

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

Closed
wants to merge 1 commit into from

Conversation

krobelus
Copy link
Member

@krobelus krobelus commented Mar 2, 2024

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
Copy link
Member

faho commented Mar 2, 2024

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
Copy link
Member Author

krobelus commented Mar 3, 2024

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
Copy link
Member

faho commented Mar 3, 2024

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.

@@ -84,7 +84,6 @@ function fish_prompt
end

if test "$fish_key_bindings" = fish_vi_key_bindings
or test "$fish_key_bindings" = fish_hybrid_key_bindings
Copy link
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.

end
set -g fish_key_bindings fish_hybrid_key_bindings
set -g fish_key_bindings my_key_bindings
Copy link
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
Copy link
Member Author

krobelus commented Mar 9, 2024

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
Copy link
Member

zanchey commented Mar 9, 2024

Retargeting this so a release note gets added

@krobelus
Copy link
Member Author

krobelus commented Mar 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants