roosta / i3wsr

Change i3-wm workspace names based on content
MIT License
178 stars 14 forks source link

WM_INSTANCE takes precedence over WM_CLASS #11

Closed luukvbaal closed 4 years ago

luukvbaal commented 4 years ago

Disclaimer: I never worked with rust before so I might have butchered the implementation but it does work. If I follow the code correctly, this PR will check if WM_INSTANCE is present in the config file and use that to match the icon. If no match is found it will still use WM_CLASS. But I'm not 100% sure of this as I am not sure I quite understand the Ok() return used here.

My goal here is to allow differentiating WM_INSTANCE from WM_CLASS and having it take precedence since WM_INSTANCE is usually more specific as far as I know. Where this comes up for me is when using browser web-apps for instance by running chromium --app="https://web.whatsapp.com". This PR allows the following behaviour: image with both icons coming from the same WM_CLASS but different WM_INSTANCE. Feel free to adapt to rust coding conventions as I have no idea what I'm doing.

roosta commented 4 years ago

Sorry about the delay on this, haven't had a chance to look this over properly. I do worry though that this change might cause issues with config. Thank you for your contribution.

roosta commented 4 years ago

I've had some time to look over the code, and this has the potential to break things, I'd hesitate to merge it as is. I need to consider including wm instance in a new release at some point but I don't have time currently. Sorry for wasting yours. I'll leave this pull request open for now, and I'll look back into it when I have the chance.

luukvbaal commented 4 years ago

Sure no problem. I haven't encountered any issues myself though so I'll keep using it for now.

roosta commented 4 years ago

I'm gonna close this pull request, I've published a version that borrows from this PR, and I've added you to the contributors list, since your PR was the inspiration for the latest version, just needed some tweaks.