Re: [PATCH] Add IME status change support on windows natively

[Eli Zaretskii]

> From: "Albert" <address@hidden>
> Date: Mon, 13 Apr 2020 12:09:00 +0800
> 
>     I added some code on src/w32fns.c and src/w32term.h to add IME status 
> change support on windows
> natively like gVim did. gVim can change IME status to chinese mode  in insert 
> mode and switch back to
> english mode when in normal mode natively.

Thanks.  Can you please tell more about the uses of this feature?  How
would a user of Emacs use this, and what is the effect of using this
on editing inside Emacs?

I'm asking because we probably need to say something about this in the
manual.

>       (add-hook 'evil-insert-state-entry-hook 'emacs-ime-enable)
>       (add-hook 'evil-insert-state-exit-hook 'emacs-ime-disable)

I don't use Evil, so I'm not sure I understand what this does.  If you
describe the typical uses of this feature, I will probably understand
better.  In particular, why is this placed on a hook?

> +/* emacs ime status change flag, used in mainThread and windowsThread */
> +static int w32_ime_status_changed = 0;
> +static int w32_emacs_ime_status = 0;

Can we do this without static variables?  E.g., can you use the 2
parameters of the PostThreadMessage API to pass these flags?  I think
that would be cleaner.

> +DEFUN ("w32-get-ime-open-status",
> +       Fw32_get_ime_open_status, Sw32_get_ime_open_status,
> +       0, 0, 0,
> +       doc: /* Return ime open status on Windows. */)

The doc string is too terse, it should say something about the
returned status and its significance.

> +DEFUN ("w32-set-ime-open-status",
> +       Fw32_set_ime_open_status, Sw32_set_ime_open_status,
> +       1, 1, 0,
> +       doc: /* Set emacs IME open status on Windows. */)

Likewise here.  AFAIU, this function doesn't "set the status", it
actually opens or closes the IME.  If so, the doc string should say
that, and it should explicitly mention the argument of the function
and its significance.

Thank you for working on this.