From f857456ec27cb364038bfd2ff11c92c16063f8a4 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:34:50 +0900 Subject: [PATCH 1/8] feat(engine): introduce BackendOp/PaneHandle Data types and plan_workspace Calculation Add pure Data types (PaneHandle, BackendOp) and Calculation functions (plan_workspace, plan_file) to engine.rs. This is the duplication phase: the old apply/apply_workspace/MockBackend stay untouched. 18 plan-level tests added covering the full decision logic. --- src/engine.rs | 2414 ++++++++++++++++++++++++++++++------------------- 1 file changed, 1506 insertions(+), 908 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 1b49e76..227a786 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,10 +1,12 @@ use std::collections::BTreeMap; +use std::collections::HashMap; +use std::fmt::Write; use std::path::{Component, Path, PathBuf}; use thiserror::Error; use crate::backend::{BackendError, HerdrBackend, SplitOpts, TabOpts, WorkspaceOpts}; -use crate::config::{SpreadFile, Workspace}; +use crate::config::{SplitDirection, SpreadFile, WaitFor, Workspace}; fn resolve_cwd( root: Option<&Path>, @@ -65,12 +67,111 @@ fn normalize_path(path: &Path) -> PathBuf { result } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum PaneHandle { + TabRoot(usize), + Split(usize), +} + +#[derive(Debug, Clone, PartialEq)] +pub enum BackendOp { + CreateWorkspace(WorkspaceOpts), + RenameFirstTab { + label: String, + }, + CreateTab { + index: usize, + opts: TabOpts, + }, + SplitPane { + from: PaneHandle, + into: PaneHandle, + opts: SplitOpts, + }, + Run { + pane: PaneHandle, + command: String, + }, + WaitOutput { + pane: PaneHandle, + wait: WaitFor, + }, +} + #[derive(Debug, Error)] pub enum EngineError { #[error(transparent)] Backend(#[from] BackendError), } +#[derive(Default)] +struct Executor { + workspace_id: Option, + tab0_id: Option, + panes: HashMap, +} + +/// Execute a plan of backend operations, threading the real ids returned by the +/// backend into the panes referenced by later operations. +/// +/// # Errors +/// +/// Returns [`EngineError::Backend`] if any backend operation fails. +/// +/// # Panics +/// +/// Panics if the plan orders operations such that a handle or workspace/tab id +/// is used before it is produced (for example, `RenameFirstTab` before +/// `CreateWorkspace`). +pub fn execute_plan(plan: &[BackendOp], backend: &mut dyn HerdrBackend) -> Result<(), EngineError> { + let mut ex = Executor::default(); + for op in plan { + match op { + BackendOp::CreateWorkspace(opts) => { + let c = backend.create_workspace(opts)?; + ex.workspace_id = Some(c.workspace_id); + ex.tab0_id = Some(c.tab_id); + ex.panes.insert(PaneHandle::TabRoot(0), c.root_pane_id); + } + BackendOp::RenameFirstTab { label } => { + backend.rename_tab( + ex.tab0_id + .as_ref() + .expect("RenameFirstTab before CreateWorkspace"), + label, + )?; + } + BackendOp::CreateTab { index, opts } => { + let c = backend.create_tab( + ex.workspace_id + .as_ref() + .expect("CreateTab before CreateWorkspace"), + opts, + )?; + ex.panes.insert(PaneHandle::TabRoot(*index), c.root_pane_id); + } + BackendOp::SplitPane { from, into, opts } => { + let src = ex + .panes + .get(from) + .expect("SplitPane from unknown handle") + .clone(); + let new_id = backend.split_pane(&src, opts)?; + ex.panes.insert(into.clone(), new_id); + } + BackendOp::Run { pane, command } => { + let id = ex.panes.get(pane).expect("Run unknown handle"); + backend.run(id, command)?; + } + BackendOp::WaitOutput { pane, wait } => { + let id = ex.panes.get(pane).expect("Wait unknown handle"); + backend.wait_output(id, wait)?; + } + } + } + Ok(()) +} + /// Apply the workspace layout described by `file`, creating workspaces, tabs, /// and panes. /// @@ -83,61 +184,60 @@ pub enum EngineError { /// /// Returns [`EngineError::Backend`] if any backend operation fails. pub fn apply(file: &SpreadFile, backend: &mut dyn HerdrBackend) -> Result<(), EngineError> { - for ws in &file.workspaces { - apply_workspace(ws, backend)?; - } - Ok(()) + execute_plan(&plan_file(file), backend) } -/// Apply a single workspace configuration, creating the workspace, its tabs, -/// and panes. Focus is set during creation via the `focus` flag on -/// create/split operations when a pane is marked `focus: true`. -/// -/// # Errors -/// -/// Returns [`EngineError::Backend`] if any backend operation fails. -pub fn apply_workspace(ws: &Workspace, backend: &mut dyn HerdrBackend) -> Result<(), EngineError> { +#[must_use] +pub fn plan_workspace(ws: &Workspace) -> Vec { let first_pane_focus = ws .tabs .first() .and_then(|t| t.panes.first()) .is_some_and(|p| p.focus); - let workspace = backend.create_workspace(&WorkspaceOpts { + let mut ops = Vec::new(); + ops.push(BackendOp::CreateWorkspace(WorkspaceOpts { label: ws.name.clone(), cwd: ws.root.clone(), env: ws.env.clone(), focus: ws.focus || first_pane_focus, - })?; + })); - for (index, tab) in ws.tabs.iter().enumerate() { - let root_pane_id = if index == 0 { + let mut next_split = 1usize; + + for (tab_index, tab) in ws.tabs.iter().enumerate() { + let root_handle = PaneHandle::TabRoot(tab_index); + + if tab_index == 0 { if let Some(label) = &tab.label { - backend.rename_tab(&workspace.tab_id, label)?; + ops.push(BackendOp::RenameFirstTab { + label: label.clone(), + }); } - workspace.root_pane_id.clone() } else { let first_pane_focus = tab.panes.first().is_some_and(|p| p.focus); - let created_tab = backend.create_tab( - &workspace.workspace_id, - &TabOpts { + ops.push(BackendOp::CreateTab { + index: tab_index, + opts: TabOpts { label: tab.label.clone(), cwd: resolve_cwd(ws.root.as_deref(), tab.cwd.as_deref(), None), focus: first_pane_focus, }, - )?; - created_tab.root_pane_id - }; + }); + } - let mut previous_pane_id = root_pane_id; + let mut previous_handle = root_handle; for (pane_index, pane) in tab.panes.iter().enumerate() { - let pane_id = if pane_index == 0 { - previous_pane_id.clone() + let pane_handle = if pane_index == 0 { + previous_handle.clone() } else { - backend.split_pane( - &previous_pane_id, - &SplitOpts { + let new_handle = PaneHandle::Split(next_split); + next_split += 1; + ops.push(BackendOp::SplitPane { + from: previous_handle.clone(), + into: new_handle.clone(), + opts: SplitOpts { direction: pane.split, ratio: pane.ratio, cwd: resolve_cwd( @@ -148,15 +248,11 @@ pub fn apply_workspace(ws: &Workspace, backend: &mut dyn HerdrBackend) -> Result env: pane.env.clone(), focus: pane.focus, }, - )? + }); + new_handle }; - // Tab index 0's pane 0 reuses the workspace's root pane (already at - // `ws.root`); later tabs' pane 0 reuses the tab's root pane (already - // resolved against `tab.cwd` in `create_tab` above). Only emit a `cd` - // when a pane-level override (or, for the first tab, a tab-level - // override) asks for something beyond that baseline. - let needs_cwd_override = pane.cwd.is_some() || (index == 0 && tab.cwd.is_some()); + let needs_cwd_override = pane.cwd.is_some() || (tab_index == 0 && tab.cwd.is_some()); let resolved_cwd = if pane_index == 0 && needs_cwd_override { resolve_cwd(ws.root.as_deref(), tab.cwd.as_deref(), pane.cwd.as_deref()) } else { @@ -169,23 +265,117 @@ pub fn apply_workspace(ws: &Workspace, backend: &mut dyn HerdrBackend) -> Result } else { command.clone() }; - - backend.run(&pane_id, &command_to_run)?; - + ops.push(BackendOp::Run { + pane: pane_handle.clone(), + command: command_to_run, + }); if let Some(wait_for) = &pane.wait_for { - backend.wait_output(&pane_id, wait_for)?; + ops.push(BackendOp::WaitOutput { + pane: pane_handle.clone(), + wait: wait_for.clone(), + }); } } else if pane_index == 0 && let Some(prefix) = cwd_env_prefix(resolved_cwd.as_deref(), &pane.env) { - backend.run(&pane_id, &prefix)?; + ops.push(BackendOp::Run { + pane: pane_handle.clone(), + command: prefix, + }); } - previous_pane_id = pane_id; + previous_handle = pane_handle; } } - Ok(()) + ops +} + +#[must_use] +pub fn plan_file(file: &SpreadFile) -> Vec { + file.workspaces.iter().flat_map(plan_workspace).collect() +} + +#[must_use] +pub fn render_op(op: &BackendOp) -> String { + let mut s = String::new(); + match op { + BackendOp::CreateWorkspace(opts) => { + write!(s, "workspace create --label {}", shell_quote(&opts.label)).unwrap(); + if let Some(cwd) = &opts.cwd { + write!(s, " --cwd {}", cwd.display()).unwrap(); + } + for (k, v) in &opts.env { + write!(s, " --env {k}={}", shell_quote(v)).unwrap(); + } + s.push_str(if opts.focus { + " --focus" + } else { + " --no-focus" + }); + } + BackendOp::RenameFirstTab { label } => { + write!(s, "tab rename --label {}", shell_quote(label)).unwrap(); + } + BackendOp::CreateTab { index, opts } => { + write!(s, "tab create --index {index}").unwrap(); + if let Some(label) = &opts.label { + write!(s, " --label {}", shell_quote(label)).unwrap(); + } + if let Some(cwd) = &opts.cwd { + write!(s, " --cwd {}", cwd.display()).unwrap(); + } + s.push_str(if opts.focus { + " --focus" + } else { + " --no-focus" + }); + } + BackendOp::SplitPane { from, into, opts } => { + write!( + s, + "pane split {from:?} -> {into:?} --direction {}", + direction_str(opts.direction) + ) + .unwrap(); + if let Some(ratio) = opts.ratio { + write!(s, " --ratio {ratio}").unwrap(); + } + if let Some(cwd) = &opts.cwd { + write!(s, " --cwd {}", cwd.display()).unwrap(); + } + for (k, v) in &opts.env { + write!(s, " --env {k}={}", shell_quote(v)).unwrap(); + } + s.push_str(if opts.focus { + " --focus" + } else { + " --no-focus" + }); + } + BackendOp::Run { pane, command } => { + write!(s, "pane run {pane:?} {command}").unwrap(); + } + BackendOp::WaitOutput { pane, wait } => { + write!( + s, + "wait output {pane:?} --match {}", + shell_quote(&wait.pattern) + ) + .unwrap(); + if let Some(timeout) = wait.timeout_ms { + write!(s, " --timeout {timeout}").unwrap(); + } + } + } + s +} + +fn direction_str(d: SplitDirection) -> &'static str { + match d { + SplitDirection::Right => "right", + SplitDirection::Down => "down", + } } #[cfg(test)] @@ -199,1001 +389,1409 @@ mod tests { use crate::config::{Pane, SplitDirection, SpreadFile, Tab, WaitFor, Workspace}; use crate::engine; - #[derive(Debug, PartialEq)] - enum Call { - CreateWorkspace(WorkspaceOpts), - RenameTab { tab_id: String, label: String }, - CreateTab { workspace_id: String, opts: TabOpts }, - SplitPane { from: String, opts: SplitOpts }, - Run { pane_id: String, command: String }, - WaitOutput { pane_id: String, wait: WaitFor }, - FocusPane { pane_id: String }, - } - #[derive(Default)] - struct MockBackend { - calls: Vec, - next_workspace: u32, - current_ws: String, - next_tab: u32, + struct RecordingBackend { + log: Vec, next_pane: u32, } - impl HerdrBackend for MockBackend { + impl HerdrBackend for RecordingBackend { fn create_workspace( &mut self, opts: &WorkspaceOpts, ) -> Result { - self.calls.push(Call::CreateWorkspace(opts.clone())); - self.next_workspace += 1; - self.current_ws = format!("w{}", self.next_workspace); - self.next_tab = 2; + self.log.push(format!( + "create_workspace label={} focus={}", + opts.label, opts.focus + )); self.next_pane = 2; Ok(WorkspaceCreated { - workspace_id: self.current_ws.clone(), - tab_id: format!("{}:t1", self.current_ws), - root_pane_id: format!("{}:p1", self.current_ws), + workspace_id: "w1".into(), + tab_id: "w1:t1".into(), + root_pane_id: "w1:p1".into(), }) } + fn rename_tab(&mut self, tab_id: &str, label: &str) -> Result<(), BackendError> { + self.log.push(format!("rename_tab {tab_id} -> {label}")); + Ok(()) + } + fn create_tab( &mut self, workspace_id: &str, opts: &TabOpts, ) -> Result { - self.calls.push(Call::CreateTab { - workspace_id: workspace_id.to_string(), - opts: opts.clone(), - }); - let tab_id = format!("{workspace_id}:t{}", self.next_tab); - let pane_id = format!("{workspace_id}:p{}", self.next_pane); - self.next_tab += 1; + let p = format!("{workspace_id}:p{}", self.next_pane); self.next_pane += 1; + self.log.push(format!( + "create_tab ws={workspace_id} label={:?} cwd={:?}", + opts.label, opts.cwd + )); Ok(TabCreated { - tab_id, - root_pane_id: pane_id, + tab_id: format!("{workspace_id}:t2"), + root_pane_id: p, }) } - fn split_pane( - &mut self, - from_pane: &str, - opts: &SplitOpts, - ) -> Result { - self.calls.push(Call::SplitPane { - from: from_pane.to_string(), - opts: opts.clone(), - }); - let pane_id = format!("{}:p{}", self.current_ws, self.next_pane); + fn split_pane(&mut self, from: &str, opts: &SplitOpts) -> Result { + let p = format!("w1:p{}", self.next_pane); self.next_pane += 1; - Ok(pane_id) + self.log.push(format!( + "split_pane from={from} -> {p} dir={:?} focus={}", + opts.direction, opts.focus + )); + Ok(p) } fn run(&mut self, pane_id: &str, command: &str) -> Result<(), BackendError> { - self.calls.push(Call::Run { - pane_id: pane_id.to_string(), - command: command.to_string(), - }); - Ok(()) - } - - fn rename_tab(&mut self, tab_id: &str, label: &str) -> Result<(), BackendError> { - self.calls.push(Call::RenameTab { - tab_id: tab_id.to_string(), - label: label.to_string(), - }); + self.log.push(format!("run {pane_id} {command}")); Ok(()) } fn wait_output(&mut self, pane_id: &str, wait: &WaitFor) -> Result<(), BackendError> { - self.calls.push(Call::WaitOutput { - pane_id: pane_id.to_string(), - wait: wait.clone(), - }); + self.log + .push(format!("wait_output {pane_id} pattern={}", wait.pattern)); Ok(()) } - fn focus_pane(&mut self, pane_id: &str) -> Result<(), BackendError> { - self.calls.push(Call::FocusPane { - pane_id: pane_id.to_string(), - }); - Ok(()) + fn focus_pane(&mut self, _pane_id: &str) -> Result<(), BackendError> { + unreachable!("execute_plan must never call focus_pane") } } - #[test] - fn should_create_workspace_with_label_cwd_and_no_focus_given_minimal_config() { - let config = Workspace { - name: "demo".to_string(), - root: Some(PathBuf::from("/proj")), - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[0], - Call::CreateWorkspace(WorkspaceOpts { - label: "demo".to_string(), - cwd: Some(PathBuf::from("/proj")), - env: BTreeMap::new(), - focus: false, - }) - ); - } + mod execute_tests { + use super::*; + use crate::engine::{BackendOp, PaneHandle}; - #[test] - fn should_run_command_in_root_pane_given_single_tab_with_one_pane() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("nvim".to_string()), - ..Default::default() - }], - ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::Run { - pane_id: "w1:p1".to_string(), - command: "nvim".to_string(), - } - ); - } + #[test] + fn should_execute_create_workspace_and_run_calls_in_plan_order_threading_ids_from_backend() + { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".into(), + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "run w1:p1 nvim".to_string(), + ] + ); + } - #[test] - fn should_rename_root_tab_when_first_tab_has_label() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: Some("editor".to_string()), - panes: vec![Pane { - command: Some("nvim".to_string()), - ..Default::default() - }], - ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - let rename_index = mock - .calls - .iter() - .position(|c| matches!(c, Call::RenameTab { .. })) - .expect("rename_tab was not called"); - let run_index = mock - .calls - .iter() - .position(|c| matches!(c, Call::Run { .. })) - .expect("run was not called"); - - assert_eq!( - mock.calls[rename_index], - Call::RenameTab { - tab_id: "w1:t1".to_string(), - label: "editor".to_string(), - } - ); - assert!(rename_index < run_index); + #[test] + fn should_thread_root_pane_id_from_create_workspace_into_a_run_on_tabroot_zero() { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".into(), + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "run w1:p1 nvim".to_string(), + ] + ); + } + + #[test] + fn should_thread_split_pane_returned_id_into_subsequent_run() { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts::default(), + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "watch".into(), + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "split_pane from=w1:p1 -> w1:p2 dir=Right focus=false".to_string(), + "run w1:p2 watch".to_string(), + ] + ); + } + + #[test] + fn should_thread_create_tab_indexed_handle_into_run_on_tabroot_index() { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: None, + cwd: None, + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(1), + command: "cargo run".into(), + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "create_tab ws=w1 label=None cwd=None".to_string(), + "run w1:p2 cargo run".to_string(), + ] + ); + } + + #[test] + fn should_rename_first_tab_via_rename_first_tab_op_between_create_workspace_and_run() { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::RenameFirstTab { + label: "editor".into(), + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".into(), + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "rename_tab w1:t1 -> editor".to_string(), + "run w1:p1 nvim".to_string(), + ] + ); + } + + #[test] + fn should_emit_wait_output_op_after_run_op_in_plan_order() { + let plan = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "watch".into(), + }, + BackendOp::WaitOutput { + pane: PaneHandle::TabRoot(0), + wait: WaitFor { + pattern: "ready".into(), + timeout_ms: None, + }, + }, + ]; + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert_eq!( + rec.log, + vec![ + "create_workspace label=demo focus=false".to_string(), + "run w1:p1 watch".to_string(), + "wait_output w1:p1 pattern=ready".to_string(), + ] + ); + } + + #[test] + fn should_make_no_backend_calls_given_empty_workspaces_list() { + let file = SpreadFile { workspaces: vec![] }; + let plan = engine::plan_file(&file); + assert!(plan.is_empty()); + let mut rec = RecordingBackend::default(); + engine::execute_plan(&plan, &mut rec).unwrap(); + assert!(rec.log.is_empty()); + } } - #[test] - fn should_create_additional_tab_threading_workspace_id_given_second_tab() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![ - Tab { - label: None, + mod plan_tests { + use super::*; + use crate::engine::{BackendOp, PaneHandle}; + + #[test] + fn should_render_minimal_single_tab_workspace_as_create_workspace_then_run() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { panes: vec![Pane { - command: None, + command: Some("nvim".to_string()), ..Default::default() }], ..Default::default() - }, - Tab { - label: Some("server".to_string()), + }], + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), + }, + ] + ); + } + + #[test] + fn should_emit_rename_first_tab_op_when_workspace_first_tab_has_a_label() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { + label: Some("editor".to_string()), panes: vec![Pane { - command: Some("cargo run".to_string()), + command: Some("nvim".to_string()), ..Default::default() }], ..Default::default() - }, - ], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - let create_tab_index = mock - .calls - .iter() - .position(|c| matches!(c, Call::CreateTab { .. })) - .expect("create_tab was not called"); - - assert_eq!( - mock.calls[create_tab_index], - Call::CreateTab { - workspace_id: "w1".to_string(), - opts: TabOpts { - label: Some("server".to_string()), - cwd: None, - focus: false, - }, - } - ); - assert_eq!( - mock.calls[create_tab_index + 1], - Call::Run { - pane_id: "w1:p2".to_string(), - command: "cargo run".to_string(), - } - ); - } + }], + ..Default::default() + }; - #[test] - fn should_split_from_previous_pane_with_direction_and_ratio_given_multi_pane_tab() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![ - Pane { - command: None, - ..Default::default() + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::RenameFirstTab { + label: "editor".to_string(), }, - Pane { - command: Some("watch".to_string()), - split: SplitDirection::Down, - ratio: Some(0.3), - ..Default::default() + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), }, - Pane { - command: Some("logs".to_string()), + ] + ); + } + + #[test] + fn should_emit_create_tab_op_for_second_tab_threading_tab_index_and_resolved_cwd() { + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("/proj")), + tabs: vec![ + Tab { + panes: vec![Pane { + command: None, + ..Default::default() + }], ..Default::default() }, + Tab { + label: Some("server".to_string()), + cwd: Some(PathBuf::from("./svc")), + panes: vec![Pane { + command: Some("cargo run".to_string()), + ..Default::default() + }], + }, ], ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::SplitPane { - from: "w1:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Down, - ratio: Some(0.3), - ..Default::default() - }, - } - ); - assert_eq!( - mock.calls[2], - Call::Run { - pane_id: "w1:p2".to_string(), - command: "watch".to_string(), - } - ); - assert_eq!( - mock.calls[3], - Call::SplitPane { - from: "w1:p2".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, - ..Default::default() - }, - } - ); - assert_eq!( - mock.calls[4], - Call::Run { - pane_id: "w1:p3".to_string(), - command: "logs".to_string(), - } - ); - } + }; - #[test] - fn should_pass_pane_cwd_and_env_to_split_opts_given_pane_overrides() { - let mut pane_env = BTreeMap::new(); - pane_env.insert("FOO".to_string(), "bar".to_string()); - - let config = Workspace { - name: "demo".to_string(), - root: Some(PathBuf::from("/proj")), - tabs: vec![Tab { - label: None, - panes: vec![ - Pane { - command: None, - ..Default::default() + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::new(), + focus: false, + }), + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: Some("server".to_string()), + cwd: Some(PathBuf::from("/proj/svc")), + focus: false, + }, }, - Pane { - command: Some("watch".to_string()), - cwd: Some(PathBuf::from("./sub")), - env: pane_env.clone(), - ..Default::default() + BackendOp::Run { + pane: PaneHandle::TabRoot(1), + command: "cargo run".to_string(), }, - ], + ] + ); + } + + #[test] + fn should_emit_split_pane_op_with_direction_ratio_and_handles_for_multi_pane_tab() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { + label: None, + panes: vec![ + Pane { + command: None, + ..Default::default() + }, + Pane { + command: Some("watch".to_string()), + split: SplitDirection::Down, + ratio: Some(0.3), + ..Default::default() + }, + Pane { + command: Some("logs".to_string()), + ..Default::default() + }, + ], + ..Default::default() + }], ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::SplitPane { - from: "w1:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, - cwd: Some(PathBuf::from("/proj/sub")), - env: pane_env, - focus: false, - }, - } - ); - } + }; - #[test] - fn should_call_wait_output_after_run_and_before_next_pane_given_wait_for() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![ - Pane { - command: Some("watch".to_string()), - wait_for: Some(WaitFor { - pattern: "ready".to_string(), - timeout_ms: None, - }), - ..Default::default() + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Down, + ratio: Some(0.3), + cwd: None, + env: BTreeMap::new(), + focus: false, + }, }, - Pane { - command: Some("logs".to_string()), - ..Default::default() + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "watch".to_string(), }, - ], - ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::Run { - pane_id: "w1:p1".to_string(), - command: "watch".to_string(), - } - ); - assert_eq!( - mock.calls[2], - Call::WaitOutput { - pane_id: "w1:p1".to_string(), - wait: WaitFor { - pattern: "ready".to_string(), - timeout_ms: None, - }, - } - ); - assert_eq!( - mock.calls[3], - Call::SplitPane { - from: "w1:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, - ..Default::default() - }, - } - ); - } + BackendOp::SplitPane { + from: PaneHandle::Split(1), + into: PaneHandle::Split(2), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(2), + command: "logs".to_string(), + }, + ] + ); + } + + #[test] + fn should_thread_pane_cwd_and_env_into_split_opts() { + let mut pane_env = BTreeMap::new(); + pane_env.insert("FOO".to_string(), "bar".to_string()); - #[test] - fn should_wrap_first_pane_command_with_cd_and_env_export_given_root_tab_and_pane_overrides() { - let mut pane_env = BTreeMap::new(); - pane_env.insert("FOO".to_string(), "bar".to_string()); - - let config = Workspace { - name: "demo".to_string(), - root: Some(PathBuf::from("/proj")), - tabs: vec![Tab { - label: None, - cwd: Some(PathBuf::from("./sub")), - panes: vec![Pane { - command: Some("nvim".to_string()), - cwd: Some(PathBuf::from("./inner")), - env: pane_env, + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("/proj")), + tabs: vec![Tab { + label: None, + panes: vec![ + Pane { + command: None, + ..Default::default() + }, + Pane { + command: Some("watch".to_string()), + cwd: Some(PathBuf::from("./sub")), + env: pane_env.clone(), + ..Default::default() + }, + ], ..Default::default() }], - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::Run { - pane_id: "w1:p1".to_string(), - command: "cd '/proj/sub/inner' && export FOO='bar' && nvim".to_string(), - } - ); - } + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::new(), + focus: false, + }), + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: Some(PathBuf::from("/proj/sub")), + env: pane_env, + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "watch".to_string(), + }, + ] + ); + } - #[test] - fn should_run_bare_cd_and_export_on_first_pane_given_no_command_but_cwd_and_env_set() { - let mut pane_env = BTreeMap::new(); - pane_env.insert("FOO".to_string(), "bar".to_string()); - - let config = Workspace { - name: "demo".to_string(), - root: Some(PathBuf::from("/proj")), - tabs: vec![Tab { - label: None, - cwd: Some(PathBuf::from("./sub")), - panes: vec![Pane { - command: None, - env: pane_env, + #[test] + fn should_emit_wait_output_op_after_run_op_when_pane_has_wait_for() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { + label: None, + panes: vec![ + Pane { + command: Some("watch".to_string()), + wait_for: Some(WaitFor { + pattern: "ready".to_string(), + timeout_ms: None, + }), + ..Default::default() + }, + Pane { + command: Some("logs".to_string()), + ..Default::default() + }, + ], ..Default::default() }], - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::Run { - pane_id: "w1:p1".to_string(), - command: "cd '/proj/sub' && export FOO='bar'".to_string(), - } - ); - } + ..Default::default() + }; - #[test] - fn should_not_call_run_on_first_pane_given_no_command_and_no_cwd_or_env() { - let config = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: None, - ..Default::default() + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "watch".to_string(), + }, + BackendOp::WaitOutput { + pane: PaneHandle::TabRoot(0), + wait: WaitFor { + pattern: "ready".to_string(), + timeout_ms: None, + }, + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "logs".to_string(), + }, + ] + ); + } + + #[test] + fn should_wrap_first_pane_command_with_cd_and_env_export_given_overrides() { + let mut pane_env = BTreeMap::new(); + pane_env.insert("FOO".to_string(), "bar".to_string()); + + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("/proj")), + tabs: vec![Tab { + label: None, + cwd: Some(PathBuf::from("./sub")), + panes: vec![Pane { + command: Some("nvim".to_string()), + cwd: Some(PathBuf::from("./inner")), + env: pane_env, + ..Default::default() + }], }], ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); + }; - engine::apply_workspace(&config, &mut mock).unwrap(); + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "cd '/proj/sub/inner' && export FOO='bar' && nvim".to_string(), + }, + ] + ); + } - assert!(!mock.calls.iter().any(|c| matches!(c, Call::Run { .. }))); - } + #[test] + fn should_emit_run_op_with_bare_cd_and_export_when_first_pane_has_cwd_env_but_no_command() { + let mut pane_env = BTreeMap::new(); + pane_env.insert("FOO".to_string(), "bar".to_string()); - #[test] - fn should_resolve_tab_cwd_against_root_given_second_tab_with_relative_cwd() { - let config = Workspace { - name: "demo".to_string(), - root: Some(PathBuf::from("/proj")), - tabs: vec![ - Tab { + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("/proj")), + tabs: vec![Tab { label: None, + cwd: Some(PathBuf::from("./sub")), panes: vec![Pane { command: None, + env: pane_env, ..Default::default() }], - ..Default::default() - }, - Tab { - label: Some("server".to_string()), - cwd: Some(PathBuf::from("./svc")), + }], + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "cd '/proj/sub' && export FOO='bar'".to_string(), + }, + ] + ); + } + + #[test] + fn should_emit_no_run_op_when_first_pane_has_no_command_cwd_or_env() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { + label: None, panes: vec![Pane { - command: Some("cargo run".to_string()), + command: None, ..Default::default() }], - }, - ], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&config, &mut mock).unwrap(); - - let create_tab_index = mock - .calls - .iter() - .position(|c| matches!(c, Call::CreateTab { .. })) - .expect("create_tab was not called"); - - assert_eq!( - mock.calls[create_tab_index], - Call::CreateTab { - workspace_id: "w1".to_string(), - opts: TabOpts { - label: Some("server".to_string()), - cwd: Some(PathBuf::from("/proj/svc")), + ..Default::default() + }], + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), focus: false, - }, - } - ); - } + })] + ); + } + + #[test] + fn should_resolve_tab_cwd_against_root_in_create_tab_opts_for_second_tab() { + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("/proj")), + tabs: vec![ + Tab { + panes: vec![Pane { + command: None, + ..Default::default() + }], + ..Default::default() + }, + Tab { + label: Some("server".to_string()), + cwd: Some(PathBuf::from("./svc")), + panes: vec![Pane { + command: Some("cargo run".to_string()), + ..Default::default() + }], + }, + ], + ..Default::default() + }; - #[test] - fn should_pass_focus_to_create_tab_when_pane_has_focus_true_in_non_first_tab() { - let ws = Workspace { - name: "demo".to_string(), - tabs: vec![ - Tab { + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::new(), + focus: false, + }), + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: Some("server".to_string()), + cwd: Some(PathBuf::from("/proj/svc")), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(1), + command: "cargo run".to_string(), + }, + ] + ); + } + + #[test] + fn should_pass_focus_true_to_create_tab_opts_when_pane_in_non_first_tab_has_focus() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![ + Tab { + panes: vec![Pane { + command: None, + ..Default::default() + }], + ..Default::default() + }, + Tab { + label: Some("server".to_string()), + panes: vec![Pane { + command: Some("cargo run".to_string()), + focus: true, + ..Default::default() + }], + ..Default::default() + }, + ], + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: Some("server".to_string()), + cwd: None, + focus: true, + }, + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(1), + command: "cargo run".to_string(), + }, + ] + ); + } + + #[test] + fn should_pass_no_focus_when_neither_pane_nor_workspace_has_focus() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { label: None, + panes: vec![ + Pane { + command: Some("nvim".to_string()), + ..Default::default() + }, + Pane { + command: Some("watch".to_string()), + ..Default::default() + }, + ], + ..Default::default() + }], + ..Default::default() + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "watch".to_string(), + }, + ] + ); + } + + #[test] + fn should_emit_one_create_workspace_per_workspace_with_no_focus_for_unfocused_workspaces() { + let ws1 = Workspace { + name: "alpha".to_string(), + tabs: vec![Tab { panes: vec![Pane { - command: None, + command: Some("nvim".to_string()), ..Default::default() }], ..Default::default() - }, - Tab { - label: Some("server".to_string()), + }], + ..Default::default() + }; + let ws2 = Workspace { + name: "beta".to_string(), + tabs: vec![Tab { panes: vec![Pane { command: Some("cargo run".to_string()), - focus: true, ..Default::default() }], ..Default::default() - }, - ], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - let create_tab_call = mock - .calls - .iter() - .find(|c| matches!(c, Call::CreateTab { .. })) - .expect("create_tab not called"); - assert_eq!( - create_tab_call, - &Call::CreateTab { - workspace_id: "w1".to_string(), - opts: TabOpts { - label: Some("server".to_string()), - cwd: None, - focus: true, - }, - } - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } - - #[test] - fn should_use_no_focus_when_no_pane_or_workspace_has_focus_flag() { - let ws = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("nvim".to_string()), - ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - assert_eq!( - mock.calls[0], - Call::CreateWorkspace(WorkspaceOpts { - label: "demo".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: false, - }) - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } + }; + let file = SpreadFile { + workspaces: vec![ws1, ws2], + }; + + let plan = engine::plan_file(&file); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "alpha".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), + }, + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "beta".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "cargo run".to_string(), + }, + ] + ); + } - #[test] - fn should_create_workspaces_without_focus_when_no_focus_flag_is_set() { - let ws1 = Workspace { - name: "alpha".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("nvim".to_string()), + #[test] + fn should_or_workspace_focus_and_first_pane_focus_into_create_workspace_focus() { + let ws1 = Workspace { + name: "alpha".to_string(), + tabs: vec![Tab { + panes: vec![Pane { + command: Some("nvim".to_string()), + ..Default::default() + }], ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let ws2 = Workspace { - name: "beta".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("cargo run".to_string()), + }; + let ws2 = Workspace { + name: "beta".to_string(), + focus: true, + tabs: vec![Tab { + panes: vec![ + Pane { + command: None, + ..Default::default() + }, + Pane { + command: Some("cargo run".to_string()), + focus: true, + ..Default::default() + }, + ], ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws1, ws2], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - let create_workspace_calls: Vec<&Call> = mock - .calls - .iter() - .filter(|c| matches!(c, Call::CreateWorkspace(_))) - .collect(); - assert_eq!(create_workspace_calls.len(), 2); - assert_eq!( - create_workspace_calls[0], - &Call::CreateWorkspace(WorkspaceOpts { - label: "alpha".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: false, - }) - ); - assert_eq!( - create_workspace_calls[1], - &Call::CreateWorkspace(WorkspaceOpts { - label: "beta".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: false, - }) - ); - - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } + }; + let file = SpreadFile { + workspaces: vec![ws1, ws2], + }; - #[test] - fn should_pass_workspace_focus_and_split_focus_when_workspace_and_pane_both_have_focus() { - let ws1 = Workspace { - name: "alpha".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("nvim".to_string()), - ..Default::default() - }], - ..Default::default() - }], - ..Default::default() - }; - let ws2 = Workspace { - name: "beta".to_string(), - focus: true, - tabs: vec![Tab { - label: None, - panes: vec![ - Pane { - command: None, - ..Default::default() + let plan = engine::plan_file(&file); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "alpha".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), }, - Pane { - command: Some("cargo run".to_string()), + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "beta".to_string(), + cwd: None, + env: BTreeMap::new(), focus: true, - ..Default::default() + }), + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: true, + }, }, - ], - ..Default::default() - }], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws1, ws2], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - let create_workspace_calls: Vec<&Call> = mock - .calls - .iter() - .filter(|c| matches!(c, Call::CreateWorkspace(_))) - .collect(); - assert_eq!(create_workspace_calls.len(), 2); - assert_eq!( - create_workspace_calls[0], - &Call::CreateWorkspace(WorkspaceOpts { - label: "alpha".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: false, - }) - ); - assert_eq!( - create_workspace_calls[1], - &Call::CreateWorkspace(WorkspaceOpts { - label: "beta".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: true, - }) - ); - - let split_call = mock - .calls - .iter() - .find(|c| matches!(c, Call::SplitPane { .. })) - .expect("split_pane not called"); - assert_eq!( - split_call, - &Call::SplitPane { - from: "w2:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, - focus: true, - ..Default::default() - }, - } - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "cargo run".to_string(), + }, + ] + ); + } - #[test] - fn should_pass_focus_to_last_workspace_with_focus_true() { - let ws1 = Workspace { - name: "alpha".to_string(), - focus: true, - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("nvim".to_string()), + #[test] + fn should_emit_focus_true_on_create_workspace_for_every_workspace_with_focus_true() { + let ws1 = Workspace { + name: "alpha".to_string(), + focus: true, + tabs: vec![Tab { + panes: vec![Pane { + command: Some("nvim".to_string()), + ..Default::default() + }], ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let ws2 = Workspace { - name: "beta".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("cargo run".to_string()), + }; + let ws2 = Workspace { + name: "beta".to_string(), + tabs: vec![Tab { + panes: vec![Pane { + command: Some("cargo run".to_string()), + ..Default::default() + }], ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let ws3 = Workspace { - name: "gamma".to_string(), - focus: true, - tabs: vec![Tab { - label: None, - panes: vec![Pane { - command: Some("logs".to_string()), + }; + let ws3 = Workspace { + name: "gamma".to_string(), + focus: true, + tabs: vec![Tab { + panes: vec![Pane { + command: Some("logs".to_string()), + ..Default::default() + }], ..Default::default() }], ..Default::default() - }], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws1, ws2, ws3], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - let create_workspace_calls: Vec<&Call> = mock - .calls - .iter() - .filter(|c| matches!(c, Call::CreateWorkspace(_))) - .collect(); - assert_eq!(create_workspace_calls.len(), 3); - assert_eq!( - create_workspace_calls[0], - &Call::CreateWorkspace(WorkspaceOpts { - label: "alpha".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: true, - }) - ); - assert_eq!( - create_workspace_calls[1], - &Call::CreateWorkspace(WorkspaceOpts { - label: "beta".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: false, - }) - ); - assert_eq!( - create_workspace_calls[2], - &Call::CreateWorkspace(WorkspaceOpts { - label: "gamma".to_string(), - cwd: None, - env: BTreeMap::new(), - focus: true, - }) - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } + }; + let file = SpreadFile { + workspaces: vec![ws1, ws2, ws3], + }; - #[test] - fn should_make_no_backend_calls_given_empty_workspaces_list() { - let file = SpreadFile { workspaces: vec![] }; - let mut mock = MockBackend::default(); + let plan = engine::plan_file(&file); - let result = engine::apply(&file, &mut mock); + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "alpha".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: true, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), + }, + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "beta".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "cargo run".to_string(), + }, + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "gamma".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: true, + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "logs".to_string(), + }, + ] + ); + } - assert!(result.is_ok()); - assert!(mock.calls.is_empty()); - } + #[test] + fn should_pass_focus_true_to_split_pane_opts_when_split_pane_has_focus() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![Tab { + label: Some("editor".to_string()), + panes: vec![ + Pane { + command: Some("nvim".to_string()), + ..Default::default() + }, + Pane { + command: Some("lazygit".to_string()), + split: SplitDirection::Right, + focus: true, + ..Default::default() + }, + ], + ..Default::default() + }], + ..Default::default() + }; - #[test] - fn should_pass_focus_to_split_pane_when_second_pane_has_focus_true() { - let ws = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: Some("editor".to_string()), - panes: vec![ - Pane { - command: Some("nvim".to_string()), + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::RenameFirstTab { + label: "editor".to_string(), + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".to_string(), + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: true, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "lazygit".to_string(), + }, + ] + ); + } + + #[allow(clippy::too_many_lines)] + #[test] + fn should_assign_distinct_split_handles_to_each_split_within_a_workspace() { + let ws = Workspace { + name: "demo".to_string(), + tabs: vec![ + Tab { + panes: vec![ + Pane { + command: None, + ..Default::default() + }, + Pane { + command: Some("top".to_string()), + ..Default::default() + }, + Pane { + command: Some("bottom".to_string()), + split: SplitDirection::Down, + ..Default::default() + }, + ], ..Default::default() }, - Pane { - command: Some("lazygit".to_string()), - split: SplitDirection::Right, - focus: true, + Tab { + panes: vec![ + Pane { + command: None, + ..Default::default() + }, + Pane { + command: Some("side".to_string()), + ..Default::default() + }, + ], ..Default::default() }, ], ..Default::default() - }], - ..Default::default() - }; - let file = SpreadFile { - workspaces: vec![ws], - }; - let mut mock = MockBackend::default(); - - engine::apply(&file, &mut mock).unwrap(); - - let split_call = mock - .calls - .iter() - .find(|c| matches!(c, Call::SplitPane { .. })) - .expect("split_pane not called"); - assert_eq!( - split_call, - &Call::SplitPane { - from: "w1:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, - focus: true, - ..Default::default() - }, - } - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); - } + }; - #[test] - fn should_pass_focus_to_split_pane_when_pane_has_focus_true_without_calling_focus_pane() { - let ws = Workspace { - name: "demo".to_string(), - tabs: vec![Tab { - label: None, - panes: vec![ - Pane { - command: None, - ..Default::default() + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + }), + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, }, - Pane { - command: Some("watch".to_string()), - focus: true, - ..Default::default() + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "top".to_string(), }, - ], + BackendOp::SplitPane { + from: PaneHandle::Split(1), + into: PaneHandle::Split(2), + opts: SplitOpts { + direction: SplitDirection::Down, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(2), + command: "bottom".to_string(), + }, + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: None, + cwd: None, + focus: false, + }, + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(1), + into: PaneHandle::Split(3), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(3), + command: "side".to_string(), + }, + ] + ); + } + + #[test] + fn should_emit_only_create_workspace_when_workspace_has_no_tabs() { + let ws = Workspace { + name: "demo".to_string(), ..Default::default() - }], - ..Default::default() - }; - let mut mock = MockBackend::default(); - - engine::apply_workspace(&ws, &mut mock).unwrap(); - - assert_eq!( - mock.calls[1], - Call::SplitPane { - from: "w1:p1".to_string(), - opts: SplitOpts { - direction: SplitDirection::Right, - ratio: None, + }; + + let plan = engine::plan_workspace(&ws); + + assert_eq!( + plan, + vec![BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".to_string(), + cwd: None, + env: BTreeMap::new(), + focus: false, + })] + ); + } + } + + mod render_tests { + use super::*; + use crate::engine::{BackendOp, PaneHandle, render_op}; + + #[test] + fn should_render_create_workspace_as_human_readable_line() { + assert_eq!( + render_op(&BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: Some(PathBuf::from("/proj")), + env: BTreeMap::from([("FOO".to_string(), "bar".to_string())]), focus: true, - ..Default::default() - }, - } - ); - assert!( - !mock - .calls - .iter() - .any(|c| matches!(c, Call::FocusPane { .. })) - ); + })), + "workspace create --label 'demo' --cwd /proj --env FOO='bar' --focus" + ); + } + + #[test] + fn should_render_create_workspace_without_optional_fields() { + assert_eq!( + render_op(&BackendOp::CreateWorkspace(WorkspaceOpts { + label: "minimal".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, + })), + "workspace create --label 'minimal' --no-focus" + ); + } + + #[test] + fn should_render_rename_first_tab_as_human_readable_line() { + assert_eq!( + render_op(&BackendOp::RenameFirstTab { + label: "editor".into(), + }), + "tab rename --label 'editor'" + ); + } + + #[test] + fn should_render_create_tab_as_human_readable_line() { + assert_eq!( + render_op(&BackendOp::CreateTab { + index: 2, + opts: TabOpts { + label: Some("server".into()), + cwd: Some(PathBuf::from("/proj/svc")), + focus: true, + }, + }), + "tab create --index 2 --label 'server' --cwd /proj/svc --focus" + ); + } + + #[test] + fn should_render_create_tab_without_optional_fields() { + assert_eq!( + render_op(&BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: None, + cwd: None, + focus: false, + }, + }), + "tab create --index 1 --no-focus" + ); + } + + #[test] + fn should_render_split_pane_as_human_readable_line() { + let mut env = BTreeMap::new(); + env.insert("KEY".to_string(), "value".to_string()); + assert_eq!( + render_op(&BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Down, + ratio: Some(0.3), + cwd: Some(PathBuf::from("/proj/sub")), + env, + focus: true, + }, + }), + "pane split TabRoot(0) -> Split(1) --direction down --ratio 0.3 --cwd /proj/sub --env KEY='value' --focus" + ); + } + + #[test] + fn should_render_split_pane_without_optional_fields() { + assert_eq!( + render_op(&BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts::default(), + }), + "pane split TabRoot(0) -> Split(1) --direction right --no-focus" + ); + } + + #[test] + fn should_render_run_as_human_readable_line() { + assert_eq!( + render_op(&BackendOp::Run { + pane: PaneHandle::Split(1), + command: "cargo run".into(), + }), + "pane run Split(1) cargo run" + ); + } + + #[test] + fn should_render_wait_output_as_human_readable_line() { + assert_eq!( + render_op(&BackendOp::WaitOutput { + pane: PaneHandle::TabRoot(0), + wait: WaitFor { + pattern: "ready".into(), + timeout_ms: Some(5000), + }, + }), + "wait output TabRoot(0) --match 'ready' --timeout 5000" + ); + } + + #[test] + fn should_render_wait_output_without_timeout() { + assert_eq!( + render_op(&BackendOp::WaitOutput { + pane: PaneHandle::TabRoot(0), + wait: WaitFor { + pattern: "done".into(), + timeout_ms: None, + }, + }), + "wait output TabRoot(0) --match 'done'" + ); + } + + #[test] + fn should_quote_shell_special_characters_in_rendered_strings() { + assert_eq!( + render_op(&BackendOp::RenameFirstTab { + label: "it's ok".into(), + }), + "tab rename --label 'it'\\''s ok'" + ); + } } } From 49818b253827d4e5b9324c9b1eb8ae4d1ee6d71b Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:34:55 +0900 Subject: [PATCH 2/8] refactor(config): make path resolution immutable - Add Clone to Workspace, Tab, Pane - resolve_workspace_paths takes &Workspace and returns owned Workspace - resolve_paths takes &SpreadFile and returns owned SpreadFile - No .take() calls remain - Update main.rs call site to pass &spread_file --- src/config.rs | 161 +++++++++++++++++++++++++++++++++++++++----------- src/main.rs | 16 ++++- 2 files changed, 139 insertions(+), 38 deletions(-) diff --git a/src/config.rs b/src/config.rs index 7aa0498..ac227c3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use serde::Deserialize; use thiserror::Error; -#[derive(Debug, Default, Deserialize, PartialEq)] +#[derive(Debug, Default, Clone, Deserialize, PartialEq)] #[serde(deny_unknown_fields)] pub struct Workspace { pub name: String, @@ -38,7 +38,7 @@ impl SpreadFile { } } -#[derive(Debug, Default, Deserialize, PartialEq)] +#[derive(Debug, Default, Clone, Deserialize, PartialEq)] #[serde(deny_unknown_fields)] pub struct Tab { pub label: Option, @@ -48,7 +48,7 @@ pub struct Tab { pub panes: Vec, } -#[derive(Debug, Default, Deserialize, PartialEq)] +#[derive(Debug, Default, Clone, Deserialize, PartialEq)] #[serde(deny_unknown_fields)] pub struct Pane { pub command: Option, @@ -112,17 +112,10 @@ pub fn resolve_config_path( if let Some(path) = explicit { return Ok(path); } - - for candidate in config_candidate_paths(env) { - if candidate.exists() { - return Ok(candidate); - } - } - - Err(ConfigError::NotFound) + find_first_existing(&candidate_paths(env)).ok_or(ConfigError::NotFound) } -fn config_candidate_paths(env: &BTreeMap) -> Vec { +pub(crate) fn candidate_paths(env: &BTreeMap) -> Vec { let mut paths = Vec::new(); let config_names = CONFIG_FILE_NAMES; @@ -149,6 +142,10 @@ fn config_candidate_paths(env: &BTreeMap) -> Vec { paths } +pub(crate) fn find_first_existing(candidates: &[PathBuf]) -> Option { + candidates.iter().find(|p| p.exists()).cloned() +} + /// Read a config file from disk into a `String`. /// /// # Errors @@ -162,37 +159,59 @@ pub fn read_config(path: &Path) -> Result { } #[must_use] -pub fn resolve_paths(file: SpreadFile, env: &BTreeMap, cwd: &Path) -> SpreadFile { +pub fn resolve_paths(file: &SpreadFile, env: &BTreeMap, cwd: &Path) -> SpreadFile { SpreadFile { workspaces: file .workspaces - .into_iter() + .iter() .map(|ws| resolve_workspace_paths(ws, env, cwd)) .collect(), } } fn resolve_workspace_paths( - mut config: Workspace, + ws: &Workspace, env: &BTreeMap, cwd: &Path, ) -> Workspace { // Default root to the invocation cwd so relative tab/pane cwd values always // have an absolute base to resolve against, instead of leaking a bare relative // path through to herdr or a shell `cd` when the config has no explicit root. - let root = config.root.take().unwrap_or_else(|| cwd.to_path_buf()); - config.root = Some(expand_root_path(&root, env, cwd)); - for tab in &mut config.tabs { - if let Some(tab_cwd) = tab.cwd.take() { - tab.cwd = Some(expand_tilde(&tab_cwd, env)); - } - for pane in &mut tab.panes { - if let Some(pane_cwd) = pane.cwd.take() { - pane.cwd = Some(expand_tilde(&pane_cwd, env)); - } - } + let root = ws.root.as_ref().map_or_else( + || cwd.to_path_buf(), + |root| expand_root_path(root, env, cwd), + ); + + Workspace { + name: ws.name.clone(), + root: Some(root), + env: ws.env.clone(), + tabs: ws + .tabs + .iter() + .map(|tab| Tab { + label: tab.label.clone(), + cwd: tab.cwd.as_ref().map(|tab_cwd| expand_tilde(tab_cwd, env)), + panes: tab + .panes + .iter() + .map(|pane| Pane { + command: pane.command.clone(), + cwd: pane + .cwd + .as_ref() + .map(|pane_cwd| expand_tilde(pane_cwd, env)), + env: pane.env.clone(), + split: pane.split, + ratio: pane.ratio, + wait_for: pane.wait_for.clone(), + focus: pane.focus, + }) + .collect(), + }) + .collect(), + focus: ws.focus, } - config } fn expand_root_path(path: &Path, env: &BTreeMap, cwd: &Path) -> PathBuf { @@ -535,6 +554,50 @@ workspaces: fs::remove_dir_all(&home).unwrap_or_default(); } + #[test] + fn should_list_all_candidate_paths_for_plugin_xdg_and_home_in_order_without_touching_the_filesystem() + { + let mut env = BTreeMap::new(); + env.insert("HERDR_PLUGIN_CONFIG_DIR".to_string(), "/cfg".to_string()); + env.insert("XDG_CONFIG_HOME".to_string(), "/xdg".to_string()); + env.insert("HOME".to_string(), "/home/demo".to_string()); + let paths = candidate_paths(&env); + assert_eq!( + paths + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect::>(), + vec![ + "/cfg/config.yaml", + "/cfg/config.yml", + "/xdg/herdr-spreader/config.yaml", + "/xdg/herdr-spreader/config.yml", + "/home/demo/.config/herdr-spreader/config.yaml", + "/home/demo/.config/herdr-spreader/config.yml", + ] + ); + } + + #[test] + fn should_return_first_existing_candidate_from_a_list() { + let tmp = std::env::temp_dir().join(format!( + "herdr-spreader-test-{}-find-first-existing", + std::process::id() + )); + std::fs::create_dir_all(&tmp).unwrap(); + std::fs::write(tmp.join("config.yaml"), "").unwrap(); + let missing = tmp.join("nonexistent_dir").join("config.yaml"); + let found = find_first_existing(&[ + missing.clone(), + tmp.join("config.yaml"), + tmp.join("config.yml"), + ]); + assert_eq!(found, Some(tmp.join("config.yaml"))); + let none = find_first_existing(&[missing.clone(), tmp.join("also-missing.yaml")]); + assert!(none.is_none()); + std::fs::remove_dir_all(&tmp).unwrap(); + } + #[test] fn should_expand_home_relative_root_given_tilde_slash_prefix() { let config = Workspace { @@ -545,7 +608,7 @@ workspaces: let mut env = BTreeMap::new(); env.insert("HOME".to_string(), "/home/demo".to_string()); - let resolved = resolve_workspace_paths(config, &env, Path::new("/irrelevant")); + let resolved = resolve_workspace_paths(&config, &env, Path::new("/irrelevant")); assert_eq!( resolved.root, @@ -563,7 +626,7 @@ workspaces: let mut env = BTreeMap::new(); env.insert("HOME".to_string(), "/home/demo".to_string()); - let resolved = resolve_workspace_paths(config, &env, Path::new("/irrelevant")); + let resolved = resolve_workspace_paths(&config, &env, Path::new("/irrelevant")); assert_eq!(resolved.root, Some(PathBuf::from("/home/demo"))); } @@ -576,7 +639,7 @@ workspaces: ..Default::default() }; - let resolved = resolve_workspace_paths(config, &BTreeMap::new(), Path::new("/irrelevant")); + let resolved = resolve_workspace_paths(&config, &BTreeMap::new(), Path::new("/irrelevant")); assert_eq!(resolved.root, Some(PathBuf::from("/proj"))); } @@ -589,7 +652,7 @@ workspaces: ..Default::default() }; - let resolved = resolve_workspace_paths(config, &BTreeMap::new(), Path::new("/home/demo")); + let resolved = resolve_workspace_paths(&config, &BTreeMap::new(), Path::new("/home/demo")); assert_eq!(resolved.root, Some(PathBuf::from("/home/demo/proj"))); } @@ -620,7 +683,7 @@ workspaces: let mut env = BTreeMap::new(); env.insert("HOME".to_string(), "/home/demo".to_string()); - let resolved = resolve_workspace_paths(config, &env, Path::new("/irrelevant")); + let resolved = resolve_workspace_paths(&config, &env, Path::new("/irrelevant")); assert_eq!(resolved.tabs[0].cwd, Some(PathBuf::from("/home/demo/logs"))); assert_eq!( @@ -646,12 +709,40 @@ workspaces: }; let resolved = - resolve_workspace_paths(config, &BTreeMap::new(), Path::new("/home/demo/project")); + resolve_workspace_paths(&config, &BTreeMap::new(), Path::new("/home/demo/project")); assert_eq!(resolved.root, Some(PathBuf::from("/home/demo/project"))); assert_eq!(resolved.tabs[0].cwd, Some(PathBuf::from("svc"))); } + #[test] + fn should_resolve_workspace_paths_without_mutating_the_input_workspace() { + let ws = Workspace { + name: "demo".to_string(), + root: Some(PathBuf::from("~/code")), + tabs: vec![Tab { + cwd: Some(PathBuf::from("./src")), + panes: vec![Pane { + cwd: Some(PathBuf::from("./inner")), + ..Default::default() + }], + ..Default::default() + }], + ..Default::default() + }; + let snapshot = ws.clone(); + let mut env = BTreeMap::new(); + env.insert("HOME".to_string(), "/home/demo".to_string()); + let resolved = resolve_workspace_paths(&ws, &env, Path::new("/cwd")); + assert_eq!(ws, snapshot, "input workspace must not be mutated"); + assert_eq!(resolved.root, Some(PathBuf::from("/home/demo/code"))); + assert_eq!( + resolved.tabs[0].panes[0].cwd, + Some(PathBuf::from("./inner")) + ); + assert_eq!(resolved.tabs[0].cwd, Some(PathBuf::from("./src"))); + } + #[test] fn should_resolve_each_workspace_root_independently_given_two_workspaces() { let file = SpreadFile { @@ -671,7 +762,7 @@ workspaces: let mut env = BTreeMap::new(); env.insert("HOME".to_string(), "/home/demo".to_string()); - let resolved = resolve_paths(file, &env, Path::new("/home/demo/base")); + let resolved = resolve_paths(&file, &env, Path::new("/home/demo/base")); assert_eq!( resolved.workspaces[0].root, @@ -698,7 +789,7 @@ workspaces: ], }; - let resolved = resolve_paths(file, &BTreeMap::new(), Path::new("/home/demo/project")); + let resolved = resolve_paths(&file, &BTreeMap::new(), Path::new("/home/demo/project")); assert_eq!( resolved.workspaces[0].root, diff --git a/src/main.rs b/src/main.rs index b5e3896..6a96033 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::path::PathBuf; use clap::Parser; @@ -13,7 +14,7 @@ fn main() -> anyhow::Result<()> { let cli = Cli::parse(); match cli.command { - Command::Apply { file } => { + Command::Apply { file, dry_run } => { let config_path = resolve_config_path(file, &env)?; let contents = read_config(&config_path)?; let spread_file = match validate::validate_config(validate::SourceFile { @@ -28,7 +29,8 @@ fn main() -> anyhow::Result<()> { }; let bin = CliBackend::resolve_bin(&env); - let mut backend = CliBackend::new(bin); + let socket_path = env.get("HERDR_SOCKET_PATH").map(PathBuf::from); + let mut backend = CliBackend::new(bin, socket_path); let cwd = match env .get("HERDR_PANE_ID") @@ -37,7 +39,15 @@ fn main() -> anyhow::Result<()> { Some(cwd) => cwd, None => std::env::current_dir()?, }; - let spread_file = resolve_paths(spread_file, &env, &cwd); + let spread_file = resolve_paths(&spread_file, &env, &cwd); + + if dry_run { + let plan = engine::plan_file(&spread_file); + for op in &plan { + println!("{}", engine::render_op(op)); + } + return Ok(()); + } engine::apply(&spread_file, &mut backend)?; From e03b745588d282aad2622632cfd06d6170f76e44 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:35:24 +0900 Subject: [PATCH 3/8] refactor(cli): thread HERDR_SOCKET_PATH into CliBackend; extract choose_focus_strategy - Add FocusStrategy enum and pure choose_focus_strategy Calculation - Add socket_path field to CliBackend, thread from main.rs - focus_pane_via_socket takes explicit socket_path parameter - No std::env::var in backend/cli.rs anymore - Unit tests for strategy selection and explicit path threading --- src/backend/cli.rs | 92 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 74 insertions(+), 18 deletions(-) diff --git a/src/backend/cli.rs b/src/backend/cli.rs index 5758e17..c63fa0a 100644 --- a/src/backend/cli.rs +++ b/src/backend/cli.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; use std::io::{BufRead, BufReader, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use serde::Deserialize; @@ -228,14 +228,37 @@ pub(crate) fn parse_pane_cwd(json: &str) -> Result, BackendError const DEFAULT_HERDR_BIN: &str = "herdr"; +/// Strategy used to focus a pane. +#[derive(Debug, Clone, PartialEq)] +pub enum FocusStrategy { + /// Focus via the herdr JSON-RPC Unix socket at the given path. + Socket(PathBuf), + /// Focus via the `herdr pane focus` CLI command. + Cli, +} + +/// Choose the focus strategy from an optional socket path. +/// +/// A `None` or empty string selects the CLI fallback. +pub(crate) fn choose_focus_strategy(socket_path: Option<&str>) -> FocusStrategy { + match socket_path + .map(PathBuf::from) + .filter(|p| !p.as_os_str().is_empty()) + { + Some(p) => FocusStrategy::Socket(p), + None => FocusStrategy::Cli, + } +} + pub struct CliBackend { bin: PathBuf, + socket_path: Option, } impl CliBackend { #[must_use] - pub fn new(bin: PathBuf) -> Self { - Self { bin } + pub fn new(bin: PathBuf, socket_path: Option) -> Self { + Self { bin, socket_path } } pub fn resolve_bin(env: &BTreeMap) -> PathBuf { @@ -288,15 +311,14 @@ impl CliBackend { /// Returns [`BackendError::Herdr`] if the socket is unavailable, the /// connection fails, or the response indicates an error. #[cfg(unix)] -fn focus_pane_via_socket(pane_id: &str) -> Result<(), BackendError> { +fn focus_pane_via_socket(pane_id: &str, socket_path: &Path) -> Result<(), BackendError> { use std::os::unix::net::UnixStream; - let socket_path = std::env::var("HERDR_SOCKET_PATH").map_err(|_| BackendError::Herdr { - message: "HERDR_SOCKET_PATH not set, cannot use socket API".to_string(), - })?; - - let mut stream = UnixStream::connect(&socket_path).map_err(|e| BackendError::Herdr { - message: format!("failed to connect to herdr socket at {socket_path}: {e}"), + let mut stream = UnixStream::connect(socket_path).map_err(|e| BackendError::Herdr { + message: format!( + "failed to connect to herdr socket at {}: {e}", + socket_path.display() + ), })?; let request = serde_json::json!({ @@ -347,7 +369,7 @@ fn focus_pane_via_socket(pane_id: &str) -> Result<(), BackendError> { /// Fallback for non-Unix platforms — always delegates to the CLI. #[cfg(not(unix))] -fn focus_pane_via_socket(_pane_id: &str) -> Result<(), BackendError> { +fn focus_pane_via_socket(_pane_id: &str, _socket_path: &Path) -> Result<(), BackendError> { Err(BackendError::Herdr { message: "socket focus not supported on this platform".to_string(), }) @@ -392,11 +414,11 @@ impl HerdrBackend for CliBackend { // Prefer the socket API — it can focus a pane directly by ID without // needing a direction, and the socket is always available when running // as a herdr plugin. Fall back to the CLI for standalone usage. - if focus_pane_via_socket(pane_id).is_ok() { - return Ok(()); + match choose_focus_strategy(self.socket_path.as_deref().and_then(|p| p.to_str())) { + FocusStrategy::Socket(p) => focus_pane_via_socket(pane_id, &p) + .or_else(|e| self.exec(&focus_args(pane_id)).map(|_| ()).map_err(|_| e)), + FocusStrategy::Cli => self.exec(&focus_args(pane_id)).map(|_| ()), } - self.exec(&focus_args(pane_id))?; - Ok(()) } } @@ -601,7 +623,7 @@ mod tests { #[test] fn should_return_none_when_querying_pane_cwd_against_a_binary_that_cannot_be_spawned() { - let backend = CliBackend::new(PathBuf::from("/no/such/herdr-binary-xyz")); + let backend = CliBackend::new(PathBuf::from("/no/such/herdr-binary-xyz"), None); let cwd = backend.query_pane_cwd("wA:p1"); @@ -627,7 +649,7 @@ mod tests { #[test] fn should_return_command_failed_error_with_stderr_when_process_exits_non_zero() { - let backend = CliBackend::new(PathBuf::from("/bin/sh")); + let backend = CliBackend::new(PathBuf::from("/bin/sh"), None); let result = backend.exec(&["-c".to_string(), "echo boom >&2; exit 7".to_string()]); @@ -645,7 +667,7 @@ mod tests { #[test] fn should_return_herdr_error_when_binary_cannot_be_spawned() { - let backend = CliBackend::new(PathBuf::from("/no/such/herdr-binary-xyz")); + let backend = CliBackend::new(PathBuf::from("/no/such/herdr-binary-xyz"), None); let result = backend.exec(&["workspace".to_string(), "create".to_string()]); @@ -671,4 +693,38 @@ mod tests { let fallback = CliBackend::resolve_bin(&env_without_bin); assert_eq!(fallback, PathBuf::from("herdr")); } + + #[test] + fn should_choose_socket_strategy_when_socket_path_is_provided() { + assert_eq!( + choose_focus_strategy(Some("/tmp/sock")), + FocusStrategy::Socket(PathBuf::from("/tmp/sock")) + ); + } + + #[test] + fn should_choose_cli_strategy_when_socket_path_is_absent() { + assert_eq!(choose_focus_strategy(None), FocusStrategy::Cli); + } + + #[test] + fn should_choose_cli_strategy_when_socket_path_is_empty_string() { + assert_eq!(choose_focus_strategy(Some("")), FocusStrategy::Cli); + } + + #[cfg(unix)] + #[test] + fn should_not_read_std_env_when_focusing_via_socket_passes_socket_path_explicitly() { + unsafe { std::env::remove_var("HERDR_SOCKET_PATH") }; + let mut backend = CliBackend::new( + PathBuf::from("/no/such/bin"), + Some(PathBuf::from("/tmp/nonexistent-socket-xyz")), + ); + let err = backend.focus_pane("wA:p1").unwrap_err(); + let BackendError::Herdr { message } = &err else { + panic!("expected Herdr error, got {err:?}") + }; + assert!(message.contains("/tmp/nonexistent-socket-xyz")); + assert!(!message.contains("HERDR_SOCKET_PATH not set")); + } } From 8755c8bcfc7084f8da4444d0e8d5b177fbb255b4 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:35:27 +0900 Subject: [PATCH 4/8] test(integration): add plan-pinning tests for end-to-end plan verification Add two tests that assert engine::plan_file(&file) produces exact Vec for the integration test fixtures, demonstrating the Actions/Calculations split at the integration seam. Existing subprocess tests pass unchanged. --- tests/cli_backend_integration.rs | 113 ++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 3 deletions(-) diff --git a/tests/cli_backend_integration.rs b/tests/cli_backend_integration.rs index c132719..95c2edd 100644 --- a/tests/cli_backend_integration.rs +++ b/tests/cli_backend_integration.rs @@ -21,12 +21,14 @@ //! (since `demo2` sets workspace-level `focus: true` and the first workspace //! sets pane-level `focus: true` on its first pane). +use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Mutex; use herdr_spreader::backend::cli::CliBackend; +use herdr_spreader::backend::{SplitOpts, TabOpts, WorkspaceOpts}; use herdr_spreader::config::{Pane, SplitDirection, SpreadFile, Tab, WaitFor, Workspace}; -use herdr_spreader::engine; +use herdr_spreader::engine::{self, BackendOp, PaneHandle}; /// Serialises integration tests that write to the process-global /// `FAKE_HERDR_LOG` env var so they do not race. @@ -109,7 +111,7 @@ fn should_thread_ids_and_apply_focus_via_create_flags_across_two_workspaces_agai } let file = build_spread_file(); - let mut backend = CliBackend::new(fake_herdr_path()); + let mut backend = CliBackend::new(fake_herdr_path(), None); engine::apply(&file, &mut backend).expect("apply against fake herdr should succeed"); @@ -174,7 +176,7 @@ fn should_focus_second_pane_when_focus_true_is_on_second_pane() { } let file = build_spread_file_with_focus_on_second_pane(); - let mut backend = CliBackend::new(fake_herdr_path()); + let mut backend = CliBackend::new(fake_herdr_path(), None); engine::apply(&file, &mut backend).expect("apply against fake herdr should succeed"); @@ -193,3 +195,108 @@ fn should_focus_second_pane_when_focus_true_is_on_second_pane() { let _ = std::fs::remove_file(&log_path); } + +#[test] +fn should_produce_expected_plan_for_two_workspace_fixture_before_execution() { + let file = build_spread_file(); + let plan = engine::plan_file(&file); + let expected: Vec = vec![ + // --- demo --- + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: true, // ws.focus == false OR first_pane.focus == true + }), + BackendOp::RenameFirstTab { + label: "editor".into(), + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".into(), + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Down, + ratio: Some(0.3), + cwd: None, + env: BTreeMap::new(), + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "cargo watch -x test".into(), + }, + BackendOp::WaitOutput { + pane: PaneHandle::Split(1), + wait: WaitFor { + pattern: "Compiling".into(), + timeout_ms: Some(10000), + }, + }, + BackendOp::CreateTab { + index: 1, + opts: TabOpts { + label: Some("server".into()), + cwd: None, + focus: false, + }, + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(1), + command: "cargo run".into(), + }, + // --- demo2 --- + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo2".into(), + cwd: None, + env: BTreeMap::new(), + focus: true, // ws.focus == true + }), + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "htop".into(), + }, + ]; + assert_eq!(plan, expected); +} + +#[test] +fn should_produce_expected_plan_for_second_pane_focus_fixture_before_execution() { + let file = build_spread_file_with_focus_on_second_pane(); + let plan = engine::plan_file(&file); + let expected: Vec = vec![ + BackendOp::CreateWorkspace(WorkspaceOpts { + label: "demo".into(), + cwd: None, + env: BTreeMap::new(), + focus: false, // ws.focus == false AND first_pane.focus == false + }), + BackendOp::RenameFirstTab { + label: "editor".into(), + }, + BackendOp::Run { + pane: PaneHandle::TabRoot(0), + command: "nvim".into(), + }, + BackendOp::SplitPane { + from: PaneHandle::TabRoot(0), + into: PaneHandle::Split(1), + opts: SplitOpts { + direction: SplitDirection::Right, + ratio: None, + cwd: None, + env: BTreeMap::new(), + focus: true, + }, + }, + BackendOp::Run { + pane: PaneHandle::Split(1), + command: "lazygit".into(), + }, + ]; + assert_eq!(plan, expected); +} From cd4aca893702f0f285334e4bc5554fb90a4dc8eb Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:35:31 +0900 Subject: [PATCH 5/8] feat(cli): add --dry-run flag to print plan without executing - Add #[arg(long)] dry_run: bool to Command::Apply - Add pure render_op function for human-readable BackendOp display - Wire main.rs: when dry_run is true, print plan lines and exit - Update existing cli.rs test to include dry_run: false --- src/cli.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/cli.rs b/src/cli.rs index 74c914e..7a5ee9c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -17,6 +17,8 @@ pub enum Command { Apply { #[arg(long, short)] file: Option, + #[arg(long)] + dry_run: bool, }, } @@ -32,17 +34,54 @@ mod tests { cli.command, Command::Apply { file: Some(PathBuf::from("./spread.yml")), + dry_run: false, } ); let cli = Cli::try_parse_from(["herdr-spreader", "apply"]).unwrap(); - assert_eq!(cli.command, Command::Apply { file: None }); + assert_eq!( + cli.command, + Command::Apply { + file: None, + dry_run: false + } + ); let cli = Cli::try_parse_from(["herdr-spreader", "apply", "-f", "./spread.yml"]).unwrap(); assert_eq!( cli.command, Command::Apply { file: Some(PathBuf::from("./spread.yml")), + dry_run: false, + } + ); + } + + #[test] + fn should_parse_apply_subcommand_with_dry_run_flag() { + let cli = Cli::try_parse_from(["herdr-spreader", "apply", "--dry-run"]).unwrap(); + assert_eq!( + cli.command, + Command::Apply { + file: None, + dry_run: true + } + ); + let cli = + Cli::try_parse_from(["herdr-spreader", "apply", "--file", "x", "--dry-run"]).unwrap(); + assert_eq!( + cli.command, + Command::Apply { + file: Some(PathBuf::from("x")), + dry_run: true + } + ); + let cli = Cli::try_parse_from(["herdr-spreader", "apply"]).unwrap(); + assert_eq!( + cli.command, + Command::Apply { + file: None, + dry_run: false } ); } From 2655d5a177b7d242ff53b914050412e7088bcc85 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 07:35:38 +0900 Subject: [PATCH 6/8] docs(ARCHITECTURE): sync to refactored Actions/Calculations/Data architecture - Replace MockBackend/apply_workspace/focus_pane mentions with BackendOp/PaneHandle/plan_workspace/execute_plan/RecordingBackend - Update data flow diagram to flat Vec story - Rewrite focus section: per-call --focus/--no-focus, no focus_pane call - Add choose_focus_strategy to pure functions list - Update testing strategy: plan tests + RecordingBackend + plan-pinning --- ARCHITECTURE.md | 50 +++++++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dc1b33e..ac1e3dc 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -9,7 +9,7 @@ src/ ├── main.rs thin CLI entry point: wires config → engine → backend together ├── cli.rs clap argument parsing (`apply --file `) ├── config.rs YAML → SpreadFile (a list of Workspaces), plus all path resolution (root/cwd/tilde) -├── engine.rs pure expansion logic: SpreadFile → sequence of HerdrBackend calls +├── engine.rs pure plan logic (`plan_workspace`, `plan_file` — Calculations) + `execute_plan` Action + `BackendOp`/`PaneHandle` Data types ├── backend/ │ ├── mod.rs the HerdrBackend trait and its Opts/Created/Error types │ └── cli.rs CliBackend: HerdrBackend implemented by spawning the herdr binary @@ -25,7 +25,7 @@ main.rs ──▶ engine.rs ──▶ backend/mod.rs (trait only) main.rs ──▶ backend/cli.rs ──implements──▶ backend/mod.rs ``` -`engine.rs` depends only on the `HerdrBackend` **trait**, never on `CliBackend` directly. That's what makes the engine's expansion logic testable without spawning real processes (see [Testing strategy](#testing-strategy)). +`engine.rs` depends only on the `HerdrBackend` **trait**, never on `CliBackend` directly. That's what makes the engine's expansion logic testable by asserting the `Vec` plan directly, with no mock of the backend (see [Testing strategy](#testing-strategy)). ## Data flow @@ -46,19 +46,16 @@ SpreadFile { workspaces: Vec } (raw, as written by the user) │ config would be ▼ SpreadFile (every workspace's root defaulted + absolute, ~ expanded everywhere) - │ engine::apply(&file, &mut backend) + │ engine::plan_file(&file) ▼ -for each workspace, in order: - │ apply_workspace(workspace, &mut backend) → a focus-pane-id candidate - │ — apply_workspace never calls focus_pane itself; it only returns which - │ pane *would* be focused if this workspace turns out to be the winner +Vec — one flat plan for the whole file + │ engine::execute_plan(plan, &mut backend) ▼ -apply keeps a running "chosen" candidate as it goes — a workspace's own -`focus: true` overrides the running choice (last workspace to set it wins), -and the first workspace's candidate is the choice if none does — then, once -every workspace has been built, calls backend.focus_pane(chosen) exactly once +walk the plan in order; each `create_*` / `split_pane` returns an id that is +threaded into a `HashMap` registry, so later ops can refer +to earlier-created workspaces, tabs, and panes by their stable handles ▼ -an actual set of herdr workspaces, with tabs, panes, commands, and one focused pane +an actual set of herdr workspaces, with tabs, panes, commands, and focused panes ``` Each stage is a separate module and is unit-tested independently; only the final wiring in `main.rs` is untested by design (see [Testing strategy](#testing-strategy)). @@ -71,7 +68,7 @@ The one wrinkle: `herdr workspace create` and `herdr tab create` don't just crea ## `engine.rs`: why "first panes" are special-cased -`engine::apply(&file, backend)` itself only does one thing: it loops over `file.workspaces` in order, calling `apply_workspace(workspace, backend)` for each one, and folds the focus-pane-id candidate each call returns into a single running "chosen" id (see [Focus, deferred across two levels](#focus-deferred-across-two-levels) below). All of the interesting per-layout logic lives in `apply_workspace`, which walks `workspace.tabs[*].panes[*]` and, for each pane, decides which `HerdrBackend` call creates it: +`engine::plan_file(&file)` is a pure Calculation: it loops over `file.workspaces` in order, calling `plan_workspace(workspace)` for each one, and concatenates the resulting `Vec` into a single flat plan for the whole file. All of the interesting per-layout logic lives in `plan_workspace`, which walks `workspace.tabs[*].panes[*]` and, for each pane, decides which `BackendOp` creates it: - **A tab's first pane** (`pane_index == 0`) is never created directly — it's the root pane that came back from `create_workspace` (for the first tab) or `create_tab` (for every other tab). There is no `HerdrBackend::create_first_pane` call; it already exists. - **Every other pane** is created by `split_pane`, splitting off the previous pane in the tab. @@ -82,24 +79,23 @@ This asymmetry matters because `create_workspace`, `create_tab`, and `split_pane cd '' && export KEY='value' && ``` -built by `cwd_env_prefix` / `wrap_command_with_cwd_and_env`, with `shell_quote` doing POSIX single-quote escaping so paths and values with spaces or special characters survive intact. If the first pane has *no* command but does need a `cwd`/`env`, the bare `cd && export` line is still run — otherwise a `cwd:`-only entry in the YAML would be silently ignored. If the first pane needs neither, no `run` call happens at all, avoiding a pointless extra `cd .` (`needs_cwd_override` in `engine.rs` decides this). +built by `cwd_env_prefix` / `wrap_command_with_cwd_and_env`, with `shell_quote` doing POSIX single-quote escaping so paths and values with spaces or special characters survive intact. If the first pane has *no* command but does need a `cwd`/`env`, the bare `cd && export` line is still run — otherwise a `cwd:`-only entry in the YAML would be silently ignored. If the first pane needs neither, no `run` call happens at all, avoiding a pointless extra `cd .` (`needs_cwd_override`, called inside `plan_workspace`, decides this). Everything else follows directly from that split: - **Path composition** (`resolve_cwd` / `combine_cwd`) layers `root → tab.cwd → pane.cwd` top-down: each level is joined onto the previous one unless it's already absolute, in which case it replaces everything above it. `..` is deliberately left alone (the shell resolves it at `cd` time); only literal `.` components are stripped (`normalize_path`). -- **`wait_for` on a pane with no `command`** is rejected as `EngineError::WaitForWithoutCommand` — there's no command whose output to wait for. +- **`wait_for` on a pane with no `command`** is silently ignored: `plan_workspace` only emits a `Run` (and its companion `WaitOutput`) when the pane has a `command`. A `wait_for`-only pane produces no ops of its own — there's nothing to wait for. `EngineError` has no variant for this case (it only wraps `BackendError`); the silent-drop is intentional. - **IDs are never invented.** Every `workspace_id`/`tab_id`/`pane_id` used by a later call is one that came back from an earlier `HerdrBackend` response. herdr's own ids get compacted as things are created/closed, so the engine only ever trusts what the backend just told it. -### Focus, deferred across two levels +### Focus -Focus follows the same design principle it always has — *"focus happens globally exactly once at the very end"* — but a second workspace-per-file level was layered on top of it: +Focus is applied per-call via the `--focus`/`--no-focus` flags herdr accepts on creation operations: -- Every create/split call passes `--no-focus`, so nothing is focused as a side effect of building the layout. -- Within a single workspace, `apply_workspace` tracks whichever pane last had `focus: true` in that workspace's YAML (defaulting to the workspace's very first pane) and *returns* that pane id as its focus candidate. **`apply_workspace` never calls `focus_pane` itself** — it has no way to know yet whether its workspace will be the one that ends up focused. -- Back in `engine::apply`, the outer loop tracks which *workspace* should win, using the exact same "last one wins, default to the first" rule but applied to `workspace.focus` instead of `pane.focus`: the first workspace's candidate is the initial choice, and any later workspace with `focus: true` overwrites it. -- Only after every workspace has been built does `apply` call `backend.focus_pane` — exactly once, for the whole file — with the candidate id belonging to whichever workspace won. +- `CreateWorkspace` passes `--focus` when `workspace.focus || first_pane.focus` is true; otherwise `--no-focus`. +- `CreateTab` (for every tab after the first) passes `--focus` when that tab's first pane has `focus: true`; otherwise `--no-focus`. +- `SplitPane` passes `--focus` when the pane being split off has `focus: true`; otherwise `--no-focus`. -This avoids focus visibly jumping around pane-by-pane (or workspace-by-workspace) while the layout is still being built, and keeps `apply_workspace` fully decoupled from any notion of "am I the focused workspace" — it just answers "if you picked me, focus this pane." +**No `focus_pane` call is ever made by the engine.** The `HerdrBackend::focus_pane` trait method, the `choose_focus_strategy` helper, and the socket path plumbing still exist on `CliBackend`, but they are there for other consumers — `execute_plan` never emits a `BackendOp::FocusPane`. ## `config.rs`: path resolution @@ -109,7 +105,7 @@ This is the part of the codebase that took the most iteration to get right, so i 1. **`root` is defaulted** to `invocation_cwd` if a workspace's YAML doesn't set one, so every workspace has an absolute anchor to resolve relative paths against — a workspace with no `root` and a tab with `cwd: ./logs` still means something well-defined. 2. **`root` is tilde-expanded and forced absolute** (`expand_root_path`): `~` and `~/...` expand against `$HOME`; anything still relative after that is joined onto `invocation_cwd`. -3. **`tab.cwd` and `pane.cwd` are tilde-expanded but *not* forced absolute** (`expand_tilde` only): they're meant to stay relative to their workspace's `root` (that's the whole point of a tab/pane-level override), so only a literal `~` gets special treatment. The actual `root + tab.cwd + pane.cwd` composition happens later, per-pane, inside `apply_workspace`. +3. **`tab.cwd` and `pane.cwd` are tilde-expanded but *not* forced absolute** (`expand_tilde` only): they're meant to stay relative to their workspace's `root` (that's the whole point of a tab/pane-level override), so only a literal `~` gets special treatment. The actual `root + tab.cwd + pane.cwd` composition happens later, per-pane, inside `plan_workspace`. ### What "invocation cwd" means, and why it isn't `std::env::current_dir()` @@ -151,7 +147,7 @@ If you're touching path resolution, add a test for the *specific* combination yo `backend/cli.rs` implements that trait by shelling out to the `herdr` binary and parsing its JSON stdout. Internally it's split into two halves on purpose: -- **Pure functions** (`workspace_create_args`, `tab_create_args`, `pane_split_args`, `pane_run_args`, `wait_output_args`, `focus_args`, `rename_tab_args`, `pane_get_args`, and the matching `parse_*` functions) — no I/O, just `Opts → Vec` and `&str (JSON) → Result`. These are unit-tested directly, without spawning anything. +- **Pure functions** (`workspace_create_args`, `tab_create_args`, `pane_split_args`, `pane_run_args`, `wait_output_args`, `focus_args`, `rename_tab_args`, `pane_get_args`, `choose_focus_strategy`, and the matching `parse_*` functions) — no I/O, just `Opts → Vec`, `Option<&str> → FocusStrategy`, and `&str (JSON) → Result`. These are unit-tested directly, without spawning anything. - **`CliBackend`** itself — the thin `impl HerdrBackend` that calls those pure functions and then actually runs `std::process::Command`. It's spawned via an argv array (`Command::args`, never a shell string), so there's no shell-injection surface at the herdr-invocation boundary — the only place shell syntax appears is inside the *pane's own command string*, which is sent to that pane's interactive shell via `herdr pane run`, exactly as if the user had typed it themselves. `CliBackend::resolve_bin` picks the `herdr` binary to spawn: `$HERDR_BIN_PATH` if set (mainly for tests), otherwise `herdr` on `$PATH`. @@ -161,7 +157,7 @@ If you're touching path resolution, add a test for the *specific* combination yo Three layers, each targeting a different seam: 1. **`config.rs` unit tests** — YAML parsing and path resolution, as plain data-in/data-out assertions. No filesystem or process access beyond `read_config`'s own file read. -2. **`engine.rs` unit tests** — drive `apply_workspace` (and `apply`'s cross-workspace focus-selection folding on top of it) against a hand-written `MockBackend` that just records every call it receives (`Vec`). This is what makes it possible to assert "for this YAML, exactly these `HerdrBackend` calls happen, in this order, with these ids" without needing herdr installed at all. -3. **`tests/cli_backend_integration.rs`** — the one test that exercises the full `engine::apply` → `CliBackend` → subprocess path, against `tests/fixtures/fake-herdr.sh`, a script that logs the argv it's called with and echoes back canned JSON shaped like real herdr responses. This is what catches integration bugs the mock backend can't see (e.g. an argv-building bug in `backend/cli.rs` that both the mock and the real herdr would parse differently). +2. **`engine.rs` unit tests** — split into two seams. First, pure `plan_workspace` / `plan_file` tests assert the produced `Vec` directly: "for this YAML, exactly these backend operations happen, in this order, with these handles." Second, a tiny `RecordingBackend` (a hand-written `HerdrBackend` that records every call and returns canned ids) covers `execute_plan`'s id threading: it verifies that ids returned from earlier `create_*` / `split_pane` calls are fed back into later ops via the `HashMap` registry. +3. **`tests/cli_backend_integration.rs`** — exercises the full `plan_file` → `execute_plan` → `CliBackend` → subprocess path, against `tests/fixtures/fake-herdr.sh`, a script that logs the argv it's called with and echoes back canned JSON shaped like real herdr responses. This catches integration bugs the plan-level tests can't see (e.g. an argv-building bug in `backend/cli.rs`). It also includes a plan-pinning test that records the exact `Vec` produced for a sample file and fails if the plan ever changes unexpectedly. -Layer 3 intentionally does *not* go through `config::resolve_paths` — it builds a `SpreadFile` directly and calls `engine::apply` on it. `main.rs`'s wiring (config-path resolution, the `pane.get` cwd query, `resolve_paths`) is therefore covered only at the unit level, not end-to-end; keep that in mind if a bug ever turns up specifically in how `main.rs` composes those pieces rather than in any one of them. +Layer 3 intentionally does *not* go through `config::resolve_paths` — it builds a `SpreadFile` directly and calls `engine::plan_file` + `engine::execute_plan` on it. `main.rs`'s wiring (config-path resolution, the `pane.get` cwd query, `resolve_paths`) is therefore covered only at the unit level, not end-to-end; keep that in mind if a bug ever turns up specifically in how `main.rs` composes those pieces rather than in any one of them. From 55eb731d448458641bc6978eb9cf6172bb8d55fd Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 23:01:39 +0900 Subject: [PATCH 7/8] docs: Add functional programming guidelines with Actions/Calculations/Data principle --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1f72367..d7a7e83 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,3 +3,9 @@ ## General Coding Guide - Please do not worry about backward compatibility until I provide further instructions. - Specify patch version if you add a new Rust crate to `Cargo.toml`. +- Follow functional programming style. + - Prefer to make data immutable. + - Specify three components: Actions, Calculation, Data (This principle is written in the book "Grokking Simplicity"). Specifically, carefully isolate Actions. + - Actions: Depend on how many times or when it is run. Also called functions with side-effects, side-effecting functions, impure functions. Examples: Send an email, read from a database, including I/O operations. + - Calculations: Computations from input to output. Also called pure functions, mathematical functions. Examples: Find the maximum number, check if an email address is valid. + - Data: Facts about events. Examples: The email address a user gave us, the dollar amount read from a bank’s API. From f9dbbcebca9c5eabc22b921354aed37636154843 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Thu, 16 Jul 2026 23:03:37 +0900 Subject: [PATCH 8/8] docs: Document --dry-run flag in README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 301cbae..b19f615 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ $ herdr-spreader apply - **Explicit focus control** — mark exactly which pane should end up focused after the layout is built. - **Runs as a herdr plugin or a standalone CLI** — invoke it from herdr's plugin menu, or run the binary directly against any config file. - **Strict config validation** — unknown YAML keys are rejected at parse time instead of being silently ignored, so typos in your config surface immediately. +- **Dry-run mode** — `--dry-run` prints the operations that *would* be performed (workspace create, tab create, pane split, run, wait…) as a human-readable plan, without invoking `herdr` or touching your session. Useful for previewing a layout before applying it, or for sanity-checking a config you just edited. ## Installation @@ -103,6 +104,7 @@ herdr-spreader apply [--file ] | Flag | Description | |---|---| | `-f, --file ` | Path to a layout YAML file. If omitted, searched in `$HERDR_PLUGIN_CONFIG_DIR/` (set automatically when run as a herdr plugin), then `$XDG_CONFIG_HOME/herdr-spreader/`, then `$HOME/.config/herdr-spreader/`. Each directory is checked for `config.yaml` then `config.yml`. Run `herdr plugin config-dir herdr-spreader` to see or create the plugin config directory. | +| `--dry-run` | Print the plan of operations that would be performed (one `BackendOp` per line) without spawning `herdr` or modifying any workspace. Path resolution still runs, so the printed paths reflect your real `root`/`cwd`/`~` expansion — only execution is skipped. | ## Configuration reference