From 6a1721b9f7540507b01f9276fc6c73e0929f0190 Mon Sep 17 00:00:00 2001 From: Lars Berger Date: Wed, 7 Aug 2024 22:00:01 +0800 Subject: [PATCH] fix: use `bind_to_monitor` from config when focusing or moving to a workspace --- .../wm/src/common/commands/reload_config.rs | 4 +- .../wm/src/monitors/commands/add_monitor.rs | 2 +- packages/wm/src/user_config.rs | 24 +++--- .../commands/move_window_to_workspace.rs | 25 +++--- .../workspaces/commands/activate_workspace.rs | 81 +++++++++++++------ .../workspaces/commands/focus_workspace.rs | 5 +- .../commands/move_workspace_in_direction.rs | 2 +- 7 files changed, 91 insertions(+), 52 deletions(-) diff --git a/packages/wm/src/common/commands/reload_config.rs b/packages/wm/src/common/commands/reload_config.rs index 1324a21c5..8c233af8a 100644 --- a/packages/wm/src/common/commands/reload_config.rs +++ b/packages/wm/src/common/commands/reload_config.rs @@ -77,7 +77,9 @@ fn update_workspace_configs( // When the workspace config is not found, the current name of the // workspace has been removed. So, we reassign the first suitable // workspace config to the workspace. - config.workspace_config_for_monitor(&monitor, &workspaces) + config + .workspace_config_for_monitor(&monitor, &workspaces) + .or_else(|| config.next_inactive_workspace_config(&workspaces)) }); match workspace_config { diff --git a/packages/wm/src/monitors/commands/add_monitor.rs b/packages/wm/src/monitors/commands/add_monitor.rs index ea0f9f5c7..a2a36882e 100644 --- a/packages/wm/src/monitors/commands/add_monitor.rs +++ b/packages/wm/src/monitors/commands/add_monitor.rs @@ -37,7 +37,7 @@ pub fn add_monitor( }); // Activate a workspace on the newly added monitor. - activate_workspace(None, &monitor, state, config) + activate_workspace(None, Some(monitor), state, config) .context("At least 1 workspace is required per monitor.")?; Ok(()) diff --git a/packages/wm/src/user_config.rs b/packages/wm/src/user_config.rs index dc63b03c1..458b1440e 100644 --- a/packages/wm/src/user_config.rs +++ b/packages/wm/src/user_config.rs @@ -287,21 +287,27 @@ impl UserConfig { let inactive_configs = self.inactive_workspace_configs(active_workspaces); - let bound_config = inactive_configs.iter().find(|&config| { + inactive_configs.into_iter().find(|&config| { config .bind_to_monitor .as_ref() .map(|monitor_index| monitor.index() == *monitor_index as usize) .unwrap_or(false) - }); + }) + } - // Get the first workspace config that isn't bound to a monitor. - bound_config - .or( - inactive_configs - .iter() - .find(|config| config.bind_to_monitor.is_none()), - ) + /// Gets the first inactive workspace config, prioritizing configs that + /// don't have a monitor binding. + pub fn next_inactive_workspace_config( + &self, + active_workspaces: &Vec, + ) -> Option<&WorkspaceConfig> { + let inactive_configs = + self.inactive_workspace_configs(active_workspaces); + + inactive_configs + .iter() + .find(|config| config.bind_to_monitor.is_none()) .or(inactive_configs.first()) .cloned() } diff --git a/packages/wm/src/windows/commands/move_window_to_workspace.rs b/packages/wm/src/windows/commands/move_window_to_workspace.rs index 065053603..cba1c139c 100644 --- a/packages/wm/src/windows/commands/move_window_to_workspace.rs +++ b/packages/wm/src/windows/commands/move_window_to_workspace.rs @@ -31,7 +31,7 @@ pub fn move_window_to_workspace( Some(_) => anyhow::Ok(target_workspace), _ => match target_workspace_name { Some(name) => { - activate_workspace(Some(&name), ¤t_monitor, state, config)?; + activate_workspace(Some(&name), None, state, config)?; Ok(state.workspace_by_name(&name)) } @@ -124,17 +124,18 @@ pub fn move_window_to_workspace( state.pending_sync.focus_change = true; } - match window { - WindowContainer::NonTilingWindow(_) => { - state.pending_sync.containers_to_redraw.push(window.into()); - } - WindowContainer::TilingWindow(_) => { - state - .pending_sync - .containers_to_redraw - .extend(current_workspace.tiling_children().map(Into::into)); - } - } + let containers_to_redraw = match window { + WindowContainer::NonTilingWindow(_) => vec![window.into()], + WindowContainer::TilingWindow(_) => current_workspace + .tiling_children() + .map(Into::into) + .collect(), + }; + + state + .pending_sync + .containers_to_redraw + .extend(containers_to_redraw); } Ok(()) diff --git a/packages/wm/src/workspaces/commands/activate_workspace.rs b/packages/wm/src/workspaces/commands/activate_workspace.rs index 1854450d5..207445441 100644 --- a/packages/wm/src/workspaces/commands/activate_workspace.rs +++ b/packages/wm/src/workspaces/commands/activate_workspace.rs @@ -3,7 +3,10 @@ use anyhow::Context; use super::sort_workspaces; use crate::{ common::TilingDirection, - containers::{commands::attach_container, traits::PositionGetters}, + containers::{ + commands::attach_container, + traits::{CommonGetters, PositionGetters}, + }, monitors::Monitor, user_config::{UserConfig, WorkspaceConfig}, wm_event::WmEvent, @@ -15,14 +18,39 @@ use crate::{ /// /// If no workspace name is provided, the first suitable workspace defined /// in the user's config will be used. +/// +/// If no target monitor is provided, the workspace is activated on +/// whichever monitor it is bound to, or the currently focused monitor. pub fn activate_workspace( workspace_name: Option<&str>, - target_monitor: &Monitor, + target_monitor: Option, state: &mut WmState, config: &UserConfig, ) -> anyhow::Result<()> { - let workspace_config = - workspace_config(workspace_name, target_monitor, state, config)?; + let workspace_config = workspace_config( + workspace_name, + target_monitor.clone(), + state, + config, + )?; + + let target_monitor = target_monitor + .or_else(|| { + workspace_config + .bind_to_monitor + .and_then(|index| { + state + .monitors() + .into_iter() + .find(|monitor| monitor.index() == index as usize) + }) + .or_else(|| { + state + .focused_container() + .and_then(|focused| focused.monitor()) + }) + }) + .context("Failed to get a target monitor for the workspace.")?; let monitor_rect = target_monitor.to_rect()?; let tiling_direction = match monitor_rect.height() > monitor_rect.width() @@ -56,28 +84,33 @@ pub fn activate_workspace( /// Gets config for the workspace to activate. fn workspace_config( workspace_name: Option<&str>, - target_monitor: &Monitor, + target_monitor: Option, state: &mut WmState, config: &UserConfig, ) -> anyhow::Result { - match workspace_name { - Some(workspace_name) => { - let found_config = config - .inactive_workspace_configs(&state.workspaces()) - .into_iter() - .find(|config| config.name == workspace_name) - .with_context(|| { - format!("Workspace with name {} doesn't exist.", workspace_name) - })?; - - Ok(found_config.clone()) - } - None => { - let inactive_config = config - .workspace_config_for_monitor(&target_monitor, &state.workspaces()) - .context("No workspace config found for monitor.")?; + let found_config = match workspace_name { + Some(workspace_name) => config + .inactive_workspace_configs(&state.workspaces()) + .into_iter() + .find(|config| config.name == workspace_name) + .with_context(|| { + format!( + "Workspace with name '{}' doesn't exist or is already active.", + workspace_name + ) + }), + None => target_monitor + .and_then(|target_monitor| { + config.workspace_config_for_monitor( + &target_monitor, + &state.workspaces(), + ) + }) + .or_else(|| { + config.next_inactive_workspace_config(&state.workspaces()) + }) + .context("No workspace config available to activate workspace."), + }; - Ok(inactive_config.clone()) - } - } + found_config.cloned() } diff --git a/packages/wm/src/workspaces/commands/focus_workspace.rs b/packages/wm/src/workspaces/commands/focus_workspace.rs index 5691043be..a56a79ede 100644 --- a/packages/wm/src/workspaces/commands/focus_workspace.rs +++ b/packages/wm/src/workspaces/commands/focus_workspace.rs @@ -35,10 +35,7 @@ pub fn focus_workspace( Some(_) => anyhow::Ok(target_workspace), _ => match target_workspace_name { Some(name) => { - let focused_monitor = - focused_workspace.monitor().context("No focused monitor.")?; - - activate_workspace(Some(&name), &focused_monitor, state, config)?; + activate_workspace(Some(&name), None, state, config)?; Ok(state.workspace_by_name(&name)) } diff --git a/packages/wm/src/workspaces/commands/move_workspace_in_direction.rs b/packages/wm/src/workspaces/commands/move_workspace_in_direction.rs index 61aa1865a..551f61f45 100644 --- a/packages/wm/src/workspaces/commands/move_workspace_in_direction.rs +++ b/packages/wm/src/workspaces/commands/move_workspace_in_direction.rs @@ -57,7 +57,7 @@ pub fn move_workspace_in_direction( // Prevent original monitor from having no workspaces. if monitor.child_count() == 0 { - activate_workspace(None, &monitor, state, config)?; + activate_workspace(None, Some(monitor), state, config)?; } sort_workspaces(target_monitor, config)?;