Rework mode handling of bind

Binds with the same sequence in multiple modes was not working right.
Fix up the implementation to propagate modes everywhere as necessary.
This means that `bind` will properly list distinct binds with the same
sequence, and `bind -e` will take mode into account properly as well.
Note that `bind -e seq` now assumes the bind is in the default bind
mode, whereas before it would erase the first binding with that sequence
regardless of mode.

`bind -e -a` still erases all binds in all modes, though `bind -M mode
-e -a` still only erases all binds in the selected mode.

builtin.cpp

@@ -400,41 +400,29 @@ static void builtin_missing_argument(parser_t &parser, const wchar_t *cmd, const
 int builtin_test(parser_t &parser, wchar_t **argv);
 
 /**
-   List all current key bindings
+   List a single key binding.
+   Returns false if no binding with that sequence and mode exists.
  */
-static void builtin_bind_list(const wchar_t *bind_mode)
+static bool builtin_bind_list_one(const wcstring& seq, const wcstring& bind_mode)
 {
-    size_t i;
-    const wcstring_list_t lst = input_mapping_get_names();
-
-    for (i=0; i<lst.size(); i++)
-    {
-        wcstring seq = lst.at(i);
-
     std::vector<wcstring> ecmds;
-        wcstring mode;
     wcstring sets_mode;
 
-        if (! input_mapping_get(seq, &ecmds, &mode, &sets_mode))
+    if (!input_mapping_get(seq, bind_mode, &ecmds, &sets_mode))
     {
-            continue;
-        }
-
-        if (bind_mode != NULL && bind_mode != mode)
-        {
-            continue;
+        return false;
     }
 
     stdout_buffer.append(L"bind");
 
     // Append the mode flags if applicable
-        if (mode != DEFAULT_BIND_MODE)
+    if (bind_mode != DEFAULT_BIND_MODE)
     {
-            const wcstring emode = escape_string(mode, ESCAPE_ALL);
+        const wcstring emode = escape_string(bind_mode, ESCAPE_ALL);
         stdout_buffer.append(L" -M ");
         stdout_buffer.append(emode);
     }
-        if (sets_mode != mode)
+    if (sets_mode != bind_mode)
     {
         const wcstring esets_mode = escape_string(sets_mode, ESCAPE_ALL);
         stdout_buffer.append(L" -m ");
@@ -464,6 +452,27 @@ static void builtin_bind_list(const wchar_t *bind_mode)
         stdout_buffer.append(escaped_ecmd);
     }
     stdout_buffer.push_back(L'\n');
+
+    return true;
+}
+
+/**
+   List all current key bindings
+ */
+static void builtin_bind_list(const wchar_t *bind_mode)
+{
+    const std::vector<input_mapping_name_t> lst = input_mapping_get_names();
+
+    for (std::vector<input_mapping_name_t>::const_iterator it = lst.begin(), end = lst.end();
+         it != end;
+         ++it)
+    {
+        if (bind_mode != NULL && bind_mode != it->mode)
+        {
+            continue;
+        }
+
+        builtin_bind_list_one(it->seq, it->mode);
     }
 }
 
@@ -564,15 +573,21 @@ static int builtin_bind_add(const wchar_t *seq, const wchar_t **cmds, size_t cmd
 
    \param seq an array of all key bindings to erase
    \param all if specified, _all_ key bindings will be erased
+   \param mode if specified, only bindings from that mode will be erased. If not given and \c all is \c false, \c DEFAULT_BIND_MODE will be used.
  */
 static int builtin_bind_erase(wchar_t **seq, int all, const wchar_t *mode, int use_terminfo)
 {
     if (all)
     {
-        const wcstring_list_t lst = input_mapping_get_names();
-        for (size_t i=0; i<lst.size(); i++)
+        const std::vector<input_mapping_name_t> lst = input_mapping_get_names();
+        for (std::vector<input_mapping_name_t>::const_iterator it = lst.begin(), end = lst.end();
+             it != end;
+             ++it)
         {
-            input_mapping_erase(lst.at(i).c_str(), mode);
+            if (mode == NULL || mode == it->mode)
+            {
+                input_mapping_erase(it->seq, it->mode);
+            }
         }
 
         return 0;
@@ -581,6 +596,8 @@ static int builtin_bind_erase(wchar_t **seq, int all, const wchar_t *mode, int u
     {
         int res = 0;
 
+        if (mode == NULL) mode = DEFAULT_BIND_MODE;
+
         while (*seq)
         {
             if (use_terminfo)

doc_src/bind.txt

@@ -43,7 +43,7 @@ The following parameters are available:
 
 - `-m NEW_MODE` or `--sets-mode NEW_MODE` Change the current mode to `NEW_MODE` after this binding is executed
 
-- `-e` or `--erase` Erase the binding with the given sequence and mode instead of defining a new one. Multiple sequences can be specified with this flag. Specifying `-a` or `--all` erases all binds in this mode regardless of sequence.
+- `-e` or `--erase` Erase the binding with the given sequence and mode instead of defining a new one. Multiple sequences can be specified with this flag. Specifying `-a` or `--all` with `-M` or `--mode` erases all binds in the given mode regardless of sequence. Specifying `-a` or `--all` without `-M` or `--mode` erases all binds in all modes regardless of sequence.
 
 - `-a` or `--all` See `--erase` and `--key-names`
 

input.cpp

@@ -772,59 +772,53 @@ wint_t input_readch(bool allow_commands)
     }
 }
 
-wcstring_list_t input_mapping_get_names()
+std::vector<input_mapping_name_t> input_mapping_get_names()
 {
     // Sort the mappings by the user specification order, so we can return them in the same order that the user specified them in
     std::vector<input_mapping_t> local_list = mapping_list;
     std::sort(local_list.begin(), local_list.end(), specification_order_is_less_than);
-    wcstring_list_t result;
+    std::vector<input_mapping_name_t> result;
     result.reserve(local_list.size());
 
     for (size_t i=0; i<local_list.size(); i++)
     {
         const input_mapping_t &m = local_list.at(i);
-        result.push_back(m.seq);
+        result.push_back((input_mapping_name_t){m.seq, m.mode});
     }
     return result;
 }
 
 
-bool input_mapping_erase(const wchar_t *sequence, const wchar_t *mode)
+bool input_mapping_erase(const wcstring &sequence, const wcstring &mode)
 {
     ASSERT_IS_MAIN_THREAD();
     bool result = false;
-    size_t i, sz = mapping_list.size();
 
-    for (i=0; i<sz; i++)
+    for (std::vector<input_mapping_t>::const_iterator it = mapping_list.begin(), end = mapping_list.end();
+         it != end;
+         ++it)
     {
-        const input_mapping_t &m = mapping_list.at(i);
-        if (sequence == m.seq && (mode == NULL || mode == m.mode))
+        if (sequence == it->seq && mode == it->mode)
         {
-            if (i != (sz-1))
-            {
-                mapping_list[i] = mapping_list[sz-1];
-            }
-            mapping_list.pop_back();
+            mapping_list.erase(it);
             result = true;
             break;
-
         }
     }
     return result;
 }
 
-bool input_mapping_get(const wcstring &sequence, wcstring_list_t *out_cmds, wcstring *out_mode, wcstring *out_sets_mode)
+bool input_mapping_get(const wcstring &sequence, const wcstring &mode, wcstring_list_t *out_cmds, wcstring *out_sets_mode)
 {
     bool result = false;
-    size_t sz = mapping_list.size();
-    for (size_t i=0; i<sz; i++)
+    for (std::vector<input_mapping_t>::const_iterator it = mapping_list.begin(), end = mapping_list.end();
+         it != end;
+         ++it)
     {
-        const input_mapping_t &m = mapping_list.at(i);
-        if (sequence == m.seq)
+        if (sequence == it->seq && mode == it->mode)
         {
-            *out_cmds = m.commands;
-            *out_mode = m.mode;
-            *out_sets_mode = m.sets_mode;
+            *out_cmds = it->commands;
+            *out_sets_mode = it->sets_mode;
             result = true;
             break;
         }

input.h

@@ -9,6 +9,7 @@ inputrc information for key bindings.
 #define FISH_INPUT_H
 
 #include <wchar.h>
+#include <utility>
 #include "input_common.h"
 
 
@@ -131,20 +132,25 @@ void input_mapping_add(const wchar_t *sequence, const wchar_t *command,
 void input_mapping_add(const wchar_t *sequence, const wchar_t **commands, size_t commands_len,
                        const wchar_t *mode = DEFAULT_BIND_MODE, const wchar_t *new_mode = DEFAULT_BIND_MODE);
 
+struct input_mapping_name_t {
+    wcstring seq;
+    wcstring mode;
+};
+
 /**
-   Returns all mapping names
+   Returns all mapping names and modes
  */
-wcstring_list_t input_mapping_get_names();
+std::vector<input_mapping_name_t> input_mapping_get_names();
 
 /**
    Erase binding for specified key sequence
  */
-bool input_mapping_erase(const wchar_t *sequence, const wchar_t *mode = DEFAULT_BIND_MODE);
+bool input_mapping_erase(const wcstring &sequence, const wcstring &mode = DEFAULT_BIND_MODE);
 
 /**
-   Gets the command bound to the specified key sequence. Returns true if it exists, false if not.
+   Gets the command bound to the specified key sequence in the specified mode. Returns true if it exists, false if not.
  */
-bool input_mapping_get(const wcstring &sequence, wcstring_list_t *out_cmds, wcstring *out_mode, wcstring *out_new_mode);
+bool input_mapping_get(const wcstring &sequence, const wcstring &mode, wcstring_list_t *out_cmds, wcstring *out_new_mode);
 
 /**
     Return the current bind mode