FelixKratz / SketchyBar

A highly customizable macOS status bar replacement
https://felixkratz.github.io/SketchyBar/
GNU General Public License v3.0
5.25k stars 82 forks source link

Duplicate keys in space_windows_change INFO #498

Closed doebi closed 5 months ago

doebi commented 5 months ago

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: https://github.com/FelixKratz/SketchyBar/blob/7d121d55e2e694c2cd9655dbdfdd70f9954e626c/src/app_windows.c#L135

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 commented 5 months ago

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.

doebi commented 5 months ago

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

FelixKratz commented 5 months ago

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.