Subject: Re: bug#74423: Low level key events

From: Stefan Monnier <[email-masked]>
Date: Fri, 03 Jan 2025 23:55:49 -0500

> In this new version I changed the way events are handled.
> Now llk-handle generates input events to be used with normal
> keymaps, instead of running a command. The function llk-bind
> activates event generation for a key and a combination of
> events press, release, double, triple.

I really like what I see.  As I think I mentioned, I had made a related
effort in local patches, but I couldn't figure out back then how to make
the low-level events useful.

I wouldn't be surprised if such low-level events remain limited in their
use, but I suspect that some specific uses of them could become very
popular.

I haven't seen many opinions of late on your code (other than my
own), sadly.  I think we need some more reviewers here.

> You suggested to auto generate the keysym table, but we
> would still need a table to relate the Windows keys to the X
> equivalent.

I don't doubt some part has to be done manually, but I think we should
strive to make it via code as much as possible.

E.g. on each platform we should try to provide a programmatic way to map
the integer key to/from a meaningful system-specific string or symbol
(at least, to the extent that the underlying system provides such
a functionality).

Mapping between those symbols in different systems will likely be (at
least partly) manual, inevitably, but I'm hoping this can be
significantly smaller (especially I'm hoping we can "align" the
system-specific symbols such that many are already identical between the
different systems).

Finally it's not 100% indispensable to have unified symbols that work
identically on all systems: users could also just use the system-specific
names.  They'd need a mapping between them only for code/configuration
that is used with different systems.  So it's only really important when
those names start to be used in libraries.

> +@cindex @code{low-level-key} event
> +@item (low-level-key @var{is-key-press} @var{key} @var{modifier} @var{time} @var{frame})
> +This event is sent on the physical press or release of keys, only on
> +systems where it is supported, currently X, MS-Windows and PGTK, and
> +only if the variable @code{enable-low-level-key-events} has a
> +non-@code{nil} value.  See its documentation for the values to use, that
> +can activate the events for all or some keys.
> +
> +@var{is-key-press} is @code{t} for a key press, @code{nil} for a key
> +release.  @var{time} is the event's time in milliseconds, as reported by
> +the underlying platform, and should only be used to measure time
> +intervals between events of this same kind.

[ This made me wonder if those timestamps can be compared between
  events coming from different X11 servers.  ]

> @var{frame} is the frame +receiving the event.

Any chance this could be changed to a window?
[ What does it even mean?
  Usually we associate keyboard events with the `window-point` of the
  currently selected window (and they don't come with the information
  attached), whereas we associated mouse events with the
  position of the mouse cursor.
  But when I do `down-meta down-mouse-1 up-mouse-1 up-meta` to generate
  a `M-mouse-1` event, the events on the `meta` key end up associated
  with the position of the mouse cursor even tho they come from
  the keyboard.  ]

> @var{modifier} is @code{nil} if the key is not a
> +modifier key, @code{t} if it is, but it is unknown which one, or one of
> +@code{shift}, @code{control}, @code{meta}, @code{alt}, @code{super},
> +@code{hyper}.

In the docstring you mention that PGTK can't distinguish modifiers.
It would be useful here to mention here also the kinds of reasons why we
might get just `t`: can it happen for some keys and not others, or is it
the case for all the events of a given kind of terminal?

> Loading @file{low-level-key.el} sets a handler
> +that can generate normal input events for key press, release and multi
> +tap.  See the function @code{llk-bind}.

Any other ELisp code could do a similar task, tho.  So, I'd probably
present it a bit differently, like:

    Because generating these events for all keys would interfere with
    normal keyboard input, they are by default filtered out by a binding in
    `special-event-map`.
    @file{low-level-key.el} is a package that lets you configure
    which of those events to turn into normal input events.  It is also
    able to recognize some specific patterns such a (multi) taps.

[ BTW, you might like to explain what is a (multi) tap here, as well.  ]

> +(cl-defstruct (low-level-key (:type list))

I recommend you add `(:constructor nil)` and `(:copier nil)` so the
macro doesn't auto-define `make-low-level-key` and
`copy-low-level-key` functions.

> +(defvar llk-bindings nil
> +  "List of bindings for low level key events (press/release/tap).
> +Use the `llk-bind' function to add bindings.  See its documentation for
> +a description of the binding information.")

[ Seeing how it's used, I suggest you use a hash-table instead of
  a list.  ]

> +EVENTS are symbols that activate one event. Possible values are `press',
> +`release', `double' and `triple'.

Any reason why there isn't an option to emit an event for a single
press+release?

> +  (llk-bind xk-shift-l \\='double)
> +
> +will generate the event `double-xk-shift-l', than can be bound to a
> +command with:

AFAICT the event will be named `double-key-xk-shift-l`.
Did I miss something?
[ I like this "key-" in the event name, FWIW, so I suggest to keep the
  code but fix the doc rather than the reverse.  Tho, if we can drop
  the `xk-`, I'd be happier yet.
  Maybe the generated event names should have "llk-" in them
  instead of "key-".  ]

> +(defun describe-low-level-key ()
> +  "Wait for key press and describe the low-level key event it generates."
> +  (interactive)
> +  (define-key special-event-map [low-level-key] 'llk--describe))

This is brittle.  I don't really have a better suggestion, sadly, but it
deserves a comment.

> +(setq llk-bindings nil)

Why?

Also the file lacks the usual end of file marker (and the call to `provide`).

I'll try and take a look at the C code later.

        Stefan