Re: [PATCH] Override Windows default Win-* key combinations when using Emacs

[Eli Zaretskii <eliz@gnu.org>]

> Cc: emacs-devel@gnu.org
> From: Jussi Lahdenniemi <jussi@aprikoodi.fi>
> Date: Sat, 9 Jan 2016 21:58:52 +0200
> 
> On 5.1.2016 19.05, Eli Zaretskii wrote:
> > Thank you for your contribution.  It is large enough to require legal
> > paperwork; I can send you the forms off-list if you agree.
> 
> I finished integrating the code more tightly into Emacs sources, 
> removing the need for the additional process and adding functionality 
> for grabbing Alt-based keyboard input (M-tab, M-ESC) in addition to the 
> Windows keys. I also updated the documentation.

Thanks.

> The patch is attached. I decided to place the new functionality in a new 
> file (w32hook.c) - it could have been added to some existing file as well.

I'd prefer not to create a new file, but to have this in w32fns.c.  It
might even allow you to make some functions static.

> -@findex w32-register-hot-key
> -@findex w32-unregister-hot-key
> -  MS-Windows reserves certain key combinations, such as

I'm not sure I understand why you moved this stuff down in the file.
Was something wrong with its original place?  You could make the
changes without moving the stuff, no?

> +   The hotkey registrations always also include all the shift and
> +control modifier combinations for the given hotkey; that is,
> +registering @kbd{s-@key{a}} as a hotkey gives you @kbd{s-@key{A}},
> +@kbd{C-s-@key{a}} and @kbd{C-s-@key{A}} as well.

I believe you meant to use "C-S-" etc. here, for the Shift key (not
"super").

> -		  if (NUMBERP (Vw32_phantom_key_code))
> -		    key = XUINT (Vw32_phantom_key_code) & 255;
> -		  else
> -		    key = VK_SPACE;

It looks like you are removing the support for this feature.  Is it
really not needed anymore?  If so, we should mention this change in
NEWS, and tell there how to achieve the same effect without this
variable.

> +/* The Windows keyboard hook callback */

Comments should have a period at the end of a sentence, and 2 spaces
after that.

> +static LRESULT CALLBACK funhook( int code, WPARAM w, LPARAM l )

A blank between the function name and the left parenthesis, and no
blank after the parenthesis.  Same near the right parenthesis.

I've briefly reviewed the rest of the patch, and it looks OK to me.
Did you make sure the new code doesn't call any API that might be
unavailable on older systems?  We still support Windows 98 and later.

Also, I understand that this code will work on versions of Windows
before v8, is that right?

Thanks.