Configuration option for minimum desktop count #22
Conversation
|
Thanks for this MR, and thanks for leaving good comments in the code |
contents/code/main.js
Outdated
| // Calculate the index of the last desktop we want to preserve when cleaning up. | ||
| // We need to preserve: | ||
| // 1. The current desktop (currentDesktopIndex) | ||
| // 2. At least MIN_DESKTOPS. Note we actually subtract two because: | ||
| // - We always have the dynamic empty desktop at the end | ||
| // - MIN_DESKTOPS is a count, but we need an index | ||
| const preserveUpToIndex = Math.max(currentDesktopIndex, MIN_DESKTOPS - 2); |
There was a problem hiding this comment.
I thought the purpose of #21 (and one another I can't find) was that we want to remove the leading desktop as well, for using this script with the Overview effect (the super-w thing): there you can drag a window to the plus sign and it will create a desktop
There was a problem hiding this comment.
Ah wait, I just tested and we need to disable creating the empty desktop altogether for that to work as you would expect. This constraint makes sense then
There was a problem hiding this comment.
Maybe I misunderstood the #21, but I linked it because I thought it's exactly the same use case that I needed, which is the capability to always preserve some desktop count.
From what you describe, I think you mean #17? I think it's outside the scope of this PR, though If it's added in some other merge request, then indeed It should be considered how would it integrate with the minDesktops configuration option.
contents/code/main.js
Outdated
| const MIN_DESKTOPS = 2; | ||
| const MIN_DESKTOPS = readConfig("minDesktops", 2); | ||
| const KEEP_EMPTY_MIDDLE_DESKTOPS = readConfig("keepEmptyMiddleDesktops", false); |
There was a problem hiding this comment.
This way the values are read only once at script startup, and not reloaded on config change, right? I don't think this is what we want
There was a problem hiding this comment.
Yes, you are right! I've actually realized that myself soon after publishing the PR, but had no time since to fix it.
There was a problem hiding this comment.
Should be resolved in 562c7c9
However, it seems that configuration options are not updated in KWin instantly anyway. New configuration is applied only after running qdbus org.kde.KWin /KWin reconfigure or I guess also after reboot/log-in
2 participants
Resolves #21
This PR adds a new configuration option
minDesktopsthat allows users to specify the minimum number of virtual desktops to preserve. Previously, the script would always clean up all empty desktops (except the first two).The default is set to 2, which preserves the original behaviour — one regular desktop plus one empty dynamic desktop at the end. Users can now increase this value through the configuration UI if they prefer to always have more desktops available.
Technical Implementation
minDesktopsconfiguration option inmain.xmlconfig.uifor adjusting this settingmain.jsto load and respect this new option