-
-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use hashtable for storage #7
Conversation
This reverts commit 03c58ea.
The hashtable is just copied out of yabai code. The rest is kinda janky again, sorry for low quality code -- most changes are kinda rushed and I have not spent too much time looking at how all the code is organized. Again, feel free to readjust w/e before merging. You probably want to do another code formatting pass to get style that fits with your code. |
I think the PR has one major problem. When moving a window to a different space, there is a brief moment when two borders with the same wid exist, because the window is created at the target space first and then destroyed at the origin space. Thats why I used wid and sid checks to find the window border. I am not sure if sticky windows will fall into the same problem. So my suggestion would be to use a hash that is combined in some clever way from wid and sid to identify the window. Or the event logic needs to be adapted to capture this eventuality. |
I believe the latest commit should adress that problem just fine. Edit: I did not do any testing with sticky windows, that might be broken. Edit2: I might have overlooked some technical differences in |
Don't merge this. I definitely missed something in your logic and screwed something important. Upon moving a window to a different space (and following focus), moving back to the previous space, now all other windows lack their border. |
It could also be I destroyed something during merging… |
Ok so I am definitely breaking your logic with the changes in this PR, but it seems to work the way we want it to. I changed the response to EVENT_UNHIDE and EVENT_HIDE to look for existing border and call Probably smart to test-drive for a while. There is likely more event logic that need adjustments. |
Ok so I have cleaned up and refactored this a bit, what do you think? I am happy with it and will merge once you approve my changes. |
Looks good to me. |
Uses hashtable for storage instead of having separate tracked arrays for windows and borders.
Also fixes bug mentioned in #6