Subject: Re: bug#44973: Add a macOS global hotkey function

From: Alan Third <[email-masked]>
Date: Wed, 30 Dec 2020 11:01:09 +0000

On Tue, Dec 29, 2020 at 08:10:00PM -0800, [email-masked] wrote:
> Here is the patch to bind a global hotkey in mac. As long as Emacs is set as
> "trusted" in macOS preferences, the user can bind a two-key hotkey of the
> form [modifier-key] or a single-key modifier [function key], for example
> (mac-bind-global-hotkey [f1] 'tetris). Binding a three-key
> combo is left to a future patch.

Hi, thanks for this.

I'll leave it up to Lars and Eli whether this gets to go in, but I've
got a few comments:

+;; Copyright (C) 1993-1994, 2005-2020 Free Software Foundation, Inc.

Since this is a new file I'm pretty sure we just specify 2020 for
copyright.

+const char *const lispy_to_mac_function_keys[] =
+  {
+    "up", "down", "left", "right",          /* NSUpArrowfunctionKey    0xF700 */
+    "f1", "f2", "f3", "f4", "f5",           /* NSF1FunctionKey         0xF704 */
+    "f6", "f7", "f8", "f9", "f10",

I take it's not possible to use the existing keyboard IO stuff for
this? Like convert_ns_to_X_keysym?

+mac_parse_and_add_hotkey (Lisp_Object key, Lisp_Object lispfn, int hook)
 ^^^

We don't use mac_ in the NS port, we use ns_. I realise this code is
mac specific, but I'd rather not add a new function name prefix, not
least because it's possible it will interfere with the Mac port's
code, but also we may want to extend this functionality to GNUstep in
the future (although that seems unlikely right now).

It also means it's easy to see all the lisp functions that relate to
the NS port by doing something like M-x ns-<TAB>.

If you're copying functionality from the Mac port feel free to set up
an alias, but please use ns for the function names.

+      || ((lisp_modifiers & ctrl_modifier)
+          && EQ (ns_command_modifier, Qcontrol))
+      )
      ^^^

I don't think we leave closing parens hanging like that, just stick it
on the end of the previous line, please.

       doc: /* Bind KEY combination as a global hotkey, and run HOOK upon
+invokation. This function assigns a hotkey that will run an elisp function
 ^^^^^^^^^^

invocation

+mac_bind_key (NSEventModifierFlags modifier, unsigned vkey,

You might want to call this something like ns_bind_global_hotkey so
it's clearer what it does, and a short descriptive comment wouldn't go
amiss.

+    hotkey_ids = [[NSMutableArray alloc] initWithCapacity: 1];

You're allocing this array, but not releasing it anywhere.

+  if (trusted) {
               ^^^

Opening brace should be on a new line. This function (mac_bind_key)
needs this fix in a few places.

+          if([[event.charactersIgnoringModifiers capitalizedString]
            ^^^

Missing space.

We could also do with some documentation.

One thing I'm unsure about is that we allow users to map the left and
right modifier keys separately but I think your code only allows for
matching with left key settings. Is that intentional? It would seem
unlikely that macOS would let us map global hotkeys to the left and
right keys separately, so I understand if it's not possible.
-- 
Alan Third