Add attachCustomKeyEventHandler

[chabou]

What

This PR add a attachCustomKeypressHandler function. It acts exactly like attachCustomKeydownHandler but for keypressevents.

Why

I'm using attachCustomKeydownHandler to prevent xterm to print some hotkeys. I want to intercept these events so I am returning false but I can't use preventDefault() on this keydown event because it will break (Electron) hotkey system. In this case, I need to prevent xterm to consume next keypress event too. But I can't.

How

I decided to duplicate Handlers, one for keydown events and another one for keypress events. The main reason is backward compatibility.
It could be made differently: adding a attachCustomKeyEventsHandler used for Terminal.prototype.keyDown and Terminal.prototype.keyPress. In this case, we need to print a warning in attachCustomKeydownHandler when used to mark it deprecated.

[chabou]

I need to return true in order to test customKeypressHandler behavior. I checked source code and this return value seems unused.

[coveralls]

Coverage increased (+0.1%) to 69.528% when pulling 95aed8f on chabou:customKeypressHandler into 663d3b2 on sourcelair:master.

[Tyriar]

I never really liked custom keydown handler, I wonder if we could do this a different way. I'm also concerned that we may track the input event in the future that may need it's own custom handler as well?

Do you think this would work if customKeydownHandler also applied to keypress/input events? We could then improve the name with a major version change. @chabou was the implementation of your keypress handler going to be exactly the same as the keydown one? Do you foresee issues with this?

[chabou]

Apply this same handler in Terminal.prototype.keyPress was my first implementation and after that I noticed that it was named customKeydownHandler and not customKeyboardHandler 😝
Yes my keypress handler was exactly the same than keydown: (e) => return !isHotkey(e).

I think that all people that using this custom handler, use preventDefault() and will not have their handler called twice for a same user action.

Another way than custom handler could be an ignoredKeys list. My hotkeys are almost static. But it seems equivalent and less powerful than handler.

[chabou]

Let me know how I can modify this PR to suit your needs

[Tyriar]

I did a quick test and it seems to work for what I have as well so what if we use customKeydownHandler for keypress too. @parisk thoughts on deprecating and renaming it customKeyEventHandler for v3?

[parisk]

@Tyriar Sounds sane. What is the API you have in mind for customKeyEventHandler? Will the user be able to hook callbacks on any of the available keyboard events?

[Tyriar]

@parisk that's what I'm thinking. Since it's the actual event as well consumers are free to check the event.type.

@chabou so I think we want a single API customKeyEventHandler which will act the same as customKeydownHandler but for other events too. Then let's mark customKeydownHandler as deprecated and make it an alias for customKeyEventHandler (just call the function from it).

[coveralls]

Coverage increased (+0.3%) to 69.748% when pulling 8e79d53 on chabou:customKeypressHandler into 663d3b2 on sourcelair:master.

[chabou]

A warning message is now written into console when attachCustomKeydownHandler is called:
attachCustomKeydownHandler() is DEPRECATED and will be removed soon. Please use attachCustomKeyEventHandler() instead.

Do you think that customKeydownHandler property can be set directly? Or is this enough ?

[coveralls]

Coverage increased (+0.06%) to 69.748% when pulling d11d746 on chabou:customKeypressHandler into 75b74ac on sourcelair:master.

[Tyriar]

LGTM

[coveralls]

Coverage increased (+0.06%) to 69.679% when pulling 0c7c307 on chabou:customKeypressHandler into 1c84881 on sourcelair:master.

[parisk]

Looks good, thanks!

[chabou]

⚠️ You should rename this PR with attachCustomKeyEventHandler name and not attachCustomKeyEventsHandler. And fix the same thing in Deprecations part of v2.8.0 release note

[Tyriar]

@chabou done 😄