Re: [PATCH v2] ui/cocoa: Comment about modifier key input quirks

[Gerd Hoffmann]

On Fri, Mar 12, 2021 at 12:12:03AM +0900, Akihiko Odaki wrote:
> Based-on: <20210310042348.21931-1-akihiko.odaki@gmail.com>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>

Well, the comments should not describe what the code is doing, that is
usually pretty clear from reading the code.  The comment should say
*why* the code is doing that, i.e. describe the problem we have to deal
with.

In this case specifically the NSEventTypeFlagsChanged event quirks we
have to deal with:

  (1) we get the keyCode but *not* whenever it was a up or down event.
  (2) we get the modifierFlags but we have only one bit for both shift
      keys so you can't tell whenever the left or right or both shift
      keys are down.

We handle this by using the modifierFlags plus our own state tracking to
generate "up" events, and we have to check both keyCode and
modifierFlags for "down" events.

> -    // emulate caps lock keydown and keyup
> +    /*
> +     * If -[NSEvent modifierFlags] has NSEventModifierFlagCapsLock,
> +     * toggle the current CapsLock state by firing "up" and "down" events in
> +     * sequence.
> +     */

Not very helpful.

> +    /*
> +     * Check for other flags of -[NSEvent modifierFlags].
> +     *
> +     * If a flag is not set, fire "up" events for all keys which correspond 
> to
> +     * the flag. Note that "down" events are not fired here because the flags
> +     * checked here do not tell what exact keys are down.
> +     *
> +     * These operations are performed for any events because the modifier 
> state
> +     * may change while the application is inactive (i.e. no events fire) and
> +     * we want to detect it as early as possible.

Ah, right, (3) for the list above: no updates for inactive apps.

> +     * Up events corresponding to a modifier flag update the current modifier
> +     * state tracked with QKbdState but they are not fired unless all keys 
> which
> +     * match to the flag are up. Therefore, it cannot synchornize Cocoa and
> +     * QkbdState if one of the keys is down. It is still nice that any
> +     * desynchronization can be fixed by completely leaving your hands from 
> the
> +     * keyboard.
> +     */

Better, but description of the NSEventTypeFlagsChanged event issues
should be added to make the motivation for that logic clear.  Feel free
to cut+paste from my lines above.

take care,
  Gerd