Refactor KeymapManager to enfore setting is_executing on key iteration

Add an iterator based remove to HashMap as that was missing. Make
KeymapManager responsible for throwing on unmap an executing mapping.

src/commands.cc

@@ -1598,7 +1598,7 @@ const CommandDesc debug_cmd = {
                 KeymapMode m = parse_keymap_mode(mode, user_modes);
                 for (auto& key : keymaps.get_mapped_keys(m))
                     write_to_debug_buffer(format(" * {} {}: {}",
-                                          mode, key, keymaps.get_mapping(key, m).docstring));
+                                          mode, key, keymaps.get_mapping_docstring(key, m)));
             }
         }
         else if (parser[0] == "regex")
@@ -1933,15 +1933,9 @@ const CommandDesc unmap_key_cmd = {
         if (key.size() != 1)
             throw runtime_error("only a single key can be unmapped");
 
-        if (not keymaps.is_mapped(key[0], keymap_mode))
+        if (keymaps.is_mapped(key[0], keymap_mode) and
-            return;
+            (parser.positional_count() < 4 or
-        auto& mapping = keymaps.get_mapping(key[0], keymap_mode);
+             keymaps.get_mapping_keys(key[0], keymap_mode) == ConstArrayView<Key>{parse_keys(parser[3])}))
-
-        if (mapping.is_executing)
-            throw runtime_error("cannot unmap key that is currently executing");
-
-        if (parser.positional_count() < 4 or
-             (mapping.keys == parse_keys(parser[3])))
             keymaps.unmap_key(key[0], keymap_mode);
     }
 };
@@ -2651,7 +2645,6 @@ void enter_user_mode(Context& context, String mode_name, KeymapMode mode, bool l
         if (not context.keymaps().is_mapped(key, mode))
             return;
 
-        auto& mapping = context.keymaps().get_mapping(key, mode);
         ScopedSetBool disable_keymaps(context.keymaps_disabled());
         ScopedSetBool disable_history(context.history_disabled());
 
@@ -2659,11 +2652,8 @@ void enter_user_mode(Context& context, String mode_name, KeymapMode mode, bool l
 
         ScopedEdition edition(context);
 
-        {
+        for (auto& key : context.keymaps().get_mapping_keys(key, mode))
-            ScopedSetBool executing_mapping{mapping.is_executing};
-            for (auto& key : mapping.keys)
             context.input_handler().handle_key(key);
-        }
 
         if (lock)
             enter_user_mode(context, std::move(mode_name), mode, true);

src/hash_map.hh

@@ -328,6 +328,15 @@ struct HashMap
         return const_cast<HashMap*>(this)->find(key);
     }
 
+    constexpr void remove(const const_iterator& it)
+    {
+        auto index = it - m_items.begin();
+        const auto hash = hash_value(it->key);
+        m_index.remove(hash, index);
+        m_items.erase(it);
+        m_index.ordered_fix_entries(index);
+    }
+
     constexpr void clear() { m_items.clear(); m_index.clear(); }
 
     constexpr size_t size() const { return m_items.size(); }

src/input_handler.cc

@@ -1774,9 +1774,7 @@ void InputHandler::handle_key(Key key)
     {
         ScopedSetBool disable_history{context().history_disabled()};
 
-        auto& mapping = keymaps.get_mapping(key, keymap_mode);
+        for (auto& k : keymaps.get_mapping_keys(key, keymap_mode))
-        ScopedSetBool executing_mapping{mapping.is_executing};
-        for (auto& k : mapping.keys)
             process_key(k);
     }
     else

src/keymap_manager.cc

@@ -18,7 +18,12 @@ void KeymapManager::map_key(Key key, KeymapMode mode,
 
 void KeymapManager::unmap_key(Key key, KeymapMode mode)
 {
-    m_mapping.remove(KeyAndMode{key, mode});
+    auto it = m_mapping.find(KeyAndMode{key, mode});
+    if (it == m_mapping.end())
+        return;
+    if (it->value.is_executing)
+        throw runtime_error("cannot unmap key that is currently executing");
+    m_mapping.remove(it);
 }
 
 void KeymapManager::unmap_keys(KeymapMode mode)

src/keymap_manager.hh

@@ -39,13 +39,13 @@ public:
     bool is_mapped(Key key, KeymapMode mode) const;
     KeyList get_mapped_keys(KeymapMode mode) const;
 
-    struct KeymapInfo
+    auto get_mapping_keys(Key key, KeymapMode mode) {
-    {
+        struct Keys : ConstArrayView<Key> { ScopedSetBool executing; };
-        KeyList keys;
+        auto& mapping = get_mapping(key, mode);
-        String docstring;
+        return Keys{mapping.keys, mapping.is_executing};
-        NestedBool is_executing{};
+    }
-    };
+
-    KeymapInfo& get_mapping(Key key, KeymapMode mode);
+    const String& get_mapping_docstring(Key key, KeymapMode mode) { return get_mapping(key, mode).docstring; }
 
     using UserModeList = Vector<String>;
     UserModeList& user_modes() {
@@ -56,6 +56,14 @@ public:
     void add_user_mode(String user_mode_name);
 
 private:
+    struct KeymapInfo
+    {
+        KeyList keys;
+        String docstring;
+        NestedBool is_executing{};
+    };
+    KeymapInfo& get_mapping(Key key, KeymapMode mode);
+
     KeymapManager()
         : m_parent(nullptr) {}
     // the only one allowed to construct a root map manager

src/normal.cc

@@ -191,7 +191,7 @@ String build_autoinfo_for_mapping(const Context& context, KeymapMode mode,
 
     for (auto& key : keymaps.get_mapped_keys(mode))
         descs.emplace_back(to_string(key),
-                           keymaps.get_mapping(key, mode).docstring);
+                           keymaps.get_mapping_docstring(key, mode));
 
     auto max_len = 0_col;
     for (auto& desc : descs)
@@ -2025,7 +2025,6 @@ void exec_user_mappings(Context& context, NormalParams params)
         if (not context.keymaps().is_mapped(key, KeymapMode::User))
             return;
 
-        auto& mapping = context.keymaps().get_mapping(key, KeymapMode::User);
         ScopedSetBool disable_keymaps(context.keymaps_disabled());
         ScopedSetBool disable_history(context.history_disabled());
 
@@ -2033,8 +2032,7 @@ void exec_user_mappings(Context& context, NormalParams params)
 
         ScopedEdition edition(context);
         ScopedSelectionEdition selection_edition{context};
-        ScopedSetBool executing_mapping{mapping.is_executing};
+        for (auto& key : context.keymaps().get_mapping_keys(key, KeymapMode::User))
-        for (auto& key : mapping.keys)
             context.input_handler().handle_key(key);
     }, "user mapping",
     build_autoinfo_for_mapping(context, KeymapMode::User, {}));