Latest version need to re-register handler in every event

[aVolpe]

With the version 0.1.3 I need to re-register the key handler every time the handler is fired, for example, in the version 0.1.2 this code works:

    setupGlobalKeybindings() {
        let hotKey = HotKey(key: .r, modifiers: [.command, .option])
        hotKey.keyDownHandler = {
            self.handleKeyDown(hotkey: hotKey)
        }
        print("Keybindings initialized")
    }
    
    func handleKeyDown(hotkey: HotKey) {
        print("Pressed at \(Date())")
    }

But in the version 0.1.3 I need to re-register the handler, like this:

    func handleKeyDown(hotkey: HotKey) {
        
        print("Pressed at \(Date())")
        // I really don't know why I must have to do this, but it's the only
        // way that it works
        hotkey.keyDownHandler = {
            self.handleKeyDown(hotkey: hotkey)
        }
    }

[jdisho]

@aVolpe I ended up creating something like this

private extension HotKey {
    func handleKeyDown(_ handler: @escaping (() -> Void)) {
        keyDownHandler = {
            handler()
            self.handleKeyDown(handler)
        }
    }
}

[tlk]

Hello @aVolpe, have you considered storing the hotkey object on an instance variable of an object that is guaranteed to exist during the entire app lifecycle? I believe that would solve the issue.

My thoughts:

• In the setupGlobalKeybindings example above, the hotKey reference is stored in a local function variable and to me this signals that the hotKey reference should be released (and therefore disabled) as soon as the function has been evaluated.
• It is my understanding that the hotKey reference is captured by the keyDownHandler closure because it is referenced from self.handleKeyDown(hotkey: hotKey). Had this not been the case, the hotkey binding would not even work once.
• Not sure what happens exactly when the keyDownHandler closure is evaluated, but judging from your observations my guess is that the hotKey reference is copied into a closure context for future use. After that, the original reference from the setupGlobalKeybindings scope is released and as a result the hotKey object is deinitialized (and disabled).

[aVolpe]

@tlk thats sound right, but it doesn't explain why it works in the version 0.1.2, all I can see in the log is that various methods became public and a new extension to KeyCombo (Sources/HotKey/KeyCombo+System.swift) was added.

[tlk]

@aVolpe Good point :-)

In order to reproduce the issue I created a new project with Xcode 11.3 (macOS, App, Swift, Storyboard, target 10.15), added HotKey as a SPM dependency and added the setupGlobalKeybindings & handleKeyDown code from above. The hotkey works repeatedly and I cannot seem to reproduce the error as reported.

Could the issue be caused by other factors than the library?

[harryworld]

Able to reproduce in Catalyst-based Plugin, but it works on a simple Cocoa-based demo app. Really weird.

[tlk]

@harryworld would you be able to share a small code sample - as small and simple as possible - that reproduces the issue?

[harryworld]

I have created a repo for this
https://github.com/harryworld/TestCatalyst

[tlk]

For some reason Catalyst.appKit is null when I run the TestCatalyst project and setup() is never called. Would you be able to simplify the example even further?

Did the Expected code block work with HotKey v0.1.2?

[aVolpe]

@tlk sorry for the late response.

I can't reproduce the issue with the latest version of xcode with Hotkey 0.1.3.

[harryworld]

@tlk this the simplest setup to run Cocoa code in Catalyst app...
Also, are you running the target destination on Mac?
Anyway, no big deal. I can still use @aVolpe hack in the beginning.
Now, I'm just curious if @aVolpe has resolved the issue and how?

Side note
In my sample project, it is working because I'm using the suggested hack. Please uncomment and replace with Expected Code to reproduce the issue.

[aVolpe]

@harryworld I try to reproduce the issue like the first time, creating a new project, adding the deps using swift packages, and then registering the key, but this time it's working.

[tlk]

@aVolpe No worries, thanks! Interesting.

@harryworld Ok. Yes I ran the TestCatalyst project on macOS Catalina.

Reading the code in MacApp.swift I see the hotKey instance variable on the MacApp class. Looking at Catalyst.swift and FirstViewController.swift the MacApp instance is only requested once (in FirstViewController.swift) and not stored anywhere.

Based on that I would expect the MacApp instance to be deinitialized when FirstViewController.viewDidLoad() is completed unless there are any references to the MacApp instance or the hotKey instance. Consequently, the hotKey instance would also be deinitialized and therefore disabled.

In your "Actual" code block the hotKey instance is captured by the keyDownHandler closure because it is referenced from self.handleKeyDown(hotkey: hotKey). This means that it is somewhat self-referencing, preventing it from being deinitialized, and I believe this is the reason why it works.

Out of curiosity, here is a slightly modified version of your "Actual" code block where the hotKey reference is not captured by the keyDownHandler closure:

// PLEASE DO NOT USE THIS
// Should not be necessary to re-register the keyDownHandler

class MacApp: NSObject, AppKit {
    var hotKey: HotKey?

    func setup() {
        let hotKey = HotKey(key: .a, modifiers: [.control, .shift])
        hotKey.keyDownHandler = {
            self.handleKeyDown()
        }
        self.hotKey = hotKey
    }
    
    func handleKeyDown() {
        print("Pressed at \(Date())")
        key.keyDownHandler = {
            self.handleKeyDown()
        }
    }
}

My guess is that this slightly modified version does not work unless you keep a reference to the MacApp instance stored somewhere during the entire app lifecycle.

If this observation is correct another (better?) solution for you would be to store a reference to the MacApp instance, fx as an instance variable on the FirstViewController. Then your "Expected" code block as well as the code above should work.