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.

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

server: Notify on all keyboard events with keysym #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nob13
Copy link

@nob13 nob13 commented Jan 10, 2025

Right now users of neatvnc only receive either a

  • key event: containing a keysym (regular Keyboard Event)
  • key_code event: not containing a keysym, but a linux key code (coming QEMU-Event)

on keyboard interactions.

Unfortunately the QEMU-key-code event lacks the keysym, which is present in the transport protocol. Without it, it's tricky to differentiate between different keyboard layouts. (E.g. " and Ä are the same Key in German and English Layout) and the server has to reconstruct it.

This PR adds another callback, which contains the keycode and keysym at the same time and is fired on both events.

I know this is probably not the best way, but extending the other callbacks would be API-Breaking.

I have read and understood CONTRIBUTING.md.

@any1
Copy link
Owner

any1 commented Jan 10, 2025

If I understand correctly, you want to guess the client's keyboard layout based on symbol and key code pairs?

@nob13
Copy link
Author

nob13 commented Jan 10, 2025

If I understand correctly, you want to guess the client's keyboard layout based on symbol and key code pairs?

No, right now I rely solely on keysym on insertion of regular text, which already has the correct keyboard layout. However keysym is not available on key_code callback, although it's present in the protocol.

@any1
Copy link
Owner

any1 commented Jan 10, 2025

I see.

The API should not expose details about extensions if at all possible.

How about adding an else for this: https://github.com/any1/neatvnc/blob/master/src/server.c#L1053 that calls key_fn instead if key_code_fn isn't defined?

@nob13
Copy link
Author

nob13 commented Jan 10, 2025

How about adding an else for this: https://github.com/any1/neatvnc/blob/master/src/server.c#L1053 that calls key_fn instead if key_code_fn isn't defined?

If the keysym is the only interesting thing, this would be enough, however it has also it's downsides:

  • What if the library needs keycode and keysym?
  • If the user is registering key_code_fn, then suddenly he doesn't receive key_fn anymore, which would be counter-intuitive behaviour.
  • Calling both callbacks at the same time would also not be intuitive, because it would be up to the user to figure out that these two calls are related to the same key.

@any1
Copy link
Owner

any1 commented Jan 10, 2025

  • What if the library needs keycode and keysym?

That's a hypothetical. It's better to look into this again if the need actually arises.

  • If the user is registering key_code_fn, then suddenly he doesn't receive key_fn anymore, which would be counter-intuitive behaviour.

These callbacks are not supposed to change after initialisation. They should only be set once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants