refactor(mac): the daemon uses the shared Container API with the hide spot

Deep dedup D-coord-13 (1), the container API. BEHAVIOUR (type-checked and
clippy-clean with the fake CC; the macOS tests are the oracle).

ContainerExt is deleted. Its four methods were the shared Container's with
the monitor's corner as an argument; every call (about 28 sites in
window_manager.rs, process_event.rs, scrolling.rs, workspace.rs, monitor.rs) is now
the shared one: add_window(window, spot), load_focused_window(spot), hide(omit,
spot), restore(), with spot = workspace.hide_spot() (MacosPlatform::hide_spot:
the workspace's corner, no display, which is what the old calls passed;
hide_on's screen argument was None at every remaining caller; the whole-workspace
hide with the display is Workspace::hide, already shared). The nine window
manager functions that differed from KDE's only in these calls (add_window_to_container,
cycle_container_window_in_direction, cycle_container_window_index_in_direction,
focus_container_window, stack_all, monocle_on, monocle_off, cycle_monocle,
transfer_window) now differ from KDE's text only in the spot and in the
error policy (let _ against ?), which the next revisions remove.

Two things change at run time, both small: monocle_on and cycle_monocle took the
corner from the monitor (monitor.extra.window_hiding_position) and now take it
from the workspace's state, which the monitor writes into every workspace when it
updates their globals, so they differ only if a workspace's globals are stale
(a config reload without a layout update in between); and Container::restore
skips the Accessibility call under cfg!(test), as hide and load_focused_window
already did.

Verified: fast-check macOS PASS; fake-CC cargo clippy -p komorebi-macos
--all-targets -D warnings clean. Not verified: any macOS test or run.
Verified on the Mini: not yet. Hand check row added to docs/dev/macos-handcheck.md.
This commit is contained in:
LGUG2Z committed 2026-11-05 15:03:26 -08:00
1 parent 401ce9112a
commit a478495049
7 files changed
+46 -177

No files matched your search

