Hammerspoon: Make chooser to refocus previous window when disappeared

[casouri]

Currently, if a chooser hides away, it doesn't return the focus to the previously focused window.

I consider returning the focus as a pretty much well-accepted behavior for a popup menu.

What do you think?

Emoji Spoon already implemented the "refocus" feature as mentioned in author's blog, I didn't read his code though.

[latenitefilms]

We currently do this manually using the hs.chooser callbacks - but yes, I agree, this would be awesome if it was built into hs.chooser.

@casouri - If you're feeling up to it, here's some other hs.chooser improvements I'd LOVE to see:

• Feature Request - Tabs for Chooser #1681
• Add hs.styledtext support to hs.chooser #1624

[casouri]

OK, I'll try to add refocus to all the choosers I use for now. And I need to get at least familiar with lua and macOS's API before I can do any substantial work.

[casouri]

I'm really interested in #1624, if you do any work please let me know. I might have a try on that one in summer (since I'm a student).

[latenitefilms]

@casouri - Will do, however, I think #1624 is a bit beyond my skillset too. I'll have another look, and maybe @cmsj or @asmagill can point me in the right direction.

[cmsj]

I note that Quicksilver is open source. It's a long while since I've used it, but if they have the right behaviour, maybe we can learn what they're doing, from their source. Anyone up for spelunking into their codebase?

[casouri]

I think it would be much easier to add a global callback for chooser before it appears and after it disappears. And do the work in the callback.

This way we can use already existed hammerspoon API and avoid messing with the objective-c code.

[cmsj]

@casouri I'm still fine with adding the callbacks, but the focus handling is the kind of thing that probably every single user would want, so to me it makes more sense to do it in the ObjC.

[asmagill]

@cmsj, I do like the idea of a global callback as I can think of some things I might want to check or do during build up and tear down, and it can by default be one which records the last focused window and then re-focus upon close, but I kind of disagree about doing the focus/unfocus in Objective-C because it adds complexity... in my mind, what can be done in Lua without a penalty in time or difficulty, should be done in Lua because it's easier to debug and doesn't fail in inscrutable ways (or at least in ways that the average user can't even begin to troubleshoot.)

The default callback could be as simple as (I haven't tested this, so it may need tweaking; especially see the comments in the "didClose" section):

~~~lua
-- this assumes a single callback for willOpen/didClose; adjust depending upon how callback
-- is implemented
hs.chooser._globalChooserCallback = function(whichChooser, state)
-- create focus store if it doesn't already exist
if not hs.chooser._lastFocused then hs.chooser._lastFocused = {} end

if state == "willOpen" then
hs.chooser._lastFocused[whichChooser] = hs.window.frontmostWindow()
elseif state == "didClose" then
-- assumes the callback for open and close gets the exact same userdata for the
-- chooser object, so I suspect this won't work... see comments below

-- might be nil if no window was focused before the chooser was opened, so check
if hs.chooser._lastFocused[whichChooser] then
  hs.chooser._lastFocused[whichChooser]:focus()
  hs.chooser._lastFocused[whichChooser] = nil
end
-- if the userdata's are distinct lua objects referring to the same objective-c object, which is
-- actually the more common approach used in Hammerspoon modules because it allows for
-- automated garbage collection in the objective-c side when lua GC occurs, use this instead:

-- local initialChooserUserdata = nil
-- for k, v in pairs(hs.chooser._lastFocused) do
--   if k == whichChooser then -- assumes userdata implements __eq method
--     initialChooserUserdata = k
--     break
--   end
-- end
-- -- might not be found if no window was focused before the chooser was opened, so check
-- if initialChooserUserdata then
--   hs.chooser[initialChooserUserdata]:focus()
--   hs.chooser[initialChooserUserdata] = nil
-- end

else
hs.printf("** unrecognized state for chooser: %s", state)
end
end
~~~

[asmagill]

Of course, the choice is yours, but I will also note that with this approach, I can disable the refocusing by simply adding hs.chooser._globalChooserCallback = function() end to my init.lua or chain it with something like this:

~~~lua
local originalChooserGlobal = hs.chooser._globalChooserCallback
hs.chooser._globalChooserCallback = function(chooser, state)
local alsoCallGlobal = true

-- my own internal logic to do whatever and determine if I also want to call the global callback
-- as I can change alsoCallGlobal here based upon my own logic and needs...

if alsoCallGlobal then originalChooserGlobal(chooser, state) end
end
~~~

[casouri]

So is there any guideline in hammerspoon about what should be done in Objective-C and what should be done in Lua?

For the callback, I'm thinking about a table rather than a function, it is easier to add and remove, kind of like how hooks work in Emacs.

[asmagill]

Officially no, not that I'm aware of, but unofficially my thoughts are that if something can be done timely in Lua, then it probably should -- if lua code fails, Hammerspoon will display information in the console, but not crash -- and maybe the average user will be able to troubleshoot it and provide a fix or at least details about what they were doing so we can fix it. Until recently, any Objective-C problems caused Hammerspoon to crash, and while @cmsj did add code to dump the stack trace into the console, this only catches exceptions -- other errors that occur in Objective-C code can (and do) still crash Hammerspoon. Often the crash occurs so quickly that the user isn't 100% positive exactly which command caused the crash, and crash logs aren't always easy to parse, even when you are familiar with the underlying Objective-C code, which most users won't be.

Obviously if speed is a factor, then Objective-C is your friend, but in this case, the speed difference is going to be close enough that the user likely won't ever notice it.

[cmsj]

As much as I prefer ObjC, @asmagill is right in this case, the default callbacks should be in Lua.

@casouri I don't know why, but I'd never really considered having tables for callbacks. It's a nice idea, but I think at this point it would be very inconsistent for Hammerspoon to behave like that, so I think we'll go with @asmagill 's plan. Maybe in a post-1.0 world we can reconsider architecture decisions like that.

[cmsj]

@asmagill would you be able to have a quick look over https://github.com/Hammerspoon/hammerspoon/pull/1750 ? I had to do a bit of refactoring to make this work and I'm not sure if I got all the userdata object lifecycle stuff correct (there's a FIXME question in the diff about part of that).

[latenitefilms]

@cmsj - Whilst you're in hs.chooser land - any chance you could give #1624 a bash too? I'm ASSUMING this won't be too hard to implement? I tried to give it a go, but I'd need some more guidance to push me in the right direction of how to get hs.styledtext and hs.chooser to play nicely together. No stress if you're flat out though - seriously appreciate all the bug-hunting you've been doing!!

[malob]

For those that might find this issue due to experiencing the issue while using Seal. This is my quick hacky fix:

sealVisible = false
windowBeforeSeal = nil

function toggleSeal()
  if sealVisible then
    spoon.Seal:toggle()
    sealVisible = false
    windowBeforeSeal:focus()
    windowBeforeSeal = nil
  else
    windowBeforeSeal = hs.window.focusedWindow()
    spoon.Seal:toggle()
    sealVisible = true
  end
end

hs.hotkey.bind("cmd", "space", toggleSeal)

[casouri]

Cool!

[cmsj]

FYI, this issue is closed now because I landed code to add a global callback, the default implementation of which, tracks window focus :)

[cmsj]

(Now we just need to do a release!)