x11: include wm_instance

[powellnorma]

Can be useful

[2e3s]

Hello, thanks for the suggestions! I think it would be better to have 9daebc8 as a separate PR as it's an unrelated functionality and seemingly even more of a bug fix.

What are the examples when wm_instance can be useful? app_id and data are both standard keys for ActivityWatch so they are used for stats and charts, and class in wm_class seems the most appropriate for the role as it's set by the app's author.

[powellnorma]

What are the examples when wm_instance can be useful? app_id and data are both standard keys for ActivityWatch so they are used for stats and charts, and class in wm_class seems the most appropriate for the role as it's set by the app's author.

It can be useful as the user can often set it itself, and it can be used to e.g. distinguish different browser instances (using different data-dir etc)
Also, those other attributes don't go away! :)

I think it would be better to have 9daebc8 as a separate PR as it's an unrelated functionality and seemingly even more of a bug fix.

Ok I opened another PR #30 - it is based upon this one, though. So this one should probably get merged first

[2e3s]

Ok I opened another PR #30 - it is based upon this one, though

Thank you, but I think it's vice versa, the proposed functionality (not sure about it yet) needs to be based on the bug fix which more certainly should be merged.

[powellnorma]

Thank you, but I think it's vice versa, the proposed functionality (not sure about it yet)

Why are you not sure about this yet? Are there any downsides that I am missing?

[2e3s]

One is that it creeps into the interface, I think it's better to make an optional map of extra data to send_active_window instead of an extra function send_active_window_with_instance for X11.

[powellnorma]

optional map of extra data

You mean like a extra_data: Option<HashMap<String, String>> ?
Would it make sense to have this for a new method like send_active_window_with_extra, or should we adjust existing client (watcher using send_active_window) code?

[2e3s]

You mean like a extra_data: Option<HashMap<String, String>> ?

Yes.

Would it make sense to have this for a new method send_active_window_with_extra

Yes, with send_active_window calling it, it seems the easiest way for the moment. Basically, same as you suggested, but not coupled to X11.

[2e3s]

I also thought about a configurable choice between wm_instance and wm_class for title instead of the extra values for sending, e.g. with replacements in filters configuration. Then it can be utilized in UI. But it's a separate improvement.

[powellnorma]

I adjusted the code

[2e3s]

Thanks, one small thing left to do, please run cargo fmt and push the change

[powellnorma]

Done! :)