-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add Alt as modifier key to git log search key binding #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PatrickF1
added a commit
that referenced
this pull request
Jun 11, 2021
Setting custom key bindings is horrible experience. 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. However, for users who want to change them, the process of customizing them is frustrating and confusing at best. This frustration has culminated in a steady stream of issues, discussions, PRs, and Wiki sections around key bindings (#17, #89, #99, #108, #103, #153, #162, [Wiki section](https://github.com/PatrickF1/fzf.fish/wiki/Cookbook/60b24ade20395cab07148c87c14966046ccbe8e9#how-can-i-customize-only-one-key-binding)). Here are the shortcomings of making custom bindings DIY: - Besides not installing the default bindings if `fzf_fish_custom_keybindings` is set, the plugin provides _zero_ assistance to the user when it comes to installing the custom bindings. Users have to study `config/fzf.fish`, `bind` documentation, and maybe even learn some fish syntax to learn how to set their own key bindings. This is a quite daunting task. Furthermore, users have to reference functions clearly denoted private (`__fzf_search_*`)., which feels and use hacky. Ideally, the plugin does not require typical users to dive into its code and will abstract private functions away completely. - `fzf_fish_custom_keybindings` is all or nothing; users have to opt out of ALL default key bindings and re-bind them themselves even if they only want to change one. This is tedious, verbose, and unwieldy. A good plugin should allow tweaking the key binding while maintaining the rest of the defaults. - Because the `fzf_fish_custom_keybindings` has to be set before `config/fzf.fish` is executed, it has to be created as a universal variable. Unfortunately, universal variables are a [very confusing point for new fish users](fish-shell/fish-shell#7317 (comment)). - And because it needs to be universal, it's [bad practice to add it to one's `config.fish`](https://gitter.im/fish-shell/fish-shell?at=60c3880fbed13a2dba7acd55). This makes it more difficult for one's `fzf.fish` configuration to be [checked into git](https://github.com/PatrickF1/dotfiles/tree/master/.config/fish). As the plugin author, I have also felt the pain of working around this DIY custom binding mechanism: - Because custom bindings are so painful to use, user forego custom bindings and get stuck with the defaults, which puts a lot of pressure on me (subconsciously and through the many issues opened) to make the default bindings suitable for _everyone_. Unfortunately, the goal of having a default set of key bindings that is suitable for even 80% of users while being easy to learn (or mnemonic) is nigh impossible, requiring a thorough understanding of the multitudes of CLIs and terminals people use, the ways they use them, and they key binding. I've concluded this task is ultimately futile because key bindings are very idiosyncratic. Therefore, custom key bindings should be a first class feature that is well supported, completely documented, flexible, painless to use, and directly encouraged. - Any change, even an addition (such as a new feature), to the default key bindings is an outsized, monumental event because it's not transparent and hard to communicate to users. In fisher, there isn't a mechanism for precisely warning users of backwards incompatible changes. Furthermore, the only place where the default key bindings are officially listed is on the readme. The best I can do is to cut a new major release and post on Reddit and Gitter to announce any and all key binding changes. But still, most users will probably be left in the dark when their key bindings silently break on the next update. If more users customized their key bindings and (which they don't because again, it's not well supported) and there was an interface to quickly view the default key bindings, this would be a much smaller problem. - Because users have to hardcode in the function names for their custom key bindings, fzf.fish's internal functions are not really private and I cannot rename them as they morph. This is important to me as the names of some features, and therefore the ideal function name, have shifted since their inception. - Finally, `fzf_fish_custom_keybindings` has an annoying typo: key binding is two words, not one. ## The new solution A new function called `fzf_configure_bindings` solves or mitigates all of the aforementioned problems. It: - serves as a wrapper around the key bindings and fzf.fish's private functions so that they are properly abstracted away under a lightweight interface. - allows customizing the key binding for each feature through namesake options (e.g. `--directory` controls the search directory key binding). Bindings can be overridden or, if the user doesn't want to use the feature, even disabled. - uses mnemonic key sequences by default for features that the user chooses to not customize. This means that to change the key bind for a single feature, only one option needs to be specified. - comes with great, easy-to-read help documentation that prints if used incorrectly. - includes the default key bindings in its help message so users can easily and quickly find them. - is robust and thoroughly tested. - will include command completion (to be implemented later).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After much feedback around git log search's
Ctrl+l
key binding conflicting with clear screen, I have decided to change its key binding toCtrl+Alt+l
. An alternative option I considered was justAlt+l
, which is shorter but I realized conflicts with__fish_list_current_token
, which is actually quite useful and I plan on using it now that I've discovered it.One huge problem with this change is that it is backwards incompatible and fisher has not provided me with a way to announce to users of
fzf.fish
that the key binding has changed. I had considered using a hack of checking if an arbitrary file existed, printing an error message if it doesn't and then creating that file, but it adds a lot of difficult-to-maintain code and slows down the shell startup.Hopefully, people who discover that
Ctrl+l
no longer works will come to this repo and see this commit. I will also create a new release announcing this change. I am sorry for any confusion caused by this.