feat(xworkspaces): Add group-by-monitor flag - #2926
Conversation
|
@patrick96 friendly ping :) |
|
Hey, sorry, really don't have too much time right now. It's on my list though Thanks :) |
8c5e204 to
7337279
Compare
|
Okay, no worries, just making sure it didn't fall between the cracks somewhere :) (the force push rebased it on top of current |
|
I've been hit by precisely this bug and found the solution here. Thanks for the work so I didn't have to write this patch myself 👍 |
patrick96
left a comment
patrick96
left a comment
There was a problem hiding this comment.
Finally found some time (and motivation) to review this. This is the second-last thing on my list before I want to release 3.7.0!
Code looks good. Just some minor changes, I think the logic is reversed. Otherwise there is nothing else to do
By default, we group workspaces by monitor with the help of _NET_DESKTOP_VIEWPORT. However, some users may experience this as an unpredictable "shuffling" of workspaces. While WMs could disable advertising the property itself, it seems more sensible to handle this at the level of polybar. Hence, introduce a new group-by-monitor flag—defaulting to true—which can be used to disable this behaviour. Closes: polybar#2603 Related: xmonad/xmonad-contrib#791 Related: qtile/qtile#3375 Co-authored-by: scaramangado <scaramangado@gmail.com>
7337279 to
6f2a804
Compare
patrick96
left a comment
patrick96
left a comment
There was a problem hiding this comment.
Looks good now. Thanks for your help :)
Now to actually preparing the release
Codecov Report
@@ Coverage Diff @@
## master #2926 +/- ##
==========================================
- Coverage 12.69% 12.69% -0.01%
==========================================
Files 160 160
Lines 12531 12533 +2
==========================================
Hits 1591 1591
- Misses 10940 10942 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
3 participants
We had someone else get bit by this, so I figured why not pick up the (pretty much already done) patch by @scaramangado from #2603 and add some docs.
What type of PR is this? (check all applicable)
Description
By default, we group workspaces by monitor with the help of _NET_DESKTOP_VIEWPORT. However, some users may experience this as an unpredictable "shuffling" of workspaces. While WMs could disable advertising the property itself, it seems more sensible to handle this at the level of polybar. Hence, introduce a new group-by-monitor flag—defaulting to true—which can be used to disable this behaviour.
Related Issues & Documents
Closes: #2603
Related: xmonad/xmonad-contrib#791
Related: qtile/qtile#3375
Documentation (check all applicable)
This PR requires changes to the Wiki documentation (describe the changes)
I suppose documentation of the flag, as well as the incompatibility with
monitor-labelshould be put into the relevant xworkspaces page. Possibly with references to some of the above issues (although I don't know whether that'd help user find it).This PR requires changes to the documentation inside the git repo (please add them to the PR).
I added a changelog entry, not sure if there's anything else to be done.
Does not require documentation changes