From 9f57887dbc9f2aa6dd4ac7c796fe657a8e44ea95 Mon Sep 17 00:00:00 2001 From: Amolith Date: Fri, 6 Mar 2026 09:19:58 -0700 Subject: [PATCH] Open editor on bare td create or td update --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/cmd/create.rs | 34 ++++++- src/cmd/update.rs | 48 ++++++++- src/editor.rs | 240 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + tests/cli_create.rs | 47 ++++++++- tests/cli_update.rs | 80 +++++++++++++++ 8 files changed, 452 insertions(+), 6 deletions(-) create mode 100644 src/editor.rs diff --git a/Cargo.lock b/Cargo.lock index 710444f03a1fb489e269219f9c6d30b58e7855ff..c4dc3ee6e895f8cf93f91dea4d6371b9482b3b0d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2586,6 +2586,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shell-words" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc6fe69c597f9c37bfeeeeeb33da3530379845f10be461a66d16d03eca2ded77" + [[package]] name = "shlex" version = "1.3.0" @@ -3519,6 +3525,7 @@ dependencies = [ "predicates", "serde", "serde_json", + "shell-words", "tempfile", "tokio", "ulid", diff --git a/Cargo.toml b/Cargo.toml index 1f908d25087b7e8844efb6b0f4be588ddd25001d..39db2eadcaa91267a52eb8161a69ba27be917653 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ path = "src/main.rs" [dependencies] anyhow = "1" +shell-words = "1" chrono = { version = "0.4", default-features = false, features = ["now"] } clap = { version = "4", features = ["derive"] } comfy-table = "7.2.2" diff --git a/src/cmd/create.rs b/src/cmd/create.rs index 3ac59021ca8ed2558ac27ad248d6bbeba768a37f..caeecd72e1bdc30ab8428eab58a29b50af931d3e 100644 --- a/src/cmd/create.rs +++ b/src/cmd/create.rs @@ -3,6 +3,7 @@ use loro::LoroMap; use std::path::Path; use crate::db; +use crate::editor; pub struct Opts<'a> { pub title: Option<&'a str>, @@ -15,11 +16,38 @@ pub struct Opts<'a> { pub json: bool, } +/// Template shown in the editor when the user runs `td create` without a title. +const TEMPLATE: &str = " +TD: Please provide the task title on the first line, and an optional +TD: description below. Lines starting with 'TD: ' will be ignored. +TD: An empty message aborts."; + pub fn run(root: &Path, opts: Opts) -> Result<()> { - let title = opts.title.ok_or_else(|| anyhow!("title required"))?; - let desc = opts.desc.unwrap_or(""); - let ts = db::now_utc(); + // If neither title nor description were supplied, try to open an editor. + // We treat the presence of TD_FORCE_EDITOR as an explicit interactive + // signal (used by tests); otherwise we check whether stdin is a tty. + let (title_owned, desc_owned); + let (title, desc) = if opts.title.is_none() && opts.desc.is_none() { + let interactive = std::env::var("TD_FORCE_EDITOR").is_ok() + || std::io::IsTerminal::is_terminal(&std::io::stdin()); + if interactive { + let (t, d) = editor::open(TEMPLATE)?; + title_owned = t; + desc_owned = d; + (title_owned.as_str(), desc_owned.as_str()) + } else { + return Err(anyhow!( + "title required; provide it as a positional argument or run interactively to open an editor" + )); + } + } else { + ( + opts.title.ok_or_else(|| anyhow!("title required"))?, + opts.desc.unwrap_or(""), + ) + }; + let ts = db::now_utc(); let store = db::open(root)?; let id = db::gen_id(); diff --git a/src/cmd/update.rs b/src/cmd/update.rs index fc6d9928574ab6431ff030c743826b1d08059c80..a0964f0ab7f13080784c56f77324031e49c1ed18 100644 --- a/src/cmd/update.rs +++ b/src/cmd/update.rs @@ -2,6 +2,7 @@ use anyhow::{anyhow, Result}; use std::path::Path; use crate::db; +use crate::editor; pub struct Opts<'a> { pub status: Option<&'a str>, @@ -19,6 +20,49 @@ pub fn run(root: &Path, id: &str, opts: Opts) -> Result<()> { let parsed_status = opts.status.map(db::parse_status).transpose()?; + // If no fields were supplied, open the editor so the user can revise the + // task's title and description interactively. + let editor_title; + let editor_desc; + let (title_override, desc_override) = if opts.status.is_none() + && opts.priority.is_none() + && opts.effort.is_none() + && opts.title.is_none() + && opts.desc.is_none() + { + let interactive = std::env::var("TD_FORCE_EDITOR").is_ok() + || std::io::IsTerminal::is_terminal(&std::io::stdin()); + if interactive { + // Load the current task so we can pre-populate the template. + let task = store + .get_task(&task_id, false)? + .ok_or_else(|| anyhow!("task not found"))?; + + let template = format!( + "{}\n\ + \n\ + {}\n\ + \n\ + TD: Edit the title and description above. The first line is the\n\ + TD: title; everything after the blank line is the description.\n\ + TD: Lines starting with 'TD: ' will be ignored. Saving an empty\n\ + TD: message aborts the update.", + task.title, task.description, + ); + + let (t, d) = editor::open(&template)?; + editor_title = t; + editor_desc = d; + (Some(editor_title.as_str()), Some(editor_desc.as_str())) + } else { + return Err(anyhow!( + "nothing to update; provide at least one flag or run interactively to open an editor" + )); + } + } else { + (opts.title, opts.desc) + }; + store.apply_and_persist(|doc| { let tasks = doc.get_map("tasks"); let task = db::get_task_map(&tasks, &task_id)?.ok_or_else(|| anyhow!("task not found"))?; @@ -32,10 +76,10 @@ pub fn run(root: &Path, id: &str, opts: Opts) -> Result<()> { if let Some(e) = opts.effort { task.insert("effort", db::effort_label(e))?; } - if let Some(t) = opts.title { + if let Some(t) = title_override { task.insert("title", t)?; } - if let Some(d) = opts.desc { + if let Some(d) = desc_override { task.insert("description", d)?; } task.insert("updated_at", ts.clone())?; diff --git a/src/editor.rs b/src/editor.rs new file mode 100644 index 0000000000000000000000000000000000000000..366fff78d65d2243439b530b46df6c39145ac0e6 --- /dev/null +++ b/src/editor.rs @@ -0,0 +1,240 @@ +/// Open the user's preferred editor to compose or revise text. +/// +/// Editor discovery follows the conventional priority order: `$VISUAL`, +/// then `$EDITOR`, then a platform default (`nano` on Unix, `notepad` on +/// Windows). The value is split into a program and optional arguments +/// using POSIX shell-word rules (via the `shell-words` crate), so values +/// like `vim -u NONE` or `code --wait` work correctly. +/// +/// In tests, set `TD_FORCE_EDITOR=` to bypass the real editor and +/// supply a fake one (e.g. a shell one-liner that writes known content). +use anyhow::{anyhow, Result}; + +/// Locate the editor binary (and any leading flags) from the environment. +/// +/// Priority: `TD_FORCE_EDITOR` (tests) → `$VISUAL` → `$EDITOR` → +/// platform default. Returns the argv list ready to pass to +/// `std::process::Command`. +pub fn find_editor() -> Result> { + // TD_FORCE_EDITOR lets tests inject a fake editor without touching the + // real environment variables. + let raw = std::env::var("TD_FORCE_EDITOR") + .or_else(|_| std::env::var("VISUAL")) + .or_else(|_| std::env::var("EDITOR")) + .unwrap_or_else(|_| { + if cfg!(windows) { + "notepad".to_owned() + } else { + "nano".to_owned() + } + }); + + let argv = shell_words::split(&raw) + .map_err(|e| anyhow!("could not parse editor command {:?}: {}", raw, e))?; + + if argv.is_empty() { + return Err(anyhow!( + "editor command is empty; set $VISUAL or $EDITOR to a valid command" + )); + } + + Ok(argv) +} + +/// Strip lines beginning with `TD: `, then extract the first non-blank line +/// as the title and everything that follows as the description. +/// +/// Lines starting with `#` are intentionally preserved — they are markdown +/// headings that belong to the task content, not directives. +/// +/// Returns `Err` when the result would be an empty title. +pub fn parse_result(content: &str) -> Result<(String, String)> { + let meaningful: Vec<&str> = content.lines().filter(|l| !l.starts_with("TD: ")).collect(); + + // Find the first non-blank line — that becomes the title. + let title_pos = meaningful + .iter() + .position(|l| !l.trim().is_empty()) + .ok_or_else(|| anyhow!("editor returned an empty message; task creation aborted"))?; + + let title = meaningful[title_pos].trim().to_owned(); + + // Everything after the title line is the description; trim surrounding + // blank lines but preserve internal ones. + let desc = meaningful[title_pos + 1..].join("\n").trim().to_owned(); + + Ok((title, desc)) +} + +/// Write `template` to a temporary file, open the editor, read the +/// result back, clean up the file, and return `parse_result`. +pub fn open(template: &str) -> Result<(String, String)> { + use std::io::Write; + + // Create a uniquely-named temp file so concurrent td invocations don't + // collide. The `.md` extension is a hint to editors to enable markdown + // highlighting. + let id = ulid::Ulid::new(); + let path = std::env::temp_dir().join(format!("td-edit-{id}.md")); + + { + let mut f = std::fs::File::create(&path)?; + f.write_all(template.as_bytes())?; + } + + // Best-effort cleanup: delete the temp file even if the editor or parse + // step fails. + let result = (|| { + let argv = find_editor()?; + let (program, args) = argv + .split_first() + .ok_or_else(|| anyhow!("editor command resolved to an empty list"))?; + + let status = std::process::Command::new(program) + .args(args) + .arg(&path) + .status() + .map_err(|e| anyhow!("failed to launch editor {:?}: {}", program, e))?; + + if !status.success() { + return Err(anyhow!("editor exited with status {}", status)); + } + + let content = std::fs::read_to_string(&path)?; + parse_result(&content) + })(); + + // Always remove the temp file, regardless of success or failure. + let _ = std::fs::remove_file(&path); + + result +} + +// ── tests ───────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + + // ── find_editor ─────────────────────────────────────────────────────────── + + #[test] + fn find_editor_prefers_td_force_editor() { + // TD_FORCE_EDITOR should take precedence over VISUAL and EDITOR. + std::env::remove_var("VISUAL"); + std::env::remove_var("EDITOR"); + std::env::set_var("TD_FORCE_EDITOR", "myfakeeditor"); + let argv = find_editor().unwrap(); + assert_eq!(argv, vec!["myfakeeditor"]); + std::env::remove_var("TD_FORCE_EDITOR"); + } + + #[test] + fn find_editor_prefers_visual_over_editor() { + std::env::remove_var("TD_FORCE_EDITOR"); + std::env::set_var("VISUAL", "vis"); + std::env::set_var("EDITOR", "ed"); + let argv = find_editor().unwrap(); + assert_eq!(argv[0], "vis"); + std::env::remove_var("VISUAL"); + std::env::remove_var("EDITOR"); + } + + #[test] + fn find_editor_falls_back_to_editor() { + std::env::remove_var("TD_FORCE_EDITOR"); + std::env::remove_var("VISUAL"); + std::env::set_var("EDITOR", "myeditor"); + let argv = find_editor().unwrap(); + assert_eq!(argv[0], "myeditor"); + std::env::remove_var("EDITOR"); + } + + #[test] + fn find_editor_splits_args() { + // Values like "vim -u NONE" must be split into separate argv entries. + std::env::remove_var("TD_FORCE_EDITOR"); + std::env::remove_var("VISUAL"); + std::env::set_var("EDITOR", "vim -u NONE"); + let argv = find_editor().unwrap(); + assert_eq!(argv, vec!["vim", "-u", "NONE"]); + std::env::remove_var("EDITOR"); + } + + #[test] + fn find_editor_splits_quoted_path() { + // Paths with spaces must survive shell-word quoting. + std::env::remove_var("TD_FORCE_EDITOR"); + std::env::set_var("VISUAL", "\"/usr/local/bin/my editor\""); + std::env::remove_var("EDITOR"); + let argv = find_editor().unwrap(); + assert_eq!(argv, vec!["/usr/local/bin/my editor"]); + std::env::remove_var("VISUAL"); + } + + #[test] + fn find_editor_errors_on_unmatched_quote() { + std::env::remove_var("TD_FORCE_EDITOR"); + std::env::remove_var("VISUAL"); + std::env::set_var("EDITOR", "vim 'unterminated"); + let result = find_editor(); + assert!(result.is_err()); + std::env::remove_var("EDITOR"); + } + + // ── parse_result ────────────────────────────────────────────────────────── + + #[test] + fn parse_strips_td_comment_lines() { + let content = "TD: ignore me\nMy title\nsome description"; + let (title, desc) = parse_result(content).unwrap(); + assert_eq!(title, "My title"); + assert_eq!(desc, "some description"); + } + + #[test] + fn parse_preserves_markdown_headings() { + // Lines starting with '#' are NOT comments — they're markdown headings + // and must be preserved as description content. + let content = "TD: ignore me\nMy title\n## Details\nsome description"; + let (title, desc) = parse_result(content).unwrap(); + assert_eq!(title, "My title"); + assert_eq!(desc, "## Details\nsome description"); + } + + #[test] + fn parse_returns_empty_desc_when_only_title() { + let content = "TD: comment\nJust a title"; + let (title, desc) = parse_result(content).unwrap(); + assert_eq!(title, "Just a title"); + assert_eq!(desc, ""); + } + + #[test] + fn parse_trims_surrounding_blank_lines_from_desc() { + let content = "Title\n\nParagraph one\n\nParagraph two\n"; + let (title, desc) = parse_result(content).unwrap(); + assert_eq!(title, "Title"); + // Leading/trailing blank lines stripped from description. + assert_eq!(desc, "Paragraph one\n\nParagraph two"); + } + + #[test] + fn parse_errors_on_only_comments() { + let content = "TD: first comment\nTD: second comment\n"; + let result = parse_result(content); + assert!(result.is_err()); + } + + #[test] + fn parse_errors_on_empty_string() { + let result = parse_result(""); + assert!(result.is_err()); + } + + #[test] + fn parse_errors_on_blank_lines_only() { + let result = parse_result(" \n\n \n"); + assert!(result.is_err()); + } +} diff --git a/src/lib.rs b/src/lib.rs index 44e052d83dc9e3491d7871eb47393b36b5452cda..5538d071a456e6b4dcadbc84ede97fbd552c435d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ pub mod cli; pub mod cmd; pub mod color; pub mod db; +pub mod editor; pub mod migrate; pub mod score; diff --git a/tests/cli_create.rs b/tests/cli_create.rs index 9a464d11431439e4e8283b1306b2531368d2184d..b0ddba8b4d8982b5891b34698f8296bf80c0e54b 100644 --- a/tests/cli_create.rs +++ b/tests/cli_create.rs @@ -101,7 +101,9 @@ fn create_with_labels() { } #[test] -fn create_requires_title() { +fn create_without_title_non_interactive_errors() { + // Without a title, in non-interactive mode (no tty), td should fail with + // a helpful message rather than silently doing nothing. let tmp = init_tmp(); td(&tmp) @@ -112,6 +114,49 @@ fn create_requires_title() { .stderr(predicate::str::contains("title required")); } +#[test] +fn create_via_editor_uses_first_line_as_title() { + // When TD_FORCE_EDITOR is set to a command that writes known content, td + // should pick up the result and create the task. + let tmp = init_tmp(); + + // The fake editor overwrites its first argument with a known payload. + let fake_editor = "sh -c 'printf \"Editor title\\nEditor description\" > \"$1\"' sh"; + + let out = td(&tmp) + .args(["--json", "create"]) + .env("TD_FORCE_EDITOR", fake_editor) + .current_dir(&tmp) + .output() + .unwrap(); + + assert!( + out.status.success(), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert_eq!(v["title"].as_str().unwrap(), "Editor title"); + assert_eq!(v["description"].as_str().unwrap(), "Editor description"); +} + +#[test] +fn create_via_editor_aborts_on_empty_file() { + // If the editor leaves the file empty (or only comments), td should exit + // with a non-zero status and not create a task. + let tmp = init_tmp(); + + let fake_editor = "sh -c 'printf \"TD: just a comment\\n\" > \"$1\"' sh"; + + td(&tmp) + .args(["create"]) + .env("TD_FORCE_EDITOR", fake_editor) + .current_dir(&tmp) + .assert() + .failure() + .stderr(predicate::str::contains("aborted")); +} + #[test] fn create_subtask_under_parent() { let tmp = init_tmp(); diff --git a/tests/cli_update.rs b/tests/cli_update.rs index b30b2f39aa84dcb6e5ed9a849d3a165581bf0c57..e66183bd35b0545ee58b32d52e64e94ad358eaf7 100644 --- a/tests/cli_update.rs +++ b/tests/cli_update.rs @@ -186,3 +186,83 @@ fn reopen_reopens_closed_task() { assert_eq!(get_task_json(&tmp, &id)["status"], "open"); } + +// ── editor fallback ─────────────────────────────────────────────────────────── + +#[test] +fn update_via_editor_changes_title_and_desc() { + // Bare `td update ` with TD_FORCE_EDITOR should open the editor + // pre-populated and apply whatever the fake editor writes back. + let tmp = init_tmp(); + let id = create_task(&tmp, "Original title"); + + let fake_editor = "sh -c 'printf \"New title\\nNew description\" > \"$1\"' sh"; + + td(&tmp) + .args(["update", &id]) + .env("TD_FORCE_EDITOR", fake_editor) + .current_dir(&tmp) + .assert() + .success(); + + let t = get_task_json(&tmp, &id); + assert_eq!(t["title"].as_str().unwrap(), "New title"); + assert_eq!(t["description"].as_str().unwrap(), "New description"); +} + +#[test] +fn update_via_editor_aborts_on_empty_file() { + // If the fake editor leaves only comments, the update should be aborted. + let tmp = init_tmp(); + let id = create_task(&tmp, "Stays the same"); + + let fake_editor = "sh -c 'printf \"TD: comment only\\n\" > \"$1\"' sh"; + + td(&tmp) + .args(["update", &id]) + .env("TD_FORCE_EDITOR", fake_editor) + .current_dir(&tmp) + .assert() + .failure() + .stderr(predicate::str::contains("aborted")); + + // Title must be unchanged. + let t = get_task_json(&tmp, &id); + assert_eq!(t["title"].as_str().unwrap(), "Stays the same"); +} + +#[test] +fn update_via_editor_preserves_existing_content_as_template() { + // The editor should be pre-populated with the task's current title and + // description, so the user can edit rather than retype from scratch. + let tmp = init_tmp(); + let id = create_task(&tmp, "Existing title"); + + // Set description first. + td(&tmp) + .args(["update", &id, "-d", "Existing description"]) + .current_dir(&tmp) + .assert() + .success(); + + // Fake editor: grep out the first non-comment, non-blank line (the title), + // then write "title: " so we can assert it was pre-populated. + let fake_editor = concat!( + "sh -c '", + r#"title=$(grep -v "^TD: " "$1" | grep -v "^[[:space:]]*$" | head -1); "#, + r#"printf "title: %s" "$title" > "$1""#, + "' sh" + ); + + td(&tmp) + .args(["update", &id]) + .env("TD_FORCE_EDITOR", fake_editor) + .current_dir(&tmp) + .assert() + .success(); + + let t = get_task_json(&tmp, &id); + // The fake editor wrote the existing title back with a prefix, proving it + // received the pre-populated template. + assert_eq!(t["title"].as_str().unwrap(), "title: Existing title"); +}