Skip to content
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

Duplicate keys in space_windows_change INFO #498

Closed
doebi opened this issue Feb 1, 2024 · 3 comments
Closed

Duplicate keys in space_windows_change INFO #498

doebi opened this issue Feb 1, 2024 · 3 comments
Labels
available on master bug Something isn't working

Comments

@doebi
Copy link

doebi commented Feb 1, 2024

Hi Felix!
Thanks for this project. For me coming from more than 10 years of Linux with i3 this is a true lifesaver in getting some of the beloved features to my new mac.

While customizing sketchybar to my needs I discovered that the event "space_windows_change" produces somewhat invalid JSON for me.

These are printouts of the $INFO payload received from the event:

{ "space": 2, "apps": { } }
{ "space": 1, "apps": { "Brave Browser": 1, "Alacritty": 1, "Alacritty": 1, "Alacritty": 1 } }
{ "space": 4, "apps": { } }
{ "space": 3, "apps": { } }

It seems that it does not detect Alacritty as being the same application but detects them as individual applications. Which in itself is not actually a bad thing, my dock is behaving in a similar manner resulting in multiple icons of the same application.

Screenshot 2024-02-01 at 13 35 27

The problem however is when I parse this string with jqit will rightfully remove all duplicate keys from the set and I end up with wrong information.

I had a look at your code to find the root of this JSON and traced it back to this part:

snprintf(payload, length, "{\n"

This kinda explains the behaviour, but leaves me quesitoning on how to solve it.
I have two thoughts:

A: fixing the count detection
to retain compatibility with existing setups fixing the count detection so duplicat keys are summed up would be one approach

B: changing the format
While looking at your code i asked myself if you wouldn't want to change the format in something more robust and potentially expandable with more metadata for future uses. For example having a window_id as key in the set or switching to an ordinary list of objects containing arbitrary data.

Let me know what you think. Cheers.

@FelixKratz
Copy link
Owner

FelixKratz commented Feb 1, 2024

I think thats an oversight I made while implementing this event. I think the fix should be option A you have mentioned to sum together apps with the same name but different pid because I don't want to do breaking changes if not badly neccessary. More info about windows is always available if needed when querying the window manager for info (e.g yabai).

Off topic: You can configure Alacritty to only use one process to handle all windows. This will be faster because you save the overhead of having separate processes.

@FelixKratz FelixKratz added the bug Something isn't working label Feb 1, 2024
@FelixKratz FelixKratz changed the title space_windows_change Duplicate keys in space_windows_change INFO Feb 1, 2024
@doebi
Copy link
Author

doebi commented Feb 8, 2024

Thanks, I updated my plugin and everything works as expected.
Working with JSON in bash is still kinda awkward.

@FelixKratz
Copy link
Owner

FelixKratz commented Feb 8, 2024

I think JSON is very easy to work with when using jq from bash. Additionally, the API exposed via the mach port of sketchybar can be interacted with by different means as well. One example would be https://github.com/FelixKratz/SbarLua where all of the communication between the lua module and sketchybar is done directly via the mach ports of both processes, cutting out the fork exec overhead entirely. Here all JSON is directly translated into lua tables by utilizing cJSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants