diff --git a/docs/dev/macos-handcheck.md b/docs/dev/macos-handcheck.md index 6ebaade9..b86aae80 100644 --- a/docs/dev/macos-handcheck.md +++ b/docs/dev/macos-handcheck.md @@ -97,6 +97,7 @@ timeout. Until then H5 documents the hang. | 11(iii) Workspace: `refactor(mac): the daemon's Workspace is komorebi_core::Workspace` (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` (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. diff --git a/komorebi-for-mac/komorebi/src/container.rs b/komorebi-for-mac/komorebi/src/container.rs index 617fc716..a4b07fbf 100644 --- a/komorebi-for-mac/komorebi/src/container.rs +++ b/komorebi-for-mac/komorebi/src/container.rs @@ -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, - screen: Option, - ) -> 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, - screen: Option, - ) -> 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(); } diff --git a/komorebi-for-mac/komorebi/src/monitor.rs b/komorebi-for-mac/komorebi/src/monitor.rs index c6e6747f..011d046b 100644 --- a/komorebi-for-mac/komorebi/src/monitor.rs +++ b/komorebi-for-mac/komorebi/src/monitor.rs @@ -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()?; diff --git a/komorebi-for-mac/komorebi/src/process_event.rs b/komorebi-for-mac/komorebi/src/process_event.rs index d4844df1..838fe544 100644 --- a/komorebi-for-mac/komorebi/src/process_event.rs +++ b/komorebi-for-mac/komorebi/src/process_event.rs @@ -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)?; } diff --git a/komorebi-for-mac/komorebi/src/scrolling.rs b/komorebi-for-mac/komorebi/src/scrolling.rs index c58d7127..9618cfb7 100644 --- a/komorebi-for-mac/komorebi/src/scrolling.rs +++ b/komorebi-for-mac/komorebi/src/scrolling.rs @@ -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 { - 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]]); diff --git a/komorebi-for-mac/komorebi/src/window_manager.rs b/komorebi-for-mac/komorebi/src/window_manager.rs index bf0c0158..62912a0d 100644 --- a/komorebi-for-mac/komorebi/src/window_manager.rs +++ b/komorebi-for-mac/komorebi/src/window_manager.rs @@ -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()?; diff --git a/komorebi-for-mac/komorebi/src/workspace.rs b/komorebi-for-mac/komorebi/src/workspace.rs index 00219297..e7aae902 100644 --- a/komorebi-for-mac/komorebi/src/workspace.rs +++ b/komorebi-for-mac/komorebi/src/workspace.rs @@ -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);