fix: use bind_to_monitor from config when focusing or moving to a workspace

Author: lars-berger

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 {

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(())

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<Workspace>,
+  ) -> 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()
   }

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), &current_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(())

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<Monitor>,
   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<Monitor>,
   state: &mut WmState,
   config: &UserConfig,
 ) -> anyhow::Result<WorkspaceConfig> {
-  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()
 }

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))
       }

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)?;