Overhaul key bindings mechanism

README.md

@@ -10,7 +10,7 @@ Augment your [Fish][] command line with mnemonic key bindings to efficiently fin
 
 ## Features
 
-Use `fzf.fish` to interactively find and insert into the command line:
+Use `fzf.fish` to interactively find and insert different shell entities into the command line:
 
 ### File paths
 
@@ -30,15 +30,15 @@ Use `fzf.fish` to interactively find and insert into the command line:
 ![git status select][]
 
 - **Search input:** the current repository's `git status`
-- **Key binding and mnemonic:** <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>S</kbd> (`S` for status, `Alt` to prevent overriding `pager-toggle-search`)
+- **Key binding and mnemonic:** <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>S</kbd> (`S` for status)
 - **Remarks:** <kbd>Tab</kbd> to multi-select
 
 ### A commit hash
 
 ![git log search][]
 
 - **Search input:** the current repository's formatted `git log`
-- **Key binding and mnemonic:** <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>L</kbd> (`L` for log, `Alt` to prevent overriding clear screen)
+- **Key binding and mnemonic:** <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>L</kbd> (`L` for log)
 - **Preview window:** commit message and diff
 
 ### A previously run command
@@ -86,15 +86,7 @@ fisher install PatrickF1/fzf.fish
 
 ### Customize the key bindings
 
-If you would like to customize the key bindings, first, prevent the default key bindings from executing by setting `fzf_fish_custom_keybindings` as an [universal variable][]. You can do this with
-
-```fish
-set --universal fzf_fish_custom_keybindings
-```
-
-Do not try to set `fzf_fish_custom_keybindings` in your `config.fish` because the key binding configuration is sourced first on shell startup and so will not see it.
-
-Next, set your own key bindings by following [conf.d/fzf.fish][] as an example. Your search variables key binding should reference the `fzf_search_vars_cmd` variable instead of hardcoding the command.
+fzf.fish has its own dedicated function for conveniently installing key bindings.
 
 ### Pass fzf options to all commands
 

conf.d/fzf.fish

@@ -1,28 +1,11 @@
 # Because of scoping rules, to capture the shell variables exactly as they are, we must read
 # them before even executing __fzf_search_shell_variables. We use psub to store the
 # variables' info in temporary files and pass in the filenames as arguments.
-# This variable is intentionally global so that it can be referenced by custom key bindings and tests
-set --global fzf_search_vars_cmd '__fzf_search_shell_variables (set --show | psub) (set --names | psub)'
+# # This variable is global so that it can be referenced by fzf_install_bindings and in tests
+set --global _fzf_search_vars_command '__fzf_search_shell_variables (set --show | psub) (set --names | psub)'
 
-# Set up the default, mnemonic key bindings unless the user has chosen to customize them
-if not set --query fzf_fish_custom_keybindings
-    # \cf is Ctrl+f
-    bind \cf __fzf_search_current_dir
-    bind \cr __fzf_search_history
-    bind \cv $fzf_search_vars_cmd
-    # The following two key binding use Alt as an additional modifier key to avoid conflicts
-    bind \e\cl __fzf_search_git_log
-    bind \e\cs __fzf_search_git_status
-
-    # set up the same key bindings for insert mode if using fish_vi_key_bindings
-    if test "$fish_key_bindings" = fish_vi_key_bindings -o "$fish_key_bindings" = fish_hybrid_key_bindings
-        bind --mode insert \cf __fzf_search_current_dir
-        bind --mode insert \cr __fzf_search_history
-        bind --mode insert \cv $fzf_search_vars_cmd
-        bind --mode insert \e\cl __fzf_search_git_log
-        bind --mode insert \e\cs __fzf_search_git_status
-    end
-end
+# Install some safe and memorable key bindings by default
+fzf_configure_keymap conflictless_mnemonic
 
 # If FZF_DEFAULT_OPTS is not set, then set some sane defaults. This also affects fzf outside of this plugin.
 # See https://github.com/junegunn/fzf#environment-variables
@@ -36,17 +19,23 @@ if not set --query FZF_DEFAULT_OPTS
     set --global --export FZF_DEFAULT_OPTS '--cycle --layout=reverse --border --height=90% --preview-window=wrap --marker="*"'
 end
 
+# Doesn't erase FZF_DEFAULT_OPTS because too hard to tell if it was set by the user or by this plugin
+# Doesn't erase autoloaded __fzf_* functions because they will not be easily accessible once key bindings are erased
 function _fzf_uninstall --on-event fzf_uninstall
-    # Not going to erase FZF_DEFAULT_OPTS because too hard to tell if it was set by the user or by this plugin
-    if not set --query fzf_fish_custom_keybindings
-        bind --erase \cf \cr \cv \e\cl \e\cs
-        bind --erase --mode insert \cf \cr \cv \e\cl \e\cs
+    _fzf_uninstall_keymap
 
-        set_color --italics cyan
-        echo "fzf.fish key bindings removed"
-        set_color normal
-    end
+    set --erase _fzf_search_vars_command
+    functions --erase _fzf_uninstall _fzf_migration_message
+    functions --erase _fzf_uninstall_keymap fzf_configure_keymap fzf_conflictless_mnemonic_keymap fzf_simple_mnemonic_keymap
+
+    set_color --italics cyan
+    echo "fzf.fish uninstalled"
+    set_color normal
+end
 
-    set --erase __fzf_search_vars_cmd
-    functions --erase _fzf_uninstall
+function _fzf_migration_message --on-event fzf_update
+    set_color FF8C00 # dark orange
+    printf '\n%s\n' 'If you last updated fzf.fish before June 2021, you will need to migrate your key bindings.'
+    printf '%s\n\n' 'Check out https://github.com/PatrickF1/fzf.fish/wiki/Migration-guides.'
+    set_color normal
 end

functions/__fzf_search_shell_variables.fish

@@ -2,13 +2,8 @@
 # argument 1 = output of (set --show | psub), i.e. a file with the scope info and values of all variables
 # argument 2 = output of (set --names | psub), i.e. a file with all variable names
 function __fzf_search_shell_variables --argument-names set_show_output set_names_output --description "Search and preview shell variables. Replace the current token with the selected variable."
-    # inform users who use custom key bindings of the backwards incompatible change
     if test -z "$set_names_output"
-        set_color red
-        printf '\n%s\n' '__fzf_search_shell_variables now requires arguments so you have to update your key bindings.' >&2
-        printf '%s\n\n' 'Please see github.com/PatrickF1/fzf.fish/releases/tag/v5.0 for the resolution.' >&2
-        set_color normal
-
+        printf '%s\n' '__fzf_search_shell_variables requires 2 arguments.' >&2
         commandline --function repaint
         return 22 # 22 means invalid argument in POSIX
     end

functions/_fzf_configure_keymap_help.fish

@@ -0,0 +1,17 @@
+function _fzf_configure_keymap_help
+    echo 'fzf_configure_keymap - change the key bindings for fzf.fish.'
+    echo 'Usage:'
+    echo '    fzf_configure_keymap --searchable_entity=key_sequence...'
+    echo 'Description:'
+    echo '    Key bindings must be specified as long options where the option name is an entity searchable by fzf.fish and the option value is a key sequence. Try fish_key_reader to generate key sequences.'
+    echo '    You do not have to specify key bindings for all entities. However, you must specify at least one.'
+    echo '    Fails if positional arguments or unknown options are provided.'
+    echo '    Completely overwrites any fzf.fish keymap previously installed by this function. This means users are free to repeatedly invoke this function to experiment with different keymaps without having to worry about residual bindings.'
+    echo 'Options:'
+    echo '    --directory=key_sequence'
+    echo '    --git_log=key_sequence'
+    echo '    --git_status=key_sequence'
+    echo '    --history=key_sequence'
+    echo '    --variables=key_sequence'
+    echo '    -h or --help'
+end

functions/fzf_configure_keymap.fish

@@ -0,0 +1,56 @@
+# Supports overriding keymaps with appended user-specified bindings
+# Always installs bindings for insert mode since for simplicity and b/c it has almost no side-effect
+# https://gitter.im/fish-shell/fish-shell?at=60a55915ee77a74d685fa6b1
+function fzf_configure_keymap --argument-names keymap_or_help --description "Change the key bindings for fzf.fish to the specified key sequences."
+    # index 1 = directory, 2 = git_log, 3 = git_status, 4 = history, 5 = variables
+    set --local key_sequences
+    switch $keymap_or_help
+        case conflictless_mnemonic
+            set key_sequences \e\cf \e\cl \cs \cr \cv
+        case simple_mnemonic
+            set key_sequences \cf \cl \cs \cr \cv
+        case simple_conflictless
+            set key_sequences \co \cl \cg \er \ex
+        case blank
+            set key_sequences "" "" "" "" ""
+        case -h --help
+            _fzf_configure_keymap_help
+            return
+        case '*'
+            echo "Invalid or missing keymap argument." 1>&2
+            _fzf_configure_keymap_help
+            return 22
+    end
+
+    set options_spec 'directory=?' 'git_log=?' 'git_status=?' 'history=?' 'variables=?'
+    argparse --max-args=0 --ignore-unknown $options_spec -- $argv[2..] 2>/dev/null #argv[1] is the keymap
+    if test $status -ne 0
+        echo "Invalid option or more than one argument provided." 1>&2
+        _fzf_configure_keymap_help
+        return 22
+    else
+        set --query _flag_directory && set key_sequences[1] "$_flag_directory"
+        set --query _flag_git_log && set key_sequences[2] "$_flag_git_log"
+        set --query _flag_git_status && set key_sequences[3] "$_flag_git_status"
+        set --query _flag_history && set key_sequences[4] "$_flag_history"
+        set --query _flag_variables && set key_sequences[5] "$_flag_variables"
+    end
+
+    # If another keymap already exists, uninstall it first for a clean slate
+    if functions --query _fzf_uninstall_keymap
+        _fzf_uninstall_keymap
+    end
+
+    for mode in default insert
+        test -n $key_sequences[1] && bind --mode $mode $key_sequences[1] __fzf_search_current_dir
+        test -n $key_sequences[2] && bind --mode $mode $key_sequences[2] __fzf_search_git_log
+        test -n $key_sequences[3] && bind --mode $mode $key_sequences[3] __fzf_search_git_status
+        test -n $key_sequences[4] && bind --mode $mode $key_sequences[4] __fzf_search_history
+        test -n $key_sequences[5] && bind --mode $mode $key_sequences[5] $_fzf_search_vars_command
+    end
+
+    function _fzf_uninstall_keymap --inherit-variable key_sequences
+        bind --erase -- $key_sequences
+        bind --erase --mode insert -- $key_sequences
+    end
+end

tests/configure_keymap/help.fish

@@ -0,0 +1,2 @@
+fzf_configure_keymap -h >/dev/null
+@test "fzf_configure_keymap help works" $status -eq 0

tests/configure_keymap/key_sequence_opts.fish

@@ -0,0 +1,23 @@
+function binding_contains_func --argument-names sequence function_name
+    string match --entire $function_name (bind $sequence) >/dev/null
+end
+
+fzf_configure_keymap conflictless_mnemonic --directory=\ca --git_log=\cb --git_status=\cc --history=\cd --variables=\ce
+@test "can override the keymap's sequence for directory" (binding_contains_func \ca __fzf_search_current_dir) $status -eq 0
+@test "can override the keymap's sequence for git log" (binding_contains_func \cb __fzf_search_git_log) $status -eq 0
+@test "can override the keymap's sequence for git status" (binding_contains_func \cc __fzf_search_git_status) $status -eq 0
+@test "can override the keymap's sequence for history" (binding_contains_func \cd __fzf_search_history) $status -eq 0
+@test "can override the keymap's sequence for variables" (binding_contains_func \ce __fzf_search_shell_variables) $status -eq 0
+
+bind --mode insert \ca >/dev/null && bind --mode insert \ce >/dev/null
+@test "installs bindings for insert mode" $status -eq 0
+
+_fzf_uninstall_keymap
+@test "custom key sequences are properly erased on keymap uninstall" -z (bind --user | string match --entire __fzf_)
+
+# intentionally test both style of passing options with no value
+fzf_configure_keymap simple_mnemonic --directory --git_status=
+@test "can remove bindings from the specified keymap" -z (bind --user | string match --entire --regex '__fzf_search_current_dir|__fzf_search_git_status')
+
+fzf_configure_keymap blank --unknown=\cq &>/dev/null
+@test "fails if passed unknown option" $status -ne 0

