python
python copied to clipboard
Protect window_switcher from split errors
This commit stops window_switcher from erroring when the wm_class has
more or less than a single .
character. For example, here are two
problematic lines of outputs from wmctrl -l -x
where the wm_class
is
N/A
or rufus-3.17.exe.rufus.3.17.exe
(from Wine):
0x01a00078 1 N/A remus Ozone X11
0x03e00003 2 rufus-3.17.exe.rufus-3.17.exe remus Rufus 3.17.1846
If there are no dots, the entry is skipped and not matched. If there is more than one dot, the part before the first dot is used for the display name (e.g. "rufus-3").
Hi, I do have a concern about this fix. It sures prevents the crash but wine applications are badly displayed :
it comes from the spit, original string is for instance program.exe.program.exe
==> win_class_parts[1] in non-wine cases should be replaced by win_class_parts[2] depending on win_class_parts length. (which I don't know how to write in a nice one-liner)
maybe this one
win_class = len(win_class_parts) > 2 and win_class_parts[2] or win_class_parts[1]
is better
I've revised the approach to skip when there are zero dots and take only the first segment when there are more than one.
In my Wine testing, I happened to find rufus-3.17.exe.rufus-3.17.exe
. I don't think there's any good way to be clever short of trying to pattern match .exe
, and even that is probably brittle. Using the first segment rufus-3
is probably a reasonable heuristic that shouldn't be too confusing.
@edjoperez @dshoreman can you please review?
Ping. This has been open for a couple months. I'm sure people are busy, but would it possible to get a review soon, please? It's only ~20 lines and half of it is a comment explaining the behavior.
LGTM
The 0.18 release will probably need changes. Since there will be no release in between I'll wait with the merge. Ty for your time. Ill comment when it is released
0.18 is out. Please check the new api . Do you mind to volunteer as a maintainer for the plugin?
I'm not really a Python person, nor do I have the time to maintain it even if I did. I was really just hoping to get this tiny change applied.
@edjoperez @dshoreman can you please review?
It's been over a year, but if my two cents are still desired I would probably move most of the new code into the parseWindow
method, potentially modifying the named tuple to have an extra key or two if needed. That would allow the loop itself to stay (relatively) clean/simple and keeping all the parsing bits in one place.
I won't be able to test though, unfortunately. While I did contribute several fixes to this plugin via a partial rewrite in ed2778c9f1f687e3c6f46d01ee837b698e791102, that was back when I was under the impression the license was GPL and I have since moved to an alternative launcher.
Sorry I haven't had the time to respond sooner.
please continue
I'm not really a Python person, nor do I have the time to maintain it even if I did. I was really just hoping to get this tiny change applied.
The problem today is that this code is incompatible with recent versions of albert. Ill close this PR since this plugin has been not been ported yet.
Id appreciate if anybody actually using it could port it.
Regards
Please continue any discussion here: https://github.com/orgs/albertlauncher/discussions/1299