bug#19994: 25.0.50; Unicode keyboard input on Windows

[Eli Zaretskii]

> Date: Tue, 3 Mar 2015 15:09:49 -0800
> From: Ilya Zakharevich <nospam-abuse@ilyaz.org>
> 
> I’m working on a patch to make Unicode keyboard input to work properly on
> Windows (in graphic mode).

Thanks!

> The patch below
> 
>   • Implements this “primacy of characters” doctrine;
>  
>   • As far as I could see, is compatible with the current work of Emacs
>     on “simple keyboard layouts”;
>  
>   • Worked at some moment (before I started a massive addition of 
>     comments ;-] — and maybe it is still working, I did not touch it for a
>     month);
>  
>   • (Currently) ignores the indent coding rules;
>  
>   • Passes all the test thrown at it by my super-puper-all-bells-and-whistles
>     layouts; see e.g.
>        http://k.ilyaz.org/windows/izKeys-visual-maps.html#examples

Any chance of coming up with a few tests for this code, and adding
them to the test/ directory?

> If I ever find more time to work on it, I plan to:
>
>   1) Add yet more documentation;
> 
>   2) Change a little bit the logic of detection of consumed/extra 
>      modifiers.  This change may be cosmetic only — or maybe, with some 
>      extremely devilous layouts, it may be beneficial.
>     
>      (I have not seen layouts where this change would matter, though!
>       And I looked though the source code of hundred(s).)
> 
>   3) Bring it in sync with the Emacs coding style.

I suggest, indeed, to clean up the code so we could commit it to the
master branch.  That way, it will get wider testing, and we can fix
whatever problems it might cause.  Any deficiencies that don't cause
regressions wrt the current code can be fixed later, or even not at
all (if we decide them to not be important enough).

Question: did you try this code with IME input methods?

> Meanwhile, I would greatly appreciate all input related to the current 
> state of the patch.

Some of that (but not much) below.

> +static int
> +get_wm_chars (HWND aWnd, int *buf, int buflen, int ignore_ctrl, int ctrl, int
                            ^^^^^^^^
Why 'int' and not 'wchar_t'?

> +  while (buflen &&                             /* Should be called only when 
>  w32_unicode_gui */
> +         PeekMessageW(&msg, aWnd, WM_KEYFIRST, WM_KEYLAST, PM_NOREMOVE | 
> PM_NOYIELD) &&

Indeed, any "wide" APIs should only be called when w32_unicode_gui is
on, and there should be alternative code for when w32_unicode_gui is
off.  We still try to support Windows 9X.

> +      if (msg.message == WM_UNICHAR || code_unit < 0xDC00 || code_unit > 
> 0xDFFF) {
> +        /* Mismatched first surrogate.  Pass both code units as if they were 
> two characters. */
> +        *buf++ = doubled;
> +        if (!--buflen) // Drop the second char if at the end of the buffer
> +          return i;
> +      } else {
> +        code_unit = (doubled << 10) + code_unit - 0x35FDC00;
> +      }
> +      doubled = 0;
> +    } else if (code_unit >= 0xD800 && code_unit <= 0xDBFF) {

Either explain the "magic" constants in comments, or, better, use
macros with descriptive names.

> +  int ctrl_cnt, buf[1024], count, is_dead;

I think buf[] should be an array of wchar_t.  Also, will this code
work for the non-w32_unicode_gui mode?

> +  if (count) {
> +    W32Msg wmsg;
> +    int *b = buf, strip_Alt = 1;

Likewise with 'b'.

> +      SHORT r = VkKeyScanW( *b );

VkKeyScanW should be called only if w32_unicode_gui is on.  (Or maybe
the caller is only called when w32_unicode_gui is on, in which case
maybe we should have an eassert there.)

> +      fprintf(stderr, "VkKeyScanW %#06x %#04x\n", (int)r, wParam);
> +      if ((r & 0xFF) == wParam && !(r & ~0x1FF)) {     /* Char available 
> without Alt modifier, so Alt is "on top" */
> +         if (*b > 0x7f && ('A' <= wParam && wParam <= 'Z'))
> +           return 0;                                   /* Another branch 
> below 
> would convert it to Alt-Latin char via wParam */        
> +         strip_Alt = 0;
> +      }
> +    }
> +    if (strip_Alt)
> +      wmsg.dwModifiers = wmsg.dwModifiers & ~(alt_modifier | meta_modifier);
> +    
> +    signal_user_input ();
> +    while (count--)
> +      {
> +        fprintf(stderr, "unichar %#06x\n", *b);
> +        my_post_msg (&wmsg, hwnd, WM_UNICHAR, *b++, lParam);
> +      }
> +    if (!ctrl_cnt)     /* Process ALSO as ctrl */
> +      return 1;
> +    else
> +        fprintf(stderr, "extra ctrl char\n");
> +    return -1;
> +  } else if (is_dead >= 0) {
> +      fprintf(stderr, "dead %#06x\n", is_dead);
> +      return 1;
> +  }

Lots of debugging output here that should be removed.

Thanks again for working on this.