From cf7c6380254c7c2733952d2899761a2afc881b44 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Mon, 29 May 2023 20:08:02 +1000 Subject: [PATCH] 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 | 22 ++++++---------------- src/hash_map.hh | 9 +++++++++ src/input_handler.cc | 4 +--- src/keymap_manager.cc | 7 ++++++- src/keymap_manager.hh | 22 +++++++++++++++------- src/normal.cc | 6 ++---- 6 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index 84fe8361..1075c332 100644 --- a/src/commands.cc +++ b/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)) - return; - auto& mapping = keymaps.get_mapping(key[0], keymap_mode); - - 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]))) + if (keymaps.is_mapped(key[0], keymap_mode) and + (parser.positional_count() < 4 or + keymaps.get_mapping_keys(key[0], keymap_mode) == ConstArrayView{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); - { - ScopedSetBool executing_mapping{mapping.is_executing}; - for (auto& key : mapping.keys) - context.input_handler().handle_key(key); - } + for (auto& key : context.keymaps().get_mapping_keys(key, mode)) + context.input_handler().handle_key(key); if (lock) enter_user_mode(context, std::move(mode_name), mode, true); diff --git a/src/hash_map.hh b/src/hash_map.hh index 5fad3235..4c1f258f 100644 --- a/src/hash_map.hh +++ b/src/hash_map.hh @@ -328,6 +328,15 @@ struct HashMap return const_cast(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(); } diff --git a/src/input_handler.cc b/src/input_handler.cc index fb1d9ff6..a36d13f1 100644 --- a/src/input_handler.cc +++ b/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); - ScopedSetBool executing_mapping{mapping.is_executing}; - for (auto& k : mapping.keys) + for (auto& k : keymaps.get_mapping_keys(key, keymap_mode)) process_key(k); } else diff --git a/src/keymap_manager.cc b/src/keymap_manager.cc index baec1373..81efe7ed 100644 --- a/src/keymap_manager.cc +++ b/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) diff --git a/src/keymap_manager.hh b/src/keymap_manager.hh index ddb374e4..9f9a6ab0 100644 --- a/src/keymap_manager.hh +++ b/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 - { - KeyList keys; - String docstring; - NestedBool is_executing{}; - }; - KeymapInfo& get_mapping(Key key, KeymapMode mode); + auto get_mapping_keys(Key key, KeymapMode mode) { + struct Keys : ConstArrayView { ScopedSetBool executing; }; + auto& mapping = get_mapping(key, mode); + return Keys{mapping.keys, mapping.is_executing}; + } + + const String& get_mapping_docstring(Key key, KeymapMode mode) { return get_mapping(key, mode).docstring; } using UserModeList = Vector; 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 diff --git a/src/normal.cc b/src/normal.cc index 87bf272a..68392199 100644 --- a/src/normal.cc +++ b/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 : mapping.keys) + for (auto& key : context.keymaps().get_mapping_keys(key, KeymapMode::User)) context.input_handler().handle_key(key); }, "user mapping", build_autoinfo_for_mapping(context, KeymapMode::User, {}));