tests/configure_keymap/keymap_arg.fish

@@ -0,0 +1,33 @@
+# append _ to history b/c history is a special variable
+function keymap_installed_successfully -a directory git_log git_status history_ variables
+    begin
+        bind $directory | string match --entire __fzf_search_current_dir &&
+            bind $git_log | string match --entire __fzf_search_git_log &&
+            bind $git_status | string match --entire __fzf_search_git_status &&
+            bind $history_ | string match --entire __fzf_search_history &&
+            bind $variables | string match --entire "$_fzf_search_vars_command"
+    end >/dev/null
+end
+
+# success cases
+fzf_configure_keymap conflictless_mnemonic && keymap_installed_successfully \e\cf \e\cl \cs \cr \cv
+@test "conflictless_mnemonic installs successfully" $status -eq 0
+
+fzf_configure_keymap simple_mnemonic && keymap_installed_successfully \cf \cl \cs \cr \cv
+@test "simple_mnemonic installs successfully" $status -eq 0
+
+fzf_configure_keymap simple_conflictless && keymap_installed_successfully \co \cl \cg \er \ex
+@test "simple_conflictless installs successfully" $status -eq 0
+
+fzf_configure_keymap blank
+@test "blank installs no key bindings" -z (bind --user | string match --entire __fzf_)
+
+# failure cases
+fzf_configure_keymap --directory=\co blank &>/dev/null
+@test "fails if the first arugment is not a keymap" $status -ne 0
+
+fzf_configure_keymap blank simple_mnemonic &>/dev/null
+@test "fails if there are two keymaps specified" $status -ne 0
+
+fzf_configure_keymap typo &>/dev/null
+@test "fails if given an invalid keymap" $status -ne 0

tests/search_shell_variables/$_in_curr_token.fish

@@ -3,5 +3,5 @@ mock commandline --current-token "echo \\\$variable"
 mock commandline "--current-token --replace" "echo \$argv" # instead of updating commandline with the result, just output it
 mock fzf \* "echo selection"
 
-set actual (eval $fzf_search_vars_cmd)
+set actual (eval $_fzf_search_vars_command)
 @test "doesn't overwrite \$ when replacing current token with selected variable" "$actual" = "\$selection"

tests/search_shell_variables/local_var.fish

@@ -2,5 +2,5 @@ set --local a_local_variable
 set --export --append FZF_DEFAULT_OPTS "--filter=a_local_variable"
 mock commandline "--current-token --replace" "echo \$argv" # instead of updating commandline with the result, just output it
 mock commandline \* "" # mock all other commandline executions to do nothing
-set actual (eval $fzf_search_vars_cmd)
+set actual (eval $_fzf_search_vars_command)
 @test "can find a local variable" "$actual" = a_local_variable

tests/uninstaller/erases_bindings.fish

@@ -0,0 +1,2 @@
+_fzf_uninstall
+@test "erases all fzf key bindings" -z (bind --user | string match --entire "__fzf_search__")

tests/uninstaller/erases_fzf_vars.fish

@@ -0,0 +1,2 @@
+_fzf_uninstall
+@test "uninstall removes _fzf_search_vars_command" (set --query _fzf_search_vars_command) $status -ne 0