From e4a5a03b990aae003c34593bbefba2cbdbf3f3fb Mon Sep 17 00:00:00 2001 From: LGUG2Z Date: Sat, 31 Oct 2026 12:03:14 -0700 Subject: [PATCH] docs: review the Workspace hook design against KDE's update and hide docs/dev/core-model.md section 9: three corrections to section 8 found by reading KDE's workspace.rs next to the Windows one (begin_update must be fallible and mutating because KDE prunes duplicates and clamps the focus first; end_update must return Pass::Again because Windows' update loops for scrolling; a scrolling_in_motion condition is read mid-update on both platforms; adjust_layout is per container for the stackbar), the error policy of place_window, the orphan-rule cost of extension traits, that WorkspaceConfig is already shared, that Monitor cannot move before Workspace (it uses about 25 of its members), the visible-windows decision for the owner, and the order of what remains. Verified: not applicable (documentation). --- docs/dev/core-model.md | 63 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/docs/dev/core-model.md b/docs/dev/core-model.md index fa82b88f..d7219b71 100644 --- a/docs/dev/core-model.md +++ b/docs/dev/core-model.md @@ -419,3 +419,66 @@ both platforms (behaviour-preserving on both), one hook per revision in the orde `unmaximize_window`. Gates: Windows `commands`, `workspaces`, `stacking`, `scrolling`, `resize`, `borders`, `animation` scenarios per hook; KDE fast tier and VM for `adjust_layout`, `after_hide`, `unmaximize_window`. + +## 9. Review of section 8 against KDE's code, and the order that follows from it + +Read against `komorebi-for-kde/komorebi/src/workspace.rs` (`update`, `hide`, `remove_window`) and the +Windows `update`. Section 8 holds, with these corrections and additions; none changes the idea, but +three change a signature. + +1. **`begin_update` must be fallible and may change the workspace.** KDE's `update` starts with + `self.prune_duplicate_windows()?` and then clamps `focused_container_idx` to the container count. + Both are before anything is laid out, so they belong in `begin_update(&mut ws) -> + eyre::Result>`, not in the shared `update`. (Windows has no such step.) +2. **`end_update` must say whether to run the pass again.** The Windows `update` is a loop: after the + placement pass, `if pass == 0 && is_scrolling && !animations_enabled() && adopt_actual_widths(..) + { pass += 1; continue; }`. A hook that returns nothing cannot express that; it must be + `end_update(&mut ws) -> eyre::Result` with `enum Pass { Done, Again }`, and the shared `update` + loops while it answers `Again` (at most one repeat, as Windows does; KDE always says `Done`). +3. **There is one more condition read in the middle of `update`, a hook of its own.** Where the scrolling + layout decides whether the pass is animated, Windows reads `crate::scrolling::animations_enabled() && + !live_resize::is_stepping()` and KDE reads `scrolling_placed.is_some_and(Placed::has_settled) && + !live_resize_in_progress()`. Section 8 files `has_settled` under `end_update`, but KDE reads it before + the layouts are computed. Add `fn scrolling_in_motion(&ws) -> bool` (default `false`); `scrolling_placed` + is set at the end of KDE's pass, in `end_update`. +4. **Error policy of `place_window` differs** and the hook returns `Result` to carry both: Windows + propagates a failed `set_rect` with `?`, KDE logs it (`tracing::warn!("failed to set window + position")`) and goes on, while `center(..)?` in the percentage override does propagate. The KDE + `place_window` swallows exactly the `set_position` error; the shared caller always uses `?`. +5. **`adjust_layout` is not enough for Windows' stackbar**: it subtracts the stackbar height from the + layout of a container with more than one window (`should_have_stackbar(window_count)`). That is a + per-container adjustment, so `adjust_layout` is called per container with its index, + `adjust_layout(&ws, idx: usize, layout: &mut Rect, work_area: &Rect)`, not once with the slice; KDE's + clamp is per layout as well and only when the workspace is not scrolling. +6. **KDE wraps the placement in a closure** (`let result = (|| { .. })(); result?;`) with nothing between the closure and the `?`, so it is equivalent to no closure (class (a) drift: the closure only keeps the `?`s of the placement local). The shared `update` needs no such shape; a Windows `AnimationBatch` guard drops on any exit and is compatible with either. +7. **Orphan rule:** a method only one platform has cannot be an inherent method of the generic + `Workspace

` in the crate. `adopt_maximized_window`, `prune_duplicate_windows` and + `reap_invalid_windows` (KDE), `border_kinds_on_show`, `windows_on_show` (Windows) become extension + traits, and each file that calls them imports the trait (6 call sites of the KDE three outside + `workspace.rs` today). The extension traits are the price of keeping the `Workspace` fields `pub`. +8. **`WorkspaceConfig` and `Wallpaper` are already shared** (`komorebi-config`, `komorebi-protocol`), so + `load_static_config` is not blocked by the daemon's `static_config.rs`; it can move into the crate + (the Windows custom-layout arms stay in it). +9. **`Monitor` cannot move before `Workspace`.** The shared Monitor code uses about 25 members of its + workspace (`layout`, `globals`, `tile`, `layer`, `containers_mut`, `add_container_to_back`, + `insert_container_at_idx`, `floating_windows_mut`, `reintegrate_monocle_container`, ...). Once + `Workspace

` exists those are inherent members and `Monitor

` takes `Ring>` + directly; before that, `Monitor` would need the 25-member trait that was ruled out. The order is + therefore: `WorkspaceGlobals` (plain data), `Workspace

`, `Monitor

`; the request to do `Monitor` + first is only possible after the first. +10. **Decision for the owner, `visible-windows`.** Windows' `visible_window_details` used `try_into` and + skipped a window whose fields could not be read; the shared `visible_window_details` uses `into`, so + after kfw's revision 4 Windows lists such a window with empty strings in `komorebic visible-windows`. + The shared code depends on the infallible form; the alternative is a `WindowDetails: TryFrom` + shared with a skip for `Err`, which KDE (always `Ok`) would not notice. Recommended: keep `into`. + +### Order of the code that remains (all gated like section 8.5) + +1. `WorkspaceGlobals` into `komorebi-core` (identical plain data; this stack does it). +2. `Workspace

` struct and the 50 identical functions, `P::Window: WorkspaceWindow` + (`ContainerWindow` plus `focus`, `maximize`, `unmaximize`, `title`, `exe`, `class`, `path`, `rect`, + `center`, `set_position`), the differing functions as hooks or extension traits as in sections 8 + and 9. About 2000 lines move; every call site that used an inherent method of a one-platform + function imports the extension trait. This is one revision per cluster of functions, and it is + the first step that cannot be verified on Windows without the e2e scenarios. +3. `Monitor

` with `#[serde(flatten)] platform: P`.