Conversation
d31d43c to
68075c0
Compare
|
Now you may or may not agree with the necessity of the additional workspaces, but the not using hardcoded values in src/fetch/monitor.h is certainly an improvement. |
dc016eb to
6b2d694
Compare
|
It's not about agreeing on the necessity, it's about complying with Also you changed formatting, which is no good |
|
The whitespace formatting can certainly be fixed, no big deal. But let's talk about DWL IPC. While it may be true that it might break DWL IPC for some specific apps, that's actually an improvement. If some downstream code is relying on Mango to replace any workspace greater than 9 with 9, then it's coded against a bug and should break. If something breaks due to this change, then it's worth breaking. Consider the fact that Mango is not DWL, it's just based on DWL. DWL is actually unmaintained at the moment; MangoWC is further ahead of DWL by quite a bit. In fact, DWL doesn't actually have IPC; there's a well known patch that adds IPC, but in its purest form, DWL does not have IPC. Now, looking at Mango's IPC, which admittedly is identical in function to the well known DWL IPC patch, every single representation of a tag or tag mask is a Keep in mind that DWM is the inspiration behind DWL, and DWM definitely supports more than 9 tags. Here's a direct quote from /* compile-time check if all tags fit into an unsigned int bit array. */
struct NumTags { char limitexceeded[LENGTH(tags) > 31 ? -1 : 1]; };Obviously the whitespace formatting needs to be fixed, but that's not a big deal. Aside from that, this change looks valid to me. This isn't my project, though; I'm just a former DWM user who is switching to Wayland and doesn't want to start from DWL. From a UX perspective, there are 10 digits on a number pad/row. I think this makes sense. This PR is a net improvement, IMO. |
|
What I forgot to mention: initially I added |
|
Hmm, if this will not break DWL IPC and other apps, then why only go for 10? Some compositors like river support up to 32 tags, so tag length should be 9 by default and should be able to be set in the config |
|
To be honest, since we now support ext-workspace, it has no restrictions at all. I originally intended to completely abandon dwl ipc and then use an independent ipc socket. This way, many functional requests can also be resolved, such as obtaining all client states through external tool requests, etc For a specific workspace 10, I tend to implement a named workspace that is not limited by quantity |
|
+1 for named workspaces. |
|
I fixed the whitespace discrepancy. |
|
Hey @DreamMaoMao , I took a look into the Mango code a bit more and I don't think the This PR actually demonstrates the issue. Notice how for (i = 1; i <= LENGTH(tags); i++) {
add_workspace_by_tag(i, m);
}To truly generalize Mango's implementation of the For an immediate improvement, would you consider this PR, or even this PR expanded to 32 tags? This is fully compatible with existing IPC and the existing Workspaces implementation (direct map to tags). There is a non-zero amount of people who would benefit from this, plus it's a very small, low-risk change that doesn't even introduce any new code paths. |
|
I've drafted an alternative implementation in #739 that makes the number of tags configurable, clamped to |
673ec40 to
db30977
Compare
|
I'm all in favour of #739. It's a clean, well-thought-out PR, which fixes the multiple issues, and doesn't conflict with further improvements. Job well done, @gplusplus314 |
I really need workspace 10 to make mangowc work for me.