From 97e5493842397fde286c056f1aa3d46b35a8cb23 Mon Sep 17 00:00:00 2001 From: Aaron Allen Date: Sat, 29 Aug 2026 00:15:47 -0500 Subject: [PATCH] fix(cli): add --no-wait to open `doing open` runs the editor and waits for it to exit. An editor that opens its own window never does, so the terminal stayed unusable until the user interrupted it. Add `process::spawn`, which starts the editor and returns, and reach it from `doing open --no-wait`. The default still waits, because a terminal editor needs the terminal to itself. Detach the child's streams when spawning. Inherited, they would keep our pipes open after we exit, so anything reading `doing`'s output would wait for the editor regardless. Closes #435 --- src/cli/commands/open.rs | 13 ++++ src/cli/process.rs | 58 ++++++++++++++++- tests/integration/commands/open.rs | 1 + .../commands/open/when_no_wait_flag.rs | 62 +++++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/integration/commands/open/when_no_wait_flag.rs diff --git a/src/cli/commands/open.rs b/src/cli/commands/open.rs index 75e5ead..6b7c50d 100644 --- a/src/cli/commands/open.rs +++ b/src/cli/commands/open.rs @@ -10,6 +10,10 @@ use crate::{Error, Result, cli::AppContext}; /// `-b` to specify a macOS bundle identifier, or `-e` to override the /// editor. Use `--backup` to open the most recent backup file instead. /// +/// `doing open` waits for the editor to exit, because a terminal editor +/// needs the terminal to itself. Pass `--no-wait` for an editor that opens +/// its own window and would otherwise hold the shell. +/// /// # Examples /// /// ```text @@ -18,6 +22,7 @@ use crate::{Error, Result, cli::AppContext}; /// doing open -e vim # open doing file in vim /// doing open -b com.apple.TextEdit # open with macOS bundle id /// doing open --backup # open the most recent backup +/// doing open --no-wait # do not wait for the editor to close /// ``` #[derive(Args, Clone, Debug)] pub struct Command { @@ -36,6 +41,10 @@ pub struct Command { /// Override the default editor #[arg(short = 'e', long = "editor")] editor: Option, + + /// Return immediately instead of waiting for the editor to exit + #[arg(long = "no-wait")] + no_wait: bool, } impl Command { @@ -56,6 +65,10 @@ impl Command { let editor = resolve_open_editor(&self.app, &self.editor, ctx); + if self.no_wait { + return crate::cli::process::spawn(&editor, Some(&file_path)); + } + crate::cli::process::launch(&editor, Some(&file_path)) } } diff --git a/src/cli/process.rs b/src/cli/process.rs index 1e2c4b5..85a6263 100644 --- a/src/cli/process.rs +++ b/src/cli/process.rs @@ -1,4 +1,7 @@ -use std::{path::Path, process::Command}; +use std::{ + path::Path, + process::{Command, Stdio}, +}; use doing_config::Config; @@ -80,6 +83,36 @@ pub fn resolve_pager(config: &Config) -> String { "less -FRX".into() } +/// Parse a shell-style command string and start it without waiting for it to exit. +/// +/// Behaves like [`launch`] except that it returns as soon as the child starts. +/// Use it where `doing` has no work left once the program is running, and never +/// for a terminal program that needs the terminal to itself. +/// +/// The child's streams are detached from ours. Were they inherited, the child +/// would hold our pipes open after we exit, and anything reading `doing`'s +/// output would wait for the editor after all. +pub fn spawn(command: &str, file: Option<&Path>) -> Result<()> { + let parts: Vec<&str> = command.split_whitespace().collect(); + let (cmd, args) = parts + .split_first() + .ok_or_else(|| Error::Config("command must not be empty".into()))?; + + let mut process = Command::new(cmd); + process.args(args); + if let Some(path) = file { + process.arg(path); + } + + process + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn()?; + + Ok(()) +} + #[cfg(test)] mod test { use doing_config::{Config, EditorsConfig}; @@ -217,4 +250,27 @@ mod test { assert_eq!(pager, "bat"); } } + mod spawn { + use super::*; + + #[test] + fn it_returns_error_for_empty_command() { + let result = spawn("", None); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("must not be empty")); + } + + #[test] + fn it_returns_without_waiting_for_the_child_to_exit() { + let start = std::time::Instant::now(); + + spawn("sleep 5", None).expect("expected sleep to start"); + + assert!( + start.elapsed() < std::time::Duration::from_secs(2), + "expected spawn to return before the child exited" + ); + } + } } diff --git a/tests/integration/commands/open.rs b/tests/integration/commands/open.rs index 80501aa..328b605 100644 --- a/tests/integration/commands/open.rs +++ b/tests/integration/commands/open.rs @@ -2,3 +2,4 @@ mod when_app_flag; mod when_bundle_id_flag; mod when_editor_flag; mod when_no_flags; +mod when_no_wait_flag; diff --git a/tests/integration/commands/open/when_no_wait_flag.rs b/tests/integration/commands/open/when_no_wait_flag.rs new file mode 100644 index 0000000..26c3069 --- /dev/null +++ b/tests/integration/commands/open/when_no_wait_flag.rs @@ -0,0 +1,62 @@ +use std::{ + fs, + os::unix::fs::PermissionsExt, + time::{Duration, Instant}, +}; + +use crate::support::helpers::DoingCmd; + +/// Write an executable stand-in editor that ignores its arguments and sleeps, +/// mimicking an editor that holds the terminal until the user quits it. +fn slow_editor(doing: &DoingCmd) -> String { + let path = doing + .doing_file_path() + .parent() + .expect("expected a temp dir") + .join("slow-editor"); + + fs::write(&path, "#!/bin/sh\nsleep 2\n").expect("failed to write stand-in editor"); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).expect("failed to mark it executable"); + + path.to_string_lossy().into_owned() +} + +#[test] +fn it_returns_before_the_editor_exits() { + let doing = DoingCmd::new(); + doing.run(["now", "Test entry"]).assert().success(); + let editor = slow_editor(&doing); + + let start = Instant::now(); + let output = doing + .run(["open", "--no-wait", "--editor", &editor]) + .output() + .expect("failed to run open --no-wait"); + + assert!( + output.status.success(), + "expected open --no-wait to succeed, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + start.elapsed() < Duration::from_secs(2), + "expected open --no-wait to return before the editor exited, took {:?}", + start.elapsed() + ); +} + +#[test] +fn it_waits_for_the_editor_without_the_flag() { + let doing = DoingCmd::new(); + doing.run(["now", "Test entry"]).assert().success(); + let editor = slow_editor(&doing); + + let start = Instant::now(); + doing.run(["open", "--editor", &editor]).assert().success(); + + assert!( + start.elapsed() >= Duration::from_secs(2), + "expected open to wait for the editor, took {:?}", + start.elapsed() + ); +} -- knotmirror 0.0.1