Skip to content

config: workspace empty behavior#816

Open
kotarac wants to merge 1 commit into
mahkoh:masterfrom
kotarac:kotarac/workspace-empty-behavior
Open

config: workspace empty behavior#816
kotarac wants to merge 1 commit into
mahkoh:masterfrom
kotarac:kotarac/workspace-empty-behavior

Conversation

@kotarac
Copy link
Copy Markdown
Contributor

@kotarac kotarac commented Mar 19, 2026

Implementation according to our discussion here #541.
Closes #519.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 5 times, most recently from 67611a2 to 1e3dc69 Compare March 24, 2026 00:20
@mahkoh
Copy link
Copy Markdown
Owner

mahkoh commented Mar 28, 2026

In case you wrote the book changes by hand: You don't have to do that. There is already enough busy work when adding new features.

@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented Mar 28, 2026

In case you wrote the book changes by hand: You don't have to do that. There are already enough busy work when adding new features.

I did not actually. I've had gpt-5.2 write the book content and before that help me better clarify the behavior of each value in spec.yaml and review the changes. Or to be precise, review the changes and then do all that.

Btw, possibly interesting piece of info: I've tried letting it help write a testcase in the integration test and I ended up with a couple GiB worth of logs in testruns/ dir because it destroyed a wl_surface before destroying a wl_viewport, resulting in leak tracking going crazy with logging.

Still need to get around to testing this on multiple outputs. I only have such setup at the office and unfortunately I haven't had any time lately to sit down and go through it.

@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented Mar 28, 2026

In case you wrote the book changes by hand: You don't have to do that. There is already enough busy work when adding new features.

Also, fwiw, while this is true and the book and control center added more, I don't really mind this at all.

In my 1st Jay contribution on the touchpad stuff, it took quite some time to go through everything, but the 1st moment I've had it compile it actually worked correctly (despite having multiple style/consistency issues) which is at least partly because of the way it's set up right now.

I suppose some of it could be distilled now to somehow have 1 central place which dictates how all "touchpoints" work, but it's not exactly a big deal once you know where everything is.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 6 times, most recently from 8c2f2f8 to e370b81 Compare April 2, 2026 22:08
@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 3 times, most recently from 7043ed0 to db0e415 Compare April 11, 2026 11:14
@mahkoh
Copy link
Copy Markdown
Owner

mahkoh commented Apr 11, 2026

If you only have one display you can manually test this using virtual outputs and wl-mirror or obs.

@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented Apr 11, 2026

If you only have one display you can manually test this using virtual outputs and wl-mirror or obs.

Got one now actually, just need to get around to it. Rebasing it to stay up to date as I'm dogfooding this. 🙂

@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented Apr 15, 2026

Needs a bit more work, couple of edge cases to go through and some more ext_workspace_manager_v1 work which has 0 test coverage and I don't usually use it.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 10 times, most recently from 35a7d00 to ef352af Compare May 3, 2026 20:58
@mahkoh
Copy link
Copy Markdown
Owner

mahkoh commented May 4, 2026

Feature detection should only be used if not doing so would regress existing functionality.

@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented May 4, 2026

Otherwise, tested the non-protocol paths quite a bit myself and have ran this for longer than the PR has been up, although almost exclusively set to destroy while not testing the work itself.

For the protocol changes, I tested a bit with waybar (I don't usually use it), but not nearly as extensively as with the above.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 2 times, most recently from aae8b65 to ca8fba2 Compare May 5, 2026 15:59
@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented May 5, 2026

image

-.-

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch from ca8fba2 to 3c8a4ef Compare May 5, 2026 16:30
@mahkoh mahkoh self-requested a review May 5, 2026 17:09
Copy link
Copy Markdown
Owner

@mahkoh mahkoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like hidden workspaces should stay attached to their output. That would also remove the need to find the correct output when unhiding the workspace and would keep the workspace in the same position as before.

Sometimes it might be useful to have a linked list containing only the visible workspaces. For such cases it might be useful to maintain two linked lists per output. One containing all workspaces and one containing only the visible workspaces.

Comment thread jay-config/src/lib.rs Outdated
Comment thread jay-config/src/workspace.rs
Comment thread src/config/handler.rs Outdated
Comment thread src/config/handler.rs Outdated
Comment thread src/config/handler.rs Outdated
Comment thread src/state.rs Outdated
Comment thread src/state.rs Outdated
Comment thread src/state.rs Outdated
Comment thread src/state.rs Outdated
Comment thread src/state.rs Outdated
@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented May 5, 2026

I feel like hidden workspaces should stay attached to their output. That would also remove the need to find the correct output when unhiding the workspace and would keep the workspace in the same position as before.

Hmm, that would help. And yes, the current approach didn't place it back where it was in case the display order was manual.

Sometimes it might be useful to have a linked list containing only the visible workspaces. For such cases it might be useful to maintain two linked lists per output. One containing all workspaces and one containing only the visible workspaces.

👍

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 2 times, most recently from bbb0d65 to ed761f7 Compare May 5, 2026 20:45
@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented May 5, 2026

Think the separate linked list only for visible workspaces is needed? Seems cheap to just filter out hidden ones where needed, but it is inconvenient that it's done in multiple places though...

Comment thread src/tree/output.rs Outdated
@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 6 times, most recently from 11885e9 to 1aa5dd4 Compare May 10, 2026 09:20
Comment thread src/state.rs Outdated
@kotarac
Copy link
Copy Markdown
Contributor Author

kotarac commented May 10, 2026

FTR since the last review, I discovered a small issue when destroy is set with floating windows, that's another extra change since.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 3 times, most recently from 54f85c1 to a17f1ab Compare May 10, 2026 13:16
Comment thread src/state.rs
mut output: Option<Rc<OutputNode>>,
) {
let mut output = || {
let mut resolve_output = || {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might make sense to extract to a helper fn.

@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch 2 times, most recently from f3f7fa2 to 612a896 Compare May 11, 2026 06:13
@kotarac kotarac force-pushed the kotarac/workspace-empty-behavior branch from 612a896 to 3e34c6b Compare May 11, 2026 06:29
Comment thread src/tree/toplevel.rs
Comment on lines +1089 to +1101
let Some(ws) = data.workspace.get() else {
return;
};
state.map_tiled_on(tl.clone(), &ws);
parent.cnode_remove_child2(&*tl, true);
state.map_tiled(tl);
} else if let Some(ws) = data.workspace.get() {
parent.cnode_remove_child2(&*tl, true);
let (width, height) = data.float_size(&ws);
state.map_floating(tl, width, height, &ws, None);
return;
}
let Some(ws) = data.workspace.get() else {
return;
};
let (width, height) = data.float_size(&ws);
state.map_floating(tl.clone(), width, height, &ws, None);
parent.cnode_remove_child2(&*tl, true);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#816 (comment)

Single window remaining on a workspace with empty behavior set to destroy being toggled between tiled/floating.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to change the behavior of toggling a window from floating to tiled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will take a look ~wednesday. Right now, looking at this on the phone and answering by memory. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workspace bar enhancements

2 participants