Fixing escape key getting filtered on Windows

[Neko-Box-Coder]

This should fix #2947

I will put a comment once I have tested this on a Windows and Linux machine.

[Neko-Box-Coder]

Just tested both on Windows and Linux. It indeed fixes the problem for Windows and doesn't affect Linux. Someone else can test this if needed. Otherwise I think we can merge this small fix @dmaluka @JoeKar

[Neko-Box-Coder]

I have changed the test to simulate windows behavior to get it pass. I have searched the codebase on how we are handling escape for a few hours but just left me scratching my head to be honest.

I don't understand why it breaks for Linux when rune is set to 27 (that special case) because it is just additional information, we still have the key code set to escape anyway.

And somehow Windows is the exact opposite which requires that extra rune.

I guess it is because we are doing exact match so that additional rune information causes it to not match? I have no idea.

I am open to any suggestions, I have tested it both on Linux and Windows so it is working.

[Neko-Box-Coder]

@JoeKar So currently the proposed solution is to create different event key depending on OS which we match later when user invokes any event.

Alternatively, I can try to handle the escape case when we process the event like what I proposed similarly in #3305 where I tried different combinations (i.e. both the escape keycode or the escape rune or both) unless we match something.

In which case we can avoid an OS check all together and I don't need to modify the test file.

However, this still doesn't change the fact that we are handling special cases and the more I think about it, the less I am sure if this is an upstream problem as tcell is just spitting out things on what it receives without modifying it (mostly)

I am happy to do whatever to get this merged quickly. I do apologize to be frequently replying and raising issues/PRs as I am an impatient guy :sweat_smile:

Would the solution I mentioned above be better or do we need more time for people to decide what to do with this?

[JoeKar]

So currently the proposed solution is to create different event key depending on OS which we match later when user invokes any event.

No, I proposed that micro should receive the same key under both OSs and that this should be abstracted by tcell. tcell should generate it either with or without rune for both, but not different between those. I already tried it without the rune in Windows, where it then worked without further modification of micro.

[gdamore]

If this is different between windows and everything else it's a bug in tcell. ESC should never be a rune.

I will check as soon as I can.

• Garrett On May 24, 2024 at 10:57 AM -0700, Jöran Karl @.***>, wrote:

So currently the proposed solution is to create different event key depending on OS which we match later when user invokes any event. No, I proposed that micro should receive the same key under both OSs and that this should be abstracted by tcell. tcell should generate it either with or without rune for both, but not different between those. I already tried it without the rune in Windows, where it then worked without further modification of micro. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[Neko-Box-Coder]

@JoeKar Yep cool. I can apply what you proposed to tcell to fix this problem. But we will need to update tcell. I don't think I can get my PR merged quickly if I want to update the one we are using, unless @zyedidia is available to do so.

Otherwise should I try to get micro working with upstream tcell or do we want to fork our current tcell and apply the changes?

[JoeKar]

@JoeKar Yep cool. I can apply what you proposed to tcell to fix this problem.

You can patch your local copy of tcell with win-esc.patch.txt and rebuild micro. I was waiting for the confirmation/correction upstream, to directly propose the original backport.

But we will need to update tcell. I don't think I can get my PR merged quickly if I want to update the one we are using, unless @zyedidia is available to do so.

Correct, only he can do the merges in the currently used tcell.

Otherwise should I try to get micro working with upstream tcell or do we want to fork our current tcell and apply the changes?

No, the upstream version wouldn't work oob, since the fork was done to add some additional changes not present upstream (e.g. the raw escape sequences). Zachary already created a much newer fork (see micro-editor/tcell vs zyedidia/tcell) within the micro-editor organization. Maybe it's common in the near future, that we switch to that. But even there we can't perform merges yet.

[JoeKar]

Done with #3337, since tcell's documentation wasn't considered correctly and undefined runes have been used, as found out by @dmaluka.