+1
View File
@@ -97,6 +97,7 @@ timeout. Until then H5 documents the hang.
| 11(iii) Workspace: `refactor(mac): the daemon's Workspace is komorebi_core::Workspace<MacosPlatform>` (see `jj log -r wip-u-kfm2`) | About 50 workspace functions (container and focus operations, placement, resize constraints, window lookups, floating helpers, the stack and monocle operations that hide windows) are now the shared ones; `hide`, `new_container_for_window` and the stack functions pass the monitor's corner through `MacosPlatform::hide_spot` (kept in `MacosWorkspaceState`, set by the monitor with the globals), and `hide` passes the display bounds for the containers through `hide_spot_on_display`. macOS's own `update`, `apply_wallpaper`, `restore`, `remove_window`, `load_static_config`, `prune_duplicate_windows`, `visible_window_details`, `reap_invalid_windows_for_application`, `restore_windows`, `calculate_layouts` and `set_scrolling_columns` are `WorkspaceExt`; the scrolling methods are `WorkspaceScrolling`. | `cargo test` first (the workspace, scrolling and window_manager tests). Then: stack and unstack, switch workspaces both ways (each stack shows its front window, the hidden ones are in their monitor's corner), toggle monocle and back, float and unfloat a window, cycle the monocle container, move a window to another workspace, move a container across monitors; with a different `window_hiding_position` on each monitor check each monitor's windows park in their own corner and that a floating window that overhangs the edge of a display is still parked on its own display. `komorebic state` keys of a workspace are unchanged (pinned by `state_pin`). Scrolling layout: focus, move, consume and expel, pan, resize a column. |
| 11(iii) Monitor: `refactor(mac): the daemon's Monitor is komorebi_core::Monitor<MacosPlatform>` (see `jj log -r wip-u-kfm2`) | The monitor is the shared generic type; the corner (`window_hiding_position`) is the platform's flattened `extra` (written last, as before), the constructors, `update_*_globals`, `load_focused_workspace`, `move_container_to_workspace(1)`, `add_container_with_direction`, `hide_unfocused_workspaces` and `restore_focused_workspace` are `MonitorExt`; `focus_workspace`, `add_container`, `remove_workspace*`, `ensure_workspace_count` and the name and layout lookups are the shared ones. The monitor gives its workspaces the corner through `workspace.state` when it updates their globals. | `cargo test` (monitor tests, `state_pin`). By hand: `komorebic state | jq '.monitors.elements[0] | keys_unsorted'` is the same list as before with `window_hiding_position` last; switch workspaces on each monitor (hide and restore of the whole workspace), focus-workspace by index and by name, move a container to another workspace and to another monitor, unplug and replug a display (the monitor and its workspaces are rebuilt through `Monitor::new` and `update_all_workspace_globals`), a config with `window_hiding_position` per monitor. |
| `wip-u-kfm3` registry fixes (see `jj log -r wip-u-kfm3`) | The registry keeps inert application copies (no blocking Accessibility call when an entry is dropped), a window is registered when its observer is attached, a clone being dropped no longer deregisters its window, `forget_pid` runs ahead of the pause and other-space returns, and the application reaper forgets what it reaps. Nothing reads the registry except the unused `MacosBackend`. | H3, H4, H6, H7 above, and the usual walk-through: no change in what the daemon does is expected. |
| deep dedup 1: `refactor(mac): the daemon uses the shared Container API with the hide spot` (see `jj log -r wip-u-mac-deep`) | `ContainerExt` (`add_window_at`, `load_focused_window_at`, `hide_on`, `restore_focused_window`) is gone: every call is the shared `Container::{add_window, load_focused_window, hide, restore}` with `workspace.hide_spot()`. `monocle_on` and `cycle_monocle` took the corner from the monitor (`monitor.extra`) and now take it from the workspace (`workspace.state`), which the monitor writes with the globals; `restore()` now skips the Accessibility call under `cfg!(test)`. | `cargo test`; then H1 (hide spot) and: toggle monocle on a workspace with two containers on a monitor with `window_hiding_position: BottomRight` (every other container parks in that corner), cycle the monocle container, stack with `komorebic stack`/`stack-all`, `cycle-stack`, `add-window-to-container`, switch workspaces. Change the monitor's `window_hiding_position` in the config, reload, and toggle monocle at once: the corner must be the new one. |
Revisions in section B change no behaviour on purpose (rename, move, route); they still need the sections C walk-through, because a mechanical mistake would show there.
+10 -127
View File
@@ -1,14 +1,11 @@
//! [`Container`], the windows that share one tile of a workspace's layout.
//!
//! The container is [`komorebi_core::Container`], shared with the other komorebi daemons, over the
//! macOS [`Window`]. What it cannot share is how a window is hidden: macOS needs the monitor's
//! `WindowHidingPosition` (and for a whole container the bounds of its display) with every hide,
//! and a hide can fail. Those four operations are [`ContainerExt`], under names the shared
//! container does not use (`add_window_at`, `load_focused_window_at`, `hide_on`,
//! `restore_focused_window`), because an inherent method of the shared type would hide an
//! extension method of the same name.
//! macOS [`Window`]. How a window is hidden is [`MacosHideSpot`]: the monitor's `WindowHidingPosition`
//! and, for the containers of a workspace that is being hidden, the bounds of its display. The
//! workspace gives it with `Workspace::hide_spot` and every container call takes it, as on the other
//! platforms (where it is `()`).
use crate::accessibility::error::AccessibilityError;
use crate::core::WindowHidingPosition;
use crate::window::Window;
use color_eyre::eyre;
@@ -94,120 +91,6 @@ impl komorebi_core::WorkspaceWindow for Window {
}
}
/// What a macOS container does that the shared one cannot: hide and restore its windows with the
/// monitor's hiding position, and say when that failed.
pub trait ContainerExt {
/// Puts a window at the back of the container and gives it the front, hiding every other
/// window of the container at `hiding_position`
fn add_window_at(
&mut self,
window: &Window,
hiding_position: WindowHidingPosition,
) -> eyre::Result<()>;
/// Hides the unfocused windows of the container and restores the focused one. This function
/// is used to make sure we update the window that should be shown on a stack. If the container
/// isn't a stack this function won't change anything.
fn load_focused_window_at(
&mut self,
hiding_position: WindowHidingPosition,
) -> Result<(), AccessibilityError>;
/// Hide the windows of the container, other than `omit`, on `screen`, the bounds of the display
/// of the monitor that has the container, or on whichever display they are found on if that is
/// not given
fn hide_on(
&mut self,
hiding_position: WindowHidingPosition,
omit: Option<u32>,
screen: Option<CGRect>,
) -> eyre::Result<()>;
/// Restores the window in front, which is the one that is shown when the container is a stack
fn restore_focused_window(&mut self) -> eyre::Result<()>;
}
impl ContainerExt for Container {
fn add_window_at(
&mut self,
window: &Window,
hiding_position: WindowHidingPosition,
) -> eyre::Result<()> {
self.windows_mut().push_back(window.clone());
self.focus_window(self.windows().len().saturating_sub(1));
if !cfg!(test) {
let focused_window_idx = self.focused_window_idx();
for (i, window) in self.windows_mut().iter_mut().enumerate() {
if i != focused_window_idx {
window.hide(hiding_position)?;
}
}
} else {
tracing::info!("not hiding windows to {hiding_position} during test execution")
}
Ok(())
}
fn load_focused_window_at(
&mut self,
hiding_position: WindowHidingPosition,
) -> Result<(), AccessibilityError> {
let focused_idx = self.focused_window_idx();
if !cfg!(test) {
for (i, window) in self.windows_mut().iter_mut().enumerate() {
if i == focused_idx {
window.restore()?;
} else {
window.hide(hiding_position)?;
}
}
} else {
tracing::info!(
"not hiding/restoring windows to/from {hiding_position} during test execution"
)
}
Ok(())
}
fn hide_on(
&mut self,
hiding_position: WindowHidingPosition,
omit: Option<u32>,
screen: Option<CGRect>,
) -> eyre::Result<()> {
for window in self.windows_mut().iter_mut().rev() {
let mut should_hide = omit.is_none();
if !should_hide
&& let Some(omit) = omit
&& omit != window.id
{
should_hide = true
}
// a window that has gone away must not leave the ones after it in view
if should_hide && let Err(error) = window.hide_on(hiding_position, screen) {
tracing::warn!("failed to hide window {}: {error}", window.id);
}
}
Ok(())
}
fn restore_focused_window(&mut self) -> eyre::Result<()> {
if let Some(window) = self.focused_window_mut() {
window.restore()?;
}
Ok(())
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -220,7 +103,7 @@ mod tests {
for i in 0..3 {
container
.add_window_at(&Window::from(i), WindowHidingPosition::BottomLeft)
.add_window(Window::from(i), Default::default())
.unwrap();
}
@@ -239,7 +122,7 @@ mod tests {
for i in 0..3 {
container
.add_window_at(&Window::from(i), WindowHidingPosition::BottomLeft)
.add_window(Window::from(i), Default::default())
.unwrap();
}
@@ -259,7 +142,7 @@ mod tests {
for i in 0..3 {
container
.add_window_at(&Window::from(i), WindowHidingPosition::BottomLeft)
.add_window(Window::from(i), Default::default())
.unwrap();
}
@@ -281,7 +164,7 @@ mod tests {
let mut container = Container::default();
container
.add_window_at(&Window::from(1), WindowHidingPosition::BottomLeft)
.add_window(Window::from(1), Default::default())
.unwrap();
assert_eq!(container.windows().len(), 1);
@@ -295,7 +178,7 @@ mod tests {
for i in 0..3 {
container
.add_window_at(&Window::from(i), WindowHidingPosition::BottomLeft)
.add_window(Window::from(i), Default::default())
.unwrap();
}
@@ -321,7 +204,7 @@ mod tests {
for i in 0..3 {
container
.add_window_at(&Window::from(i), WindowHidingPosition::BottomLeft)
.add_window(Window::from(i), Default::default())
.unwrap();
}
+2 -3
View File
@@ -6,7 +6,6 @@
//! was), and what it does differently from the shared monitor is [`MonitorExt`].
use crate::container::Container;
use crate::container::ContainerExt;
use crate::core::DefaultLayout;
use crate::core::Layout;
use crate::core::OperationDirection;
@@ -396,7 +395,7 @@ impl MonitorExt for Monitor {
if target_workspace.monocle_container.is_some() {
for container in target_workspace.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
for window in target_workspace.floating_windows_mut() {
@@ -454,7 +453,7 @@ impl MonitorExt for Monitor {
if target_workspace.monocle_container.is_some() {
for container in target_workspace.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
target_workspace.reintegrate_monocle_container()?;
@@ -13,7 +13,6 @@ use crate::accessibility::notification_constants::AccessibilityNotification;
use crate::animation::animator;
use crate::app_kit_notification_constants::AppKitWorkspaceNotification;
use crate::border_manager;
use crate::container::ContainerExt;
use crate::core::DefaultLayout;
use crate::core::Layout;
use crate::core::Rect;
@@ -701,11 +700,11 @@ impl WindowManager {
self.update_focused_workspace(false, false)?;
}
WindowContainerBehaviour::Append => {
let window_hiding_position = workspace.state.window_hiding_position;
let spot = workspace.hide_spot();
workspace
.focused_container_mut()
.ok_or_eyre("there is no focused container")?
.add_window_at(&window, window_hiding_position)?;
.add_window(window.clone(), spot)?;
workspace.layer = WorkspaceLayer::Tiling;
self.update_focused_workspace(true, false)?;
}
+8 -9
View File
@@ -13,7 +13,6 @@
use crate::arrival;
use crate::container::Container;
use crate::container::ContainerExt;
use crate::core::LayoutOptions;
use crate::core::OperationDirection;
use crate::core::Rect;
@@ -60,7 +59,7 @@ fn show(container: &mut Container) -> eyre::Result<()> {
});
#[cfg(not(test))]
container.restore_focused_window()?;
container.restore()?;
Ok(())
}
@@ -632,7 +631,7 @@ impl WorkspaceScrolling for Workspace {
}
fn stack_scrolling_column(&mut self, column: &[String]) -> eyre::Result<()> {
let hiding_position = self.state.window_hiding_position;
let spot = self.hide_spot();
let focused_window = self
.focused_container()
@@ -654,7 +653,7 @@ impl WorkspaceScrolling for Workspace {
if let Some(container) = self.containers_mut().get_mut(idx) {
for window in &windows {
container.add_window_at(window, hiding_position)?;
container.add_window(window.clone(), spot)?;
}
// the window that had focus is the one that is showing
@@ -665,7 +664,7 @@ impl WorkspaceScrolling for Workspace {
}
if !cfg!(test) {
container.load_focused_window_at(hiding_position)?;
container.load_focused_window(spot)?;
}
}
@@ -675,7 +674,7 @@ impl WorkspaceScrolling for Workspace {
}
fn unstack_scrolling_column(&mut self) -> eyre::Result<bool> {
let hiding_position = self.state.window_hiding_position;
let spot = self.hide_spot();
let idx = self.focused_container_idx();
let Some(container) = self.containers_mut().get_mut(idx) else {
@@ -703,7 +702,7 @@ impl WorkspaceScrolling for Workspace {
let mut ids = vec![];
for (offset, window) in rest.iter().enumerate() {
let mut container = Container::default();
container.add_window_at(window, hiding_position)?;
container.add_window(window.clone(), spot)?;
ids.push(container.id.clone());
// it was one of the tabs that were out of sight, and has to be put where it is shown
@@ -1448,7 +1447,7 @@ mod tests {
.containers_mut()
.front_mut()
.unwrap()
.add_window_at(&Window::from(6_000_062), WindowHidingPosition::default())
.add_window(Window::from(6_000_062), Default::default())
.unwrap();
workspace.layout_scrolling(&AREA, 0);
@@ -1839,7 +1838,7 @@ mod tests {
.containers_mut()
.front_mut()
.unwrap()
.add_window_at(&Window::from(212), WindowHidingPosition::default())
.add_window(Window::from(212), Default::default())
.unwrap();
workspace.layout_scrolling(&AREA, 0);
assert_eq!(windows_of(&workspace), [vec![210, 212], vec![211]]);
+17 -28
View File
@@ -17,7 +17,6 @@ use crate::application::Application;
use crate::border_manager;
use crate::border_manager::BorderLook;
use crate::container::Container;
use crate::container::ContainerExt;
use crate::core::Arrangement;
use crate::core::Axis;
use crate::core::CrossBoundaryBehaviour;
@@ -1249,9 +1248,9 @@ impl WindowManager {
}
}
let window_hiding_position = workspace.state.window_hiding_position;
let spot = workspace.hide_spot();
if changed_focus && let Some(container) = workspace.focused_container_mut() {
container.load_focused_window_at(window_hiding_position)?;
container.load_focused_window(spot)?;
if let Some(window) = container.focused_window() {
window.focus(mouse_follows_focus)?;
}
@@ -1332,7 +1331,7 @@ impl WindowManager {
tracing::info!("cycling container windows");
let mouse_follows_focus = self.mouse_follows_focus;
let window_hiding_position = self.focused_workspace()?.state.window_hiding_position;
let spot = self.focused_workspace()?.hide_spot();
let container = self.focused_container_mut()?;
@@ -1347,7 +1346,7 @@ impl WindowManager {
let next_idx = direction.next_idx(current_idx, len);
container.focus_window(next_idx);
container.load_focused_window_at(window_hiding_position)?;
container.load_focused_window(spot)?;
if let Some(window) = container.focused_window() {
window.focus(mouse_follows_focus)?;
@@ -1545,17 +1544,12 @@ impl WindowManager {
pub fn monocle_on(&mut self) -> eyre::Result<()> {
tracing::info!("enabling monocle");
let hiding_position = self
.focused_monitor()
.ok_or_eyre("there is no monitor")?
.extra
.window_hiding_position;
let workspace = self.focused_workspace_mut()?;
workspace.new_monocle_container()?;
let spot = workspace.hide_spot();
for container in workspace.containers_mut() {
container.hide_on(hiding_position, None, None)?;
container.hide(None, spot)?;
}
Ok(())
@@ -1568,7 +1562,7 @@ impl WindowManager {
let workspace = self.focused_workspace_mut()?;
for container in workspace.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
workspace.reintegrate_monocle_container()
@@ -1582,17 +1576,12 @@ impl WindowManager {
return Ok(());
}
let hiding_position = self
.focused_monitor()
.ok_or_eyre("there is no monitor")?
.extra
.window_hiding_position;
self.focused_workspace_mut()?
.cycle_monocle_container(direction)?;
let spot = self.focused_workspace()?.hide_spot();
for container in self.focused_workspace_mut()?.containers_mut() {
container.hide_on(hiding_position, None, None)?;
container.hide(None, spot)?;
}
self.update_focused_workspace(true, true)
@@ -2611,7 +2600,7 @@ impl WindowManager {
if target_workspace.monocle_container.is_some() {
for container in target_workspace.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
for window in target_workspace.floating_windows_mut() {
@@ -2881,7 +2870,7 @@ impl WindowManager {
tracing::info!("cycling container window index");
let mouse_follows_focus = self.mouse_follows_focus;
let window_hiding_position = self.focused_workspace()?.state.window_hiding_position;
let spot = self.focused_workspace()?.hide_spot();
let container =
if let Some(container) = &mut self.focused_workspace_mut()?.monocle_container {
@@ -2902,7 +2891,7 @@ impl WindowManager {
container.windows_mut().swap(current_idx, next_idx);
container.focus_window(next_idx);
container.load_focused_window_at(window_hiding_position)?;
container.load_focused_window(spot)?;
if let Some(window) = container.focused_window() {
window.focus(mouse_follows_focus)?;
@@ -2916,7 +2905,7 @@ impl WindowManager {
tracing::info!("focusing container window at index {idx}");
let mouse_follows_focus = self.mouse_follows_focus;
let window_hiding_position = self.focused_workspace()?.state.window_hiding_position;
let spot = self.focused_workspace()?.hide_spot();
let container =
if let Some(container) = &mut self.focused_workspace_mut()?.monocle_container {
@@ -2937,7 +2926,7 @@ impl WindowManager {
}
container.focus_window(idx);
container.load_focused_window_at(window_hiding_position)?;
container.load_focused_window(spot)?;
if let Some(window) = container.focused_window() {
window.focus(mouse_follows_focus)?;
@@ -2951,7 +2940,7 @@ impl WindowManager {
tracing::info!("stacking all windows on workspace");
let workspace = self.focused_workspace_mut()?;
let window_hiding_position = workspace.state.window_hiding_position;
let spot = workspace.hide_spot();
let mut focused_hwnd = None;
if let Some(container) = workspace.focused_container()
@@ -2979,7 +2968,7 @@ impl WindowManager {
&& let Some(w_idx_to_focus) = c.idx_for_window(hwnd)
{
c.focus_window(w_idx_to_focus);
c.load_focused_window_at(window_hiding_position)?;
c.load_focused_window(spot)?;
}
self.update_focused_workspace(self.mouse_follows_focus, true)
}
@@ -3783,7 +3772,7 @@ impl WindowManager {
.ok_or_eyre("there is no workspace for this monitor")?;
for container in origin_workspace.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
origin_workspace.reintegrate_monocle_container()?;
+6 -7
View File
@@ -8,7 +8,6 @@
use crate::DATA_DIR;
use crate::REGEX_IDENTIFIERS;
use crate::container::ContainerExt;
use crate::container::MacosHideSpot;
use crate::core::DefaultLayout;
use crate::core::Layout;
@@ -464,7 +463,7 @@ impl WorkspaceExt for Workspace {
}
for c in self.containers_mut() {
c.restore_focused_window()?;
c.restore()?;
}
return Ok(window);
@@ -474,7 +473,7 @@ impl WorkspaceExt for Workspace {
.container_idx_for_window(window_id)
.ok_or_eyre("there is no window")?;
let window_hiding_position = self.state.window_hiding_position;
let spot = self.hide_spot();
// a strip of columns has to know what there is before it is told what goes
self.sync_scrolling_containers();
@@ -502,7 +501,7 @@ impl WorkspaceExt for Workspace {
self.focus_previous_container();
}
} else {
container.load_focused_window_at(window_hiding_position)?;
container.load_focused_window(spot)?;
if let Some(window) = container.focused_window() {
window.focus(false)?;
}
@@ -523,7 +522,7 @@ impl WorkspaceExt for Workspace {
fn restore_windows(&mut self, mouse_follows_focus: bool) -> eyre::Result<()> {
if let Some(container) = &mut self.monocle_container {
container.restore_focused_window()?;
container.restore()?;
if let Some(window) = container.focused_window() {
window.focus(mouse_follows_focus)?;
@@ -544,7 +543,7 @@ impl WorkspaceExt for Workspace {
}
for container in self.containers_mut() {
container.restore_focused_window()?;
container.restore()?;
}
if let Some(container) = self.focused_container_mut() {
@@ -1074,7 +1073,7 @@ mod tests {
let window = ws.floating_windows_mut().pop_back().unwrap();
let mut container = Container::default();
container
.add_window_at(&window, WindowHidingPosition::BottomLeft)
.add_window(window, Default::default())
.unwrap();
ws.insert_container_at_idx(ws.focused_container_idx(), container);