Allow map/unmap during mapping execution

Commits e49c0fb04 (unmap: fail if the mapping is currently executing,
2023-05-14) 42be0057a (map: fail if key is currently executing,
2023-06-24) fixed potential use-after-free issues. By doing so,
it broke configurations that in practice have not triggered any
crashes [1] [2].

For example with,

	set -remove global autocomplete insert
	hook global InsertCompletionShow .* %{
	    map window insert <esc> <c-o>
	}
	hook global InsertCompletionHide .* %{
	    unmap window insert <esc> <c-o>
	}

The execution of the <esc> mapping triggers InsertCompletionHide fails
at unmapping. This seems legit and I don't see an obvious alternative
way to write it (InsertIdle would not be correct though it would work
in practice).

Fix the regression by allowing map and unmap again while keeping the
mappings alive until they have finished executing.

Applying map/unmap immediately seems like the most obvious semantics.
Alternatively, we could apply them in between key presses.

[1]: <https://github.com/kak-lsp/kak-lsp/issues/689>
[2]: <https://github.com/alexherbo2/auto-pairs.kak/issues/60>

5 files changed, 8 insertions, 19 deletions

diff --git a/src/commands.cc b/src/commands.cc
index 446ecd29..519d040c 100644
--- a/src/commands.cc
+++ b/src/commands.cc
@@ -1935,7 +1935,7 @@ const CommandDesc unmap_key_cmd = {
 
         if (keymaps.is_mapped(key[0], keymap_mode) and
             (parser.positional_count() < 4 or
-             keymaps.get_mapping_keys(key[0], keymap_mode) == ConstArrayView<Key>{parse_keys(parser[3])}))
+             keymaps.get_mapping_keys(key[0], keymap_mode) == parse_keys(parser[3])))
             keymaps.unmap_key(key[0], keymap_mode);
     }
 };
diff --git a/src/keymap_manager.cc b/src/keymap_manager.cc
index 26579bf7..582a271d 100644
--- a/src/keymap_manager.cc
+++ b/src/keymap_manager.cc
@@ -13,20 +13,12 @@ namespace Kakoune
 void KeymapManager::map_key(Key key, KeymapMode mode,
                             KeyList mapping, String docstring)
 {
-    if (auto it = m_mapping.find(KeyAndMode{key, mode}); it != m_mapping.end())
-        if (it->value.is_executing)
-            throw runtime_error("cannot map key that is currently executing");
     m_mapping[KeyAndMode{key, mode}] = {std::move(mapping), std::move(docstring)};
 }
 
 void KeymapManager::unmap_key(Key key, KeymapMode 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);
+    m_mapping.remove(KeyAndMode{key, mode});
 }
 
 void KeymapManager::unmap_keys(KeymapMode mode)
@@ -48,8 +40,8 @@ bool KeymapManager::is_mapped(Key key, KeymapMode mode) const
            (m_parent and m_parent->is_mapped(key, mode));
 }
 
-KeymapManager::KeymapInfo&
-KeymapManager::get_mapping(Key key, KeymapMode mode)
+const KeymapManager::KeymapInfo&
+KeymapManager::get_mapping(Key key, KeymapMode mode) const
 {
     auto it = m_mapping.find(KeyAndMode{key, mode});
     if (it != m_mapping.end())
diff --git a/src/keymap_manager.hh b/src/keymap_manager.hh
index 9f9a6ab0..208b44f1 100644
--- a/src/keymap_manager.hh
+++ b/src/keymap_manager.hh
@@ -40,9 +40,7 @@ public:
     KeyList get_mapped_keys(KeymapMode mode) const;
 
     auto get_mapping_keys(Key key, KeymapMode mode) {
-        struct Keys : ConstArrayView<Key> { ScopedSetBool executing; };
-        auto& mapping = get_mapping(key, mode);
-        return Keys{mapping.keys, mapping.is_executing};
+        return get_mapping(key, mode).keys;
     }
 
     const String& get_mapping_docstring(Key key, KeymapMode mode) { return get_mapping(key, mode).docstring; }
@@ -60,9 +58,8 @@ private:
     {
         KeyList keys;
         String docstring;
-        NestedBool is_executing{};
     };
-    KeymapInfo& get_mapping(Key key, KeymapMode mode);
+    const KeymapInfo& get_mapping(Key key, KeymapMode mode) const;
 
     KeymapManager()
         : m_parent(nullptr) {}
diff --git a/test/regression/4896-remap-executing-mapping/rc b/test/regression/4896-remap-executing-mapping/rc
index 56624d18..b61a277e 100644
--- a/test/regression/4896-remap-executing-mapping/rc
+++ b/test/regression/4896-remap-executing-mapping/rc
@@ -1,3 +1,3 @@
-try %{ map global user s %exp{:source %%{%val{source}}<ret>} -docstring "re-source my kakrc" }
+map global user s %exp{:source %%{%val{source}}<ret>} -docstring "re-source my kakrc"
 declare-option int source_count
 set-option -add global source_count 1
diff --git a/test/regression/4896-unmap-executing-mapping/out b/test/regression/4896-unmap-executing-mapping/out
index 190a1803..9f358a4a 100644
--- a/test/regression/4896-unmap-executing-mapping/out
+++ b/test/regression/4896-unmap-executing-mapping/out
@@ -1 +1 @@
-123
+123456