From 375571d06329e7be4a87f4c296489014103f3a82 Mon Sep 17 00:00:00 2001 From: LGUG2Z Date: Sat, 31 Oct 2026 11:29:05 -0700 Subject: [PATCH] refactor(windows): the container is komorebi-core's Container over the Windows Window BEHAVIOUR (Windows runtime): not verified on a machine; see below. komorebi/src/container.rs is now `type Container = komorebi_core::Container` plus the ContainerWindow impl for the Windows Window (id is the hwnd, exe_name is Window::exe().ok(), hide and restore are backend().hide / .restore exactly as the container called them, and the stacked pair is hide_with_border(false) / restore_with_border(false) as load_focused_window used). hwnd_from_exe, which only Windows has, is the ContainerExt trait that workspace.rs imports. The container's operations, their order and the serialised form are the ones in the crate (Container.locked keeps its serde default, and the schemars schema is still named Container). The generic container tests moved into komorebi-core; the JSON pin and the deserialize tests stay here. komorebi-core is a path dependency, with its schemars feature under komorebi's. Why BEHAVIOUR: the container is the type of every container in the state JSON and the code of every stack and tile operation, and I could not run it. By construction the calls are the same, in the same order. Windows e2e scenarios needed: the tiled, stack and float state scenarios whose fixtures are e2e/lib/tests/fixtures/kfw-{tiled,stack, float}.json (komorebic state must be byte-identical), stack/unstack and cycle-stack commands, a stack on a monitor move, monocle and move-to-workspace of a stacked container, and the schema check (schema.json unchanged). Verified: tools/fast-check windows step = nix develop cargo clippy --workspace --locked --all-targets -D warnings: PASS (komorebi tests compiled, not run); komorebi-core tests PASS (native); macOS fake-CC type-check PASS (it does not use the crate). Not verified: any run of the Windows tests or daemon. --- Cargo.lock | 1 + komorebi/Cargo.toml | 2 + komorebi/src/container.rs | 287 +++++--------------------------------- komorebi/src/workspace.rs | 1 + 4 files changed, 42 insertions(+), 249 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4fcb610..6195435c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3349,6 +3349,7 @@ dependencies = [ "komorebi-backend", "komorebi-collections", "komorebi-config", + "komorebi-core", "komorebi-layouts", "komorebi-protocol", "komorebi-scrolling", diff --git a/komorebi/Cargo.toml b/komorebi/Cargo.toml index ee512acd..01d4685c 100644 --- a/komorebi/Cargo.toml +++ b/komorebi/Cargo.toml @@ -10,6 +10,7 @@ edition = "2024" [dependencies] komorebi-backend = { path = "../crates/komorebi-backend" } komorebi-collections = { path = "../crates/komorebi-collections" } +komorebi-core = { path = "../crates/komorebi-core" } komorebi-config = { path = "../crates/komorebi-config", default-features = false } komorebi-layouts = { path = "../komorebi-layouts", features = ["win32"] } komorebi-scrolling = { path = "../komorebi-scrolling" } @@ -73,6 +74,7 @@ deadlock_detection = ["parking_lot/deadlock_detection"] schemars = [ "dep:schemars", "komorebi-collections/schemars", + "komorebi-core/schemars", "komorebi-config/schemars", "komorebi-layouts/schemars", "komorebi-protocol/schemars", diff --git a/komorebi/src/container.rs b/komorebi/src/container.rs index e9116ffd..4e5b329c 100644 --- a/komorebi/src/container.rs +++ b/komorebi/src/container.rs @@ -1,97 +1,52 @@ -use nanoid::nanoid; -use serde::Deserialize; -use serde::Serialize; +//! [`Container`], the windows that share one tile of a workspace's layout. +//! +//! The container itself is [`komorebi_core::Container`], shared with the other komorebi daemons; +//! here it is the one over the Windows [`Window`], a handle that is an `HWND`. -use crate::Lockable; -use crate::ring::Ring; use crate::window::Window; use crate::windows_backend::backend; use komorebi_backend::WindowId; +use komorebi_core::ContainerWindow; -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] -pub struct Container { - pub id: String, - #[serde(default)] - pub locked: bool, - pub windows: Ring, -} +/// The windows that share one tile of a workspace's layout +pub type Container = komorebi_core::Container; -impl_ring_elements!(Container, Window); +impl ContainerWindow for Window { + type Id = isize; -impl Default for Container { - fn default() -> Self { - Self { - id: nanoid!(), - locked: false, - windows: Ring::default(), - } + fn id(&self) -> isize { + self.hwnd + } + + fn exe_name(&self) -> Option { + Window::exe(*self).ok() + } + + fn hide_window(&self) { + let _ = backend().hide(WindowId::from_hwnd(self.hwnd)); + } + + fn restore_window(&self) { + let _ = backend().restore(WindowId::from_hwnd(self.hwnd)); + } + + fn hide_stacked(&self) { + self.hide_with_border(false); + } + + fn restore_stacked(&self) { + self.restore_with_border(false); } } -impl Lockable for Container { - fn locked(&self) -> bool { - self.locked - } - - fn set_locked(&mut self, locked: bool) -> &mut Self { - self.locked = locked; - self - } +/// What only the Windows container has +pub trait ContainerExt { + /// The handle of the first window of the executable `exe` in this container + fn hwnd_from_exe(&self, exe: &str) -> Option; } -impl Container { - pub fn preselect() -> Self { - Self { - id: "PRESELECT".to_string(), - locked: false, - windows: Default::default(), - } - } - - pub fn is_preselect(&self) -> bool { - self.id == "PRESELECT" - } - - pub fn hide(&self, omit: Option) { - for window in self.windows().iter().rev() { - let mut should_hide = omit.is_none(); - - if !should_hide - && let Some(omit) = omit - && omit != window.hwnd - { - should_hide = true - } - - if should_hide { - let _ = backend().hide(WindowId::from_hwnd(window.hwnd)); - } - } - } - - pub fn restore(&self) { - if let Some(window) = self.focused_window() { - let _ = backend().restore(WindowId::from_hwnd(window.hwnd)); - } - } - - /// 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. - pub fn load_focused_window(&mut self) { - let focused_idx = self.focused_window_idx(); - - for (i, window) in self.windows_mut().iter_mut().enumerate() { - if i == focused_idx { - window.restore_with_border(false); - } else { - window.hide_with_border(false); - } - } - } - - pub fn hwnd_from_exe(&self, exe: &str) -> Option { +impl ContainerExt for Container { + fn hwnd_from_exe(&self, exe: &str) -> Option { for window in self.windows() { if let Ok(window_exe) = window.exe() && exe == window_exe @@ -102,180 +57,14 @@ impl Container { None } - - pub fn idx_from_exe(&self, exe: &str) -> Option { - for (idx, window) in self.windows().iter().enumerate() { - if let Ok(window_exe) = window.exe() - && exe == window_exe - { - return Option::from(idx); - } - } - - None - } - - pub fn contains_window(&self, hwnd: isize) -> bool { - for window in self.windows() { - if window.hwnd == hwnd { - return true; - } - } - - false - } - - pub fn idx_for_window(&self, hwnd: isize) -> Option { - for (i, window) in self.windows().iter().enumerate() { - if window.hwnd == hwnd { - return Option::from(i); - } - } - - None - } - - pub fn remove_window_by_idx(&mut self, idx: usize) -> Option { - let window = self.windows_mut().remove(idx); - self.focus_window(idx.saturating_sub(1)); - window - } - - pub fn remove_focused_window(&mut self) -> Option { - let focused_idx = self.focused_window_idx(); - self.remove_window_by_idx(focused_idx) - } - - pub fn add_window(&mut self, window: Window) { - self.windows_mut().push_back(window); - self.focus_window(self.windows().len().saturating_sub(1)); - let focused_window_idx = self.focused_window_idx(); - - for (i, window) in self.windows().iter().enumerate() { - if i != focused_window_idx { - let _ = backend().hide(WindowId::from_hwnd(window.hwnd)); - } - } - } - - #[tracing::instrument(skip(self))] - pub fn focus_window(&mut self, idx: usize) { - tracing::info!("focusing window"); - self.windows.focus(idx); - } } #[cfg(test)] mod tests { use super::*; + use crate::Lockable; use serde_json; - #[test] - fn test_contains_window() { - let mut container = Container::default(); - - for i in 0..3 { - container.add_window(Window::from(i)); - } - - // Should return true for existing windows - assert!(container.contains_window(1)); - assert_eq!(container.idx_for_window(1), Some(1)); - - // Should return false since window 4 doesn't exist - assert!(!container.contains_window(4)); - assert_eq!(container.idx_for_window(4), None); - } - - #[test] - fn test_remove_window_by_idx() { - let mut container = Container::default(); - - for i in 0..3 { - container.add_window(Window::from(i)); - } - - // Remove window 1 - container.remove_window_by_idx(1); - - // Should only have 2 windows left - assert_eq!(container.windows().len(), 2); - - // Should return false since window 1 was removed - assert!(!container.contains_window(1)); - } - - #[test] - fn test_remove_focused_window() { - let mut container = Container::default(); - - for i in 0..3 { - container.add_window(Window::from(i)); - } - - // Should be focused on the last created window - assert_eq!(container.focused_window_idx(), 2); - - // Remove the focused window - container.remove_focused_window(); - - // Should be focused on the window before the removed one - assert_eq!(container.focused_window_idx(), 1); - - // Should only have 2 windows left - assert_eq!(container.windows().len(), 2); - } - - #[test] - fn test_add_window() { - let mut container = Container::default(); - - container.add_window(Window::from(1)); - - assert_eq!(container.windows().len(), 1); - assert_eq!(container.focused_window_idx(), 0); - assert!(container.contains_window(1)); - } - - #[test] - fn test_focus_window() { - let mut container = Container::default(); - - for i in 0..3 { - container.add_window(Window::from(i)); - } - - // Should focus on the last created window - assert_eq!(container.focused_window_idx(), 2); - - // focus on the window at index 1 - container.focus_window(1); - - // Should be focused on window 1 - assert_eq!(container.focused_window_idx(), 1); - - // focus on the window at index 0 - container.focus_window(0); - - // Should be focused on window 0 - assert_eq!(container.focused_window_idx(), 0); - } - - #[test] - fn test_idx_for_window() { - let mut container = Container::default(); - - for i in 0..3 { - container.add_window(Window::from(i)); - } - - // Should return the index of the window - assert_eq!(container.idx_for_window(1), Some(1)); - - // Should return None since window 4 doesn't exist - assert_eq!(container.idx_for_window(4), None); - } - #[test] fn deserializes_with_missing_locked_field_defaults_to_false() { let json = r#"{ diff --git a/komorebi/src/workspace.rs b/komorebi/src/workspace.rs index 1d5e83cf..323637fb 100644 --- a/komorebi/src/workspace.rs +++ b/komorebi/src/workspace.rs @@ -22,6 +22,7 @@ use crate::WindowContainerBehaviour; use crate::animation::engine::AnimationBatch; use crate::border_manager; use crate::container::Container; +use crate::container::ContainerExt; use crate::core::Axis; use crate::core::CustomLayout; use crate::core::CycleDirection;