Auto-remove desktops that contain only pinned (on-all-desktops) windows - #48
Open
mpollock wants to merge 1 commit into
Open
Auto-remove desktops that contain only pinned (on-all-desktops) windows#48mpollock wants to merge 1 commit into
mpollock wants to merge 1 commit into
Conversation
removeEmptyVirtualDesktops() bailed out entirely (`return`) the moment it saw any window with onAllDesktops set. With one or more "pin to all desktops" windows present, that disabled the feature completely: every desktop looked occupied. Skip on-all-desktops windows instead (`continue`). A window that is on every desktop cannot be what makes a particular desktop non-empty, so a desktop whose only remaining windows are pinned is now correctly removed. Also: - add a Workspace.onWindowRemoved backstop so the sweep runs on every window removal, not only windows registered through addWindow(). - fix the trailing-index off-by-one in the removal loop (started at length, so counts[length] was always undefined on the first pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DjxQ7ZqJwqb3ohScJYUwC
Author
|
Unsure if this is desired behavior for others, but I find it helpful. Perhaps it can be a part of settings? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With Auto-remove empty virtual desktops enabled,
removeEmptyVirtualDesktops()returns early the moment it encounters any window withonAllDesktopsset:If you keep even one "pin to all desktops" window around (common with multi-monitor setups, media players, chat, per-monitor-virtual-desktop scripts, etc.), the feature is effectively disabled — every desktop looks occupied, so nothing is ever removed.
Repro
Change
Skip on-all-desktops windows (
continue) instead of aborting the whole sweep. A window that is on every desktop cannot be the thing that makes a particular desktop non-empty, so a desktop whose only remaining windows are pinned is now correctly removed. The first desktop is still never removed.No change for users who don't use pinned windows, and none when the setting is off.
Also included (happy to split out if you'd rather)
Workspace.onWindowRemovedbackstop — the per-windowclosedhandler wired inaddWindow()only covers windows that passedisValidWindow()at add time; this workspace-level signal fires for every removal.removeEmptyVirtualDesktops()already short-circuits on the config flag, so it's a cheap no-op when disabled.virtualDesktopWindowCount.length, sovirtualDesktopWindowCount[length]wasundefinedon the first iteration (harmless, but=== 0was never true there anyway).Testing
Bazzite / KDE Plasma 6.7.4, Wayland. With the patch: empty desktops are removed as soon as their last non-pinned window is closed or moved away, whether or not pinned windows are present; desktop 1 is preserved; no change observed with the setting disabled.