agzam / spacehammer

Hammerspoon config inspired by Spacemacs
MIT License
556 stars 70 forks source link

bugfix: window jumps #188

Closed jaawerth closed 9 months ago

jaawerth commented 9 months ago

Alternative to #150 - I noticed jumps weren't working, and went and fixed it before noticing the open PR! This continues using the filter, so will only jump to windows on the current space.

jump-window was trying to access a nonexistent :filter property of windows instead of hs.window.filter. Corrected to instead call (: hs.window.filter :focusWindow<Dir> <frontmost-window> true true).

One minor change from the original is the use of (hs.window.frontmostWindow) as the reference window in place of (hs.window.focusedWindow); this will normally use the focused window, but if no window has focus, falls back to the one on "top."

Note: Another option would be to use (hs.window.filter:focus<Dir>), which calls hs.window.focusWindowDir(nil, nil, true), nearly identical to what's here except it will include windows covered (or partially covered) by others in the current space/filter when picking which window to focus.

I erred on the side of sticking with the originally-intended behavior, but I did play around with it the other way, and I wasn't entirely sure which I liked better. Still, thought I'd mention it for consideration. Might be nice if that's configurable, but I may be over-complicating things.

EDIT: I tried it with the hs.window.filter.focus<Dir> approach and, yeah, when you have a number of layered windows, it's way too annoying 😆

Grazfather commented 9 months ago

This look OK to me, but will defer to @agzam since he experiences the issue.

jaawerth commented 9 months ago

Makes sense! Actually, when I first read through the other PR I thought the issue at hand was just the behavior resulting from using the window focus methods, which have a note in the docs warning about how it would pick from too wide a range of windows - but I see in the comments that someone linked to a change that's functionally identical to mine, so apologies for that - you may want to just close this rather than re-test something that may have already been tested..

That said, it sounds like the extant issue for both my patch and the commented one is related to the hammer menu hanging around after the jump. For what it's worth, I'd suggest merging either this or a similar patch so the feature is at least not totally broken; the menu thing seems like a slightly different scope, so it'd be a step in the right direction.

Personally, I find I don't necessarily mind the menu staying there for some debounced period so you can keep jumping - the issue at hand being it doesn't always stay on the "window" menu, but sometimes goes back to the top menu... I'll play around with the menu thing too in the meantime - I love spacehammer (it's pretty essential to making my work macbook comfortable to use on a daily basis at this point!), definitely enough to spend some helping work out the kinks!

Grazfather commented 9 months ago

@agzam would you be able to test this out?