Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

'unmap': cannot unmap key that is currently executing #689

Closed
altunyurt opened this issue Jul 4, 2023 · 4 comments
Closed

'unmap': cannot unmap key that is currently executing #689

altunyurt opened this issue Jul 4, 2023 · 4 comments
Labels
external dependency Something need to be fixed upstream

Comments

@altunyurt
Copy link

altunyurt commented Jul 4, 2023 •
edited

I'm not really sure if it's directly related to kak-lsp but sometimes completion fails and leaves kakoune unresponsive for mode switch, esc key does not work.

I see the following logs in the debug buffer.

error running hook InsertCompletionHide(3.39,3.58)/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing
error running hook InsertCompletionHide()/: 4:5: 'unmap': cannot unmap key that is currently executing

There's another similar issue on autopairs repo alexherbo2/auto-pairs.kak#60 referring to this commit in kakoune tree mawww/kakoune@e49c0fb

Parent commit of the one above, seems not to have this issue and works fine.
@krobelus
Copy link
Member

krobelus commented Jul 5, 2023

wow how has that bug report not reached me for one month; thanks for reporting this
can you post your InsertCompletionHide hook? Maybe there is a workaround. Else we could allow map/unmap during mapping execution

@altunyurt
Copy link
Author

Here're the hooks for show and hide

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

krobelus added a commit to krobelus/kakoune that referenced this issue Jul 6, 2023
@krobelus
Allow map/unmap during mapping execution
999dba5 
Commits e49c0fb (unmap: fail if the mapping is currently executing,
2023-05-14) 42be005 (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 garbage collection. This is similar to how we
treat hooks and others.
To-do: we can probably clean up the KeymapManager a bit.

[1]: <kakoune-lsp/kakoune-lsp#689>
[2]: <alexherbo2/auto-pairs.kak#60>
krobelus added a commit to krobelus/kakoune that referenced this issue Jul 6, 2023
@krobelus
Allow map/unmap during mapping execution
89cca38 
Commits e49c0fb (unmap: fail if the mapping is currently executing,
2023-05-14) 42be005 (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 garbage collection. This is similar to how we
treat hooks and others.

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

To-do: we can probably clean up the KeymapManager a bit.

[1]: <kakoune-lsp/kakoune-lsp#689>
[2]: <alexherbo2/auto-pairs.kak#60>
@krobelus
Copy link
Member

krobelus commented Jul 6, 2023

seems legit, mawww/kakoune#4945 proposes a fix

@krobelus krobelus mentioned this issue Jul 7, 2023
Merged
krobelus added a commit to krobelus/kakoune that referenced this issue Jul 8, 2023
@krobelus
Allow map/unmap during mapping execution
98b5b70 
Commits e49c0fb (unmap: fail if the mapping is currently executing,
2023-05-14) 42be005 (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 garbage collection. This is similar to how we
treat hooks and others.

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

To-do: we can probably clean up the KeymapManager a bit.

[1]: <kakoune-lsp/kakoune-lsp#689>
[2]: <alexherbo2/auto-pairs.kak#60>
krobelus added a commit to krobelus/kakoune that referenced this issue Jul 9, 2023
@krobelus
Allow map/unmap during mapping execution
b3146d6 
Commits e49c0fb (unmap: fail if the mapping is currently executing,
2023-05-14) 42be005 (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 garbage collection. This is similar to how we
treat hooks and others.

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

To-do: we can probably clean up the KeymapManager a bit.

[1]: <kakoune-lsp/kakoune-lsp#689>
[2]: <alexherbo2/auto-pairs.kak#60>
krobelus added a commit to krobelus/kakoune that referenced this issue Jul 20, 2023
@krobelus
Allow map/unmap during mapping execution
1231041 
Commits e49c0fb (unmap: fail if the mapping is currently executing,
2023-05-14) 42be005 (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]: <kakoune-lsp/kakoune-lsp#689>
[2]: <alexherbo2/auto-pairs.kak#60>
@altunyurt
Copy link
Author

Apparently the fix is merged into the main tree, and i haven't had experience any problems yet. Thank you.

@krobelus krobelus added the external dependency Something need to be fixed upstream label Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
external dependency Something need to be fixed upstream
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@altunyurt @krobelus