Feature query windows via hammerspoon#18
Conversation
…et fdm=marker' in vim
…ces bin/yabai-get-stacks. The goal of this file is to eliminate the need to 'shell out' to yabai to query window data needed to render stackline, which would address #8. The main problem with relying on yabai is that a 0.03s sleep is required in the yabai script to ensure that the changes that triggered hammerspoon's window event subscriber are, in fact, represented in the query response from yabai. There are probably secondary downsides, such as overall performance, and specifically *yabai* performance (I've noticed that changing focus is slower when lots of yabai queries are happening simultaneously). ┌────────┐ │ Status │ └────────┘ We're not yet using any of the code in this file to actually render the indiators or query ata — all of that is still achieved via the "old" methods. However, this file IS being required by ./core.lua and runs one every window focus event, and the resulting "stack" data is printed to the hammerspoon console. The stack data structure differs from that used in ./stack.lua enough that it won't work as a drop-in replacement. I think that's fine (and it wouldn't be worth attempting to make this a non-breaking change, esp. since zero people rely on it as of 2020-08-02. ┌──────┐ │ Next │ └──────┘ - [ ] Integrate appropriate functionality in this file into the Stack module - [ ] Update key Stack module functions to have basic compatiblity with the new data structure - [ ] Simplify / refine Stack functions to leverage the benefits of having access to the hs.window module for each tracked window - [ ] Integrate appropriate functionality in this file into the Core module - [ ] … see if there's anything left and decide where it should live ┌───────────┐ │ WIP NOTES │ └───────────┘ Much of the functionality in this file should either be integrated into stack.lua or core.lua — I don't think a new file is needed. Rather than calling out to the script ../bin/yabai-get-stacks, we're using hammerspoon's mature (if complicated) hs.window.filter and hs.window modules to achieve the same goal natively within hammerspon. There might be other benefits in addition to fixing the problems that inspired tracked by stackline, which will probably make it easier to implement enhancements that we haven't even considered yet. This approach should also be easier to maintain, *and* we get to drop the jq dependency!
…n be called in place of Window:process() to redraw a window.
…cators display on left/right edge of window based on which side of the screen the window resides. Known bug: changes broke multiple stacks on the same space.
… all stack-is-occluded functionality from Query to StackMgr and Stack modules.
…ndicators update when switching between windows of the same app.
…troying them & rendering from scratch.
stackline/query.lua
Outdated
| @@ -0,0 +1,206 @@ | |||
| -- luacheck: ignore (spaces isn't used, but augments hs.window module) | |||
| local spaces = require("hs._asm.undocumented.spaces") | |||
There was a problem hiding this comment.
This doesn't seem to be available on my machine:
2020-08-10 10:27:53: *** ERROR: ...app/Contents/Resources/extensions/hs/_coresetup/init.lua:597: module 'hs._asm.undocumented.spaces' not found:
no field package.preload['hs._asm.undocumented.spaces']
no file '/Users/alin/.hammerspoon/hs/_asm/undocumented/spaces.lua'
no file '/Users/alin/.hammerspoon/hs/_asm/undocumented/spaces/init.lua'
no file '/Users/alin/.hammerspoon/Spoons/hs/_asm/undocumented/spaces.spoon/init.lua'
no file '/usr/local/share/lua/5.3/hs/_asm/undocumented/spaces.lua'
no file '/usr/local/share/lua/5.3/hs/_asm/undocumented/spaces/init.lua'
no file '/usr/local/lib/lua/5.3/hs/_asm/undocumented/spaces.lua'
no file '/usr/local/lib/lua/5.3/hs/_asm/undocumented/spaces/init.lua'
no file './hs/_asm/undocumented/spaces.lua'
no file './hs/_asm/undocumented/spaces/init.lua'
no file '/Applications/Hammerspoon.app/Contents/Resources/extensions/hs/_asm/undocumented/spaces.lua'
no file '/Applications/Hammerspoon.app/Contents/Resources/extensions/hs/_asm/undocumented/spaces/init.lua'
no file '/Users/alin/.hammerspoon/hs/_asm/undocumented/spaces.so'
no file '/usr/local/lib/lua/5.3/hs/_asm/undocumented/spaces.so'
no file '/usr/local/lib/lua/5.3/loadall.so'
no file './hs/_asm/undocumented/spaces.so'
no file '/Applications/Hammerspoon.app/Contents/Resources/extensions/hs/_asm/undocumented/spaces.so'
no file '/Users/alin/.hammerspoon/hs.so'
no file '/usr/local/lib/lua/5.3/hs.so'
no file '/usr/local/lib/lua/5.3/loadall.so'
no file './hs.so'
no file '/Applications/Hammerspoon.app/Contents/Resources/extensions/hs.so'
stack traceback:
[C]: in function 'rawrequire'
...app/Contents/Resources/extensions/hs/_coresetup/init.lua:597: in function 'require'
/Users/alin/.hammerspoon/stackline/stackline/query.lua:2: in main chunk
[C]: in function 'rawrequire'
...app/Contents/Resources/extensions/hs/_coresetup/init.lua:597: in function 'require'
/Users/alin/.hammerspoon/stackline/stackline/stackMgr.lua:5: in main chunk
[C]: in function 'rawrequire'
...app/Contents/Resources/extensions/hs/_coresetup/init.lua:597: in function 'require'
/Users/alin/.hammerspoon/stackline/stackline/core.lua:4: in main chunk
[C]: in function 'rawrequire'
...app/Contents/Resources/extensions/hs/_coresetup/init.lua:597: in function 'require'
/Users/alin/.hammerspoon/init.lua:13: in main chunk
[C]: in function 'xpcall'
...app/Contents/Resources/extensions/hs/_coresetup/init.lua:648: in function 'hs._coresetup.setup'
(...tail calls...)I commented it and the code started running, but I can't see any indicators and on one space I got this:
bin/yabai-get-stack-idx
Outdated
| @@ -0,0 +1,15 @@ | |||
| #!/usr/local/bin/dash | |||
There was a problem hiding this comment.
Maybe change this to /bin/dash?
| #!/usr/local/bin/dash | |
| #!/bin/dash |
|
@AdamWagner the code looks cleaner now, I like this approach! But I can't seem to get it working yet, I've left some comments about the issues I have. Keep in mind that on multi-monitor setups, coordinates can also be negative for the window frame. I'm thinking this might be why I don't see the indicators at all on my external monitor but sometimes they appear on the built-in monitor. While this approach is being reviewed, could we merge these two small improvements? #14 #17 |
I'm so sorry for missing both of these! I will do a quick review and get these merged tonight. I'm especially impressed with the upstream change to Hammerspoon. We may want to take a similar route with Hammerspoon/hammerspoon#2400, which is pretty problematic for stackline, and resulted in a lot of "workaround logic". |
|
Re: dual monitors - I have an extra monitor that I can use to test that case. Thanks for calling it out. Re: the "full screen icon" bug - I have encountered that on even a single monitor after switching spaces. It appears intermittent, which led me to letting it slide in this draft. I'll prioritize it for the next fix, tho. |
|
@alin23 Apologies for missing another dependency. Can you install https://github.com/asmagill/hs._asm.undocumented.spaces and try this again? Here's a teaser for motivation. Snappy!
|
Oh wow, that looks awesome! I can't seem to get I see it uses the |
|
Shoot. I'm definitely using an older version of I think I'll need to rework this so that it does not depend on the undocumented spaces module. It should be possible, given that |
I see, I thought that could be the difference so I tried older versions but I gave up after 2 versions that didn't work 😅 Probably should have just kept trying 2-3 more. But yes, if we could work without it, it would be much easier for users to install it. I think we should strive for a 2-step installation process: git clone and echo require inside init.lua |
I support this. |
…urrentSpace which depends on it.'
…ack, and centralize indicator config settings & retrieval.
Delete unused utils, consolidate utils into single file.
|
@alin23 – I'm hopeful that you'll be able to get this branch running more easily now. Do you think you'll have time to test it out this week? |
|
@AdamWagner I finally got it working and it was very snappy for moving through the stack. 😄 I had to revert because I'm in a very busy period at my daily job and didn't have the time to debug it. I can try this again when things slow down at my job. |
I totally understand this. I assumed you have been busy with work, so thanks for trying it out again. Re: slow yabai actions… I'm surprised that the changes in this branch would have any (further) negative effect on yabai actions. Just curious – did you comment out the signals from Some general observations:
All of this said, I believe you and will take a closer look at stack resizing / creation speed. I'll try to quantify it so we can compare. |
|
Here's a quick recording just to give you a sense of what I'm seeing so we can calibrate: The clip starts by resizing a space with 5 total windows (2 2x stacks, 1 solo window) without stackline running.
|
|
It's possible that when I tested this the first time, I had a GPU terminal window that took more time to redraw its contents on resize and I assumed it was caused by this branch. When testing it now, the resize looks the same in both setups, only the stack creation is slower so I perceive it to be slower overall. This shouldn't be a problem as it is not an action that will happen often. Yabai Signals
Hammerspoon Window Filter
I had to add a small check because some windows didn't have the stackIdx property and I was getting nil errors: alin23@dcb7a29 Overall, it looks good to go! There's a lot of work put into this so I'd love to see this merged and build on top of it. Some questions I have before the merge:
|
Yes, definitely. I forgot that I added the binding into stackline and not my own
I don't know how hard it will be. A lot of the indicator draw/redraw/color attribute building has changed, but it should be fairly straightforward to plug into / customize.
Yes, I've had the same thought. I'll create an issue to address this after merging the changes here. |
… nil value (field 'stackIdx') by checking for stackIdx > 0
…ine to work well with leasot / rg
…r instances from main stackline.lua Remove keybinding from config.lua & move to readme.
…t go off screen
2 participants
I just created this to have a place to note/discuss questionable changes. I expect this will remain open for at least another couple of weeks.