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

Overhaul key bindings mechanism #158

Closed
wants to merge 47 commits into from
Closed

Overhaul key bindings mechanism #158

wants to merge 47 commits into from

Conversation

PatrickF1
Copy link
Owner

@PatrickF1 PatrickF1 commented May 10, 2021

Abandoned in favor of #172


Fzf.fish's key bindings are hard to manage. It's time to give it a make over!

Background and motivation

fzf.fish ships with default key bindings that are mnemonic and have minimal conflicts with existing fish key bindings out of the box. However, for the users who don't want to use them, they get no assistance from the plugin and process of customizing them is tedious, unwieldy, and difficult to learn. This frustration has culminated in a steady stream of issues, discussions, Cookbook sections, and PRs around key bindings (#17, #89, #99, #108, #103, #153, #162). Here're the shortcomings of leaving it to users to set fzf_fish_custom_keybindings and do all their own bindings:

  • There is no one size fits all when it comes to key bindings. Users seem to fall into several sizable, distinct clusters. Examples: those who prefer prefer Emacs bindings, or take advantage of iTerm shortcuts, or are on Windows. Certainly not all users can be grouped with one distinct cluster, but those that do they seem to want to keep their habits and have a stable, consistent set of key bindings across their many developer tools. This has manifested in a trend of developer tools supporting many convenient, pre-defined, inspired keymaps that users can choose from: Fish itself, VSCode, IntelliJ, Sublime Text, CodeMirror, Cloud9 and Insomnia. Even the most sane and circumspect default set of key binding cannot replace the value of having multiple keymaps that cater to specific swaths of users.
  • fzf_fish_custom_keybindings is all or nothing; it's not possible to change just one or two key bindings. Users have to opt out of ALL default key bindings and re-bind all the functions themselves. This is tedious, verbose, and unwieldy.
  • Users have to study config/fzf.fish to learn how to set their own key bindings.
  • Because the variable has to be set before config/fzf.fish is executed, it can't be specified in the user's config.fish, which is the traditional home for user-level fish configuration. This is a confusing point for users new to fish and also makes it more difficult for one's fzf.fish configuration to be checked into git.

As the plugin author, I have also felt supporting all the different key binding use cases very difficult and has hindered development:

  • The goal of having a default set of key bindings that is suitable for 90% of users while being easy to learn (or mnemonic) is much more ambitious than it sounds. It requires having deep familiarity with the many developer tools people use and the multi-faceted ways they are used. It turns out this requires a lot of research, a lot of listening to people, and yet, is ultimately a futile goal because key bindings are very idiosyncratic. Therefore, custom key bindings should be a first class feature that is well documented, flexible, optimized, and even directly encouraged.
  • Changing the default key bindings is a very painful process for both me and users. In fisher, there isn't a mechanism I can leverage to warn users of backwards incompatible changes. Short of a better solution for notifying users, I cut a new major release and post on Reddit and Gitter to announce any and all key binding changes. But still, it's likely most users will be left in the dark when their key bindings silently break on the next update.
  • Users have to hardcode in the function names for their custom key bindings. This is a major abstraction leak that keeps functions from being renamed as they should be. This is important to me as the name of each feature is still subject to change and I'd prefer for the function names to match the feature name.
  • Finally, fzf_fish_custom_keybindings has an annoying typo: key binding is two words, not one.

The new solution

  • The base of the new solution is a new function fzf_install_bindings that, given an ordered list of key sequences, will install the key bindings for each command.
  • This means users who want to specify their own key bindings can do so with just one line.
  • This also provides a great starting point for pre-configured keymaps that ship with the plugin. For example, this PR adds two new keymaps: one that is mnemonic and should not conflict with the key bindings of commonly used tools, and one that is mnemonic and has minimal modifier keys and so is very easy to remember but will conflict with other key bindings.

@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from 2327661 to 20a3980 Compare May 28, 2021 16:52
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch 2 times, most recently from a6831e7 to ea1db70 Compare May 29, 2021 21:31
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from ea1db70 to db5bb0b Compare May 29, 2021 21:32
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from 5f86e7f to f8ac6d9 Compare May 30, 2021 03:33
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from f2d9671 to 3769cb1 Compare June 1, 2021 02:55
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from 9ac2223 to 0a64729 Compare June 2, 2021 21:09
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from 0a64729 to 60956cf Compare June 2, 2021 21:11
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch 3 times, most recently from cfdbb39 to 2a5ad20 Compare June 3, 2021 02:17
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch 3 times, most recently from 0354a38 to ebaa80c Compare June 3, 2021 04:12
@PatrickF1 PatrickF1 force-pushed the key-bind-mechanism branch from ebaa80c to bd5f33c Compare June 3, 2021 17:30
@PatrickF1
Copy link
Owner Author

PatrickF1 commented Jun 4, 2021

Hi @kidonng, could you provide your perspective and expansive CLI wisdom please? I'm wondering if the idea I have in this PR is too complicated.
Currently, there's a new function called fzf_configure_keymap that takes in a keymap, which is a set of pre-defined bindings, and more custom overrides, so the user can use it like fzf_configure_keymap simple_mnemonic --directory=\e\cf. However, the code, the tests, and the documentation is getting really complicated and I'm wondering if I should simplify it with a simple function that just binds whatever you pass it. So something like fzf_configure_bindings --directory=\cf --variables=\cv git_log=\cl. LMK if that makes sense.

Btw, the code and the tests are done, but not the CLI help text, readme, or PR description, so don't use those things to understand what I built.

@PatrickF1
Copy link
Owner Author

PatrickF1 commented Jun 10, 2021

I've decided this approach is too complicated in terms of

  • ease of learning
  • ease of use
  • code and docs required to support this

I'm going to close this and restart with a simpler but fairly similar approach in #172.

@PatrickF1 PatrickF1 closed this Jun 10, 2021
@PatrickF1 PatrickF1 deleted the key-bind-mechanism branch June 16, 2021 23:18
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.

1 participant