From 6068a0ed9fc574f6b3aaa3e79943630c88df51be Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Sun, 24 Aug 2025 09:48:12 -0600 Subject: [PATCH] refactoring, cleanup --- Cargo.lock | 7 + Cargo.toml | 1 + src/commands.rs | 362 ++++++++++++++++++++++--------------- src/content.rs | 2 +- src/main.rs | 39 ++-- src/markdown.rs | 143 ++++++++++----- src/migration.rs | 68 ++++--- src/tests.rs | 45 +++-- tests/integration_tests.rs | 350 ++++++++++++++++++----------------- tests/validation_tests.rs | 130 +++++++++++++ 10 files changed, 730 insertions(+), 417 deletions(-) create mode 100644 tests/validation_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 08e956d..70f75c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,6 +67,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "anyhow" +version = "1.0.99" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100" + [[package]] name = "autocfg" version = "1.5.0" @@ -244,6 +250,7 @@ dependencies = [ name = "ldr" version = "1.0.2" dependencies = [ + "anyhow", "chrono", "clap", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index a0208c9..ca34f96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ keywords = ["todo", "cli", "productivity"] categories = ["command-line-utilities"] [dependencies] +anyhow = "1.0" clap = { version = "4.5.41", features = ["derive"] } xdg = "3.0.0" termion = "4.0.5" diff --git a/src/commands.rs b/src/commands.rs index 0a3b29a..6ae3f9f 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -5,17 +5,17 @@ //! Now supports subtasks and multiple lists in Markdown format. use crate::markdown::{ - parse_todo_file, generate_todo_file, parse_archive_file, generate_archive_file, - Task, TodoFile, ArchiveFile, TaskRef + generate_archive_file, generate_todo_file, parse_archive_file, parse_todo_file, ArchiveFile, + Task, TaskRef, TodoFile, }; +use anyhow::{anyhow, Context, Result}; use std::collections::HashSet; use std::env; +use std::fmt; use std::fs; -use std::io; use std::path::Path; use std::process::Command; use termion::color; -use std::fmt; // Custom 256-color support struct Color256(u8); @@ -32,21 +32,21 @@ fn hsv_to_rgb(h: f32, s: f32, v: f32) -> (u8, u8, u8) { let c = v * s; let x = c * (1.0 - ((h % 2.0) - 1.0).abs()); let m = v - c; - - let (r, g, b) = if h >= 0.0 && h < 1.0 { + + let (r, g, b) = if (0.0..1.0).contains(&h) { (c, x, 0.0) - } else if h >= 1.0 && h < 2.0 { + } else if (1.0..2.0).contains(&h) { (x, c, 0.0) - } else if h >= 2.0 && h < 3.0 { + } else if (2.0..3.0).contains(&h) { (0.0, c, x) - } else if h >= 3.0 && h < 4.0 { + } else if (3.0..4.0).contains(&h) { (0.0, x, c) - } else if h >= 4.0 && h < 5.0 { + } else if (4.0..5.0).contains(&h) { (x, 0.0, c) } else { (c, 0.0, x) }; - + ( ((r + m) * 255.0) as u8, ((g + m) * 255.0) as u8, @@ -58,10 +58,28 @@ fn hsv_to_rgb(h: f32, s: f32, v: f32) -> (u8, u8, u8) { fn rgb_to_256_color(r: u8, g: u8, b: u8) -> u8 { // For colors 16-231: 6x6x6 color cube // Each component ranges from 0-5, mapped from 0-255 - let r_index = if r < 48 { 0 } else if r < 115 { 1 } else { (r - 55) / 40 }; - let g_index = if g < 48 { 0 } else if g < 115 { 1 } else { (g - 55) / 40 }; - let b_index = if b < 48 { 0 } else if b < 115 { 1 } else { (b - 55) / 40 }; - + let r_index = if r < 48 { + 0 + } else if r < 115 { + 1 + } else { + (r - 55) / 40 + }; + let g_index = if g < 48 { + 0 + } else if g < 115 { + 1 + } else { + (g - 55) / 40 + }; + let b_index = if b < 48 { + 0 + } else if b < 115 { + 1 + } else { + (b - 55) / 40 + }; + 16 + 36 * r_index + 6 * g_index + b_index } @@ -74,11 +92,11 @@ fn hsv_color(h: f32, s: f32, v: f32) -> Color256 { // Color scheme configuration struct ColorScheme { // Main task colors (bright, high saturation) - task1_hue: f32, // Primary hue for odd tasks - task2_hue: f32, // Primary hue for even tasks + task1_hue: f32, // Primary hue for odd tasks + task2_hue: f32, // Primary hue for even tasks main_saturation: f32, main_value: f32, - + // Subtask adjustments value_reduction: f32, // Amount to reduce brightness for subtasks } @@ -88,27 +106,27 @@ impl ColorScheme { if Self::is_dark_terminal() { // Dark terminal scheme - bright colors ColorScheme { - task1_hue: 200.0, // Light cyan-blue - task2_hue: 40.0, // Light desert tan/gold + task1_hue: 200.0, // Light cyan-blue + task2_hue: 40.0, // Light desert tan/gold main_saturation: 0.7, - main_value: 0.95, // Very bright + main_value: 0.95, // Very bright value_reduction: 0.2, } } else { // Light terminal scheme - darker colors ColorScheme { - task1_hue: 210.0, // Darker blue - task2_hue: 30.0, // Darker orange + task1_hue: 210.0, // Darker blue + task2_hue: 30.0, // Darker orange main_saturation: 0.8, - main_value: 0.6, // Much darker for light backgrounds + main_value: 0.6, // Much darker for light backgrounds value_reduction: 0.15, } } } - + fn is_dark_terminal() -> bool { // Check various indicators for dark terminal - + // Check COLORFGBG environment variable (format: "15;0" means white fg, black bg) if let Ok(colorfgbg) = std::env::var("COLORFGBG") { if let Some(bg) = colorfgbg.split(';').nth(1) { @@ -118,14 +136,14 @@ impl ColorScheme { } } } - + // Check for common dark terminal themes in environment if let Ok(term) = std::env::var("TERM") { if term.contains("dark") { return true; } } - + // Check common terminal emulators that default to dark if let Ok(term_program) = std::env::var("TERM_PROGRAM") { match term_program.as_str() { @@ -133,11 +151,11 @@ impl ColorScheme { _ => {} } } - + // Default to dark terminal (most developers use dark themes) true } - + fn get_main_task_color(&self, task_num: usize) -> Color256 { let hue = if task_num % 2 == 1 { self.task1_hue @@ -146,18 +164,18 @@ impl ColorScheme { }; hsv_color(hue, self.main_saturation, self.main_value) } - + fn get_subtask_color(&self, task_num: usize, _subtask_idx: usize) -> Color256 { let base_hue = if task_num % 2 == 1 { self.task1_hue } else { self.task2_hue }; - + // Simply inherit parent color but reduce saturation and value let reduced_saturation = self.main_saturation - 0.15; // Reduce saturation by 15% let reduced_value = self.main_value - self.value_reduction; - + hsv_color(base_hue, reduced_saturation, reduced_value) } } @@ -165,10 +183,25 @@ impl ColorScheme { /// Adds a new entry to the todo file. /// Creates the file if it doesn't exist, otherwise prepends to the main list. /// Can add as subtask if `under` is specified. -pub fn add_entry(path: &Path, text: &str, under: Option) -> io::Result<()> { +pub fn add_entry(path: &Path, text: &str, under: Option) -> Result<()> { + // Validate input + if text.trim().is_empty() { + return Err(anyhow!("Cannot add empty task")); + } + + // Limit task text length to prevent abuse + const MAX_TASK_LENGTH: usize = 500; + if text.len() > MAX_TASK_LENGTH { + return Err(anyhow!( + "Task text too long ({}). Maximum length is {} characters", + text.len(), + MAX_TASK_LENGTH + )); + } let mut todo_file = if path.exists() { - let content = fs::read_to_string(path)?; - parse_todo_file(&content).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))? + let content = fs::read_to_string(path) + .with_context(|| format!("Failed to read file: {}", path.display()))?; + parse_todo_file(&content).map_err(|e| anyhow!("Failed to parse file: {}", e))? } else { TodoFile::new("TODOs".to_string()) }; @@ -176,12 +209,24 @@ pub fn add_entry(path: &Path, text: &str, under: Option) -> io::Result<() if let Some(task_num) = under { // Add as subtask if task_num == 0 || task_num > todo_file.tasks.len() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Invalid task number: {}. Valid range: 1-{}", task_num, todo_file.tasks.len()) + return Err(anyhow!( + "Invalid task number: {}. Valid range: 1-{}", + task_num, + todo_file.tasks.len() )); } - + + // Limit number of subtasks per task + const MAX_SUBTASKS: usize = 26; // a-z + let task = &todo_file.tasks[task_num - 1]; + if task.subtasks.len() >= MAX_SUBTASKS { + return Err(anyhow!( + "Task {} already has maximum number of subtasks ({})", + task_num, + MAX_SUBTASKS + )); + } + todo_file.tasks[task_num - 1].add_subtask(text.to_string()); println!( "{}✓ Added subtask to task {}: {}{}", @@ -192,6 +237,15 @@ pub fn add_entry(path: &Path, text: &str, under: Option) -> io::Result<() ); } else { // Add as new main task at top + // Limit total number of tasks to prevent abuse + const MAX_TASKS: usize = 1000; + if todo_file.tasks.len() >= MAX_TASKS { + return Err(anyhow!( + "Maximum number of tasks ({}) reached. Please archive or remove some tasks first", + MAX_TASKS + )); + } + let task = Task::new(text.to_string()); todo_file.prepend_task(task); println!( @@ -203,12 +257,12 @@ pub fn add_entry(path: &Path, text: &str, under: Option) -> io::Result<() } let content = generate_todo_file(&todo_file); - fs::write(path, content) + fs::write(path, content).with_context(|| format!("Failed to write file: {}", path.display())) } /// Lists tasks with numbered display including subtasks. /// Displays task numbers and subtask letters, supports filtering. -pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io::Result<()> { +pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> Result<()> { if !path.exists() { println!( "{}No notes yet.{}", @@ -218,9 +272,10 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io return Ok(()); } - let content = fs::read_to_string(path)?; - let todo_file = parse_todo_file(&content) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; + let content = fs::read_to_string(path) + .with_context(|| format!("Failed to read file: {}", path.display()))?; + let todo_file = + parse_todo_file(&content).map_err(|e| anyhow!("Failed to parse file: {}", e))?; if todo_file.is_empty() { println!( @@ -235,11 +290,11 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io let display_items: Vec<_> = if let Some(filter_text) = filter { let mut filtered = Vec::new(); let filter_lower = filter_text.to_lowercase(); - + for (task_idx, task) in todo_file.tasks.iter().enumerate() { let task_num = task_idx + 1; let task_matches = task.text.to_lowercase().contains(&filter_lower); - + // Check which subtasks match let mut matching_subtasks = Vec::new(); for (subtask_idx, subtask) in task.subtasks.iter().enumerate() { @@ -247,12 +302,12 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io matching_subtasks.push(subtask_idx); } } - + if task_matches { // If task matches, include task and ALL its subtasks let task_line = format!("{:3}. {}", task_num, task.text); filtered.push((task_num, None, task_line)); - + for (subtask_idx, subtask) in task.subtasks.iter().enumerate() { let letter = (b'a' + subtask_idx as u8) as char; let subtask_line = format!(" {}. {}", letter, subtask); @@ -262,7 +317,7 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io // If only subtasks match, include task and only matching subtasks let task_line = format!("{:3}. {}", task_num, task.text); filtered.push((task_num, None, task_line)); - + for &subtask_idx in &matching_subtasks { let letter = (b'a' + subtask_idx as u8) as char; let subtask_line = format!(" {}. {}", letter, &task.subtasks[subtask_idx]); @@ -270,7 +325,7 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io } } } - + filtered } else { // No filter - include everything @@ -295,7 +350,7 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io println!( "{}No items found matching filter: \"{}\"{}", color::Fg(color::Yellow), - filter.unwrap(), + filter.unwrap_or(""), color::Fg(color::Reset) ); } else { @@ -315,26 +370,16 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io }; let color_scheme = ColorScheme::new(); - + for (task_num, subtask_idx, line) in display_items.iter().take(display_count) { if subtask_idx.is_none() { // Main task - use HSV-based bright colors let color = color_scheme.get_main_task_color(*task_num); - println!( - "{}{}{}", - color, - line, - color::Fg(color::Reset) - ); + println!("{}{}{}", color, line, color::Fg(color::Reset)); } else { // Subtask - use same color family as parent but dimmer let color = color_scheme.get_subtask_color(*task_num, subtask_idx.unwrap()); - println!( - "{}{}{}", - color, - line, - color::Fg(color::Reset) - ); + println!("{}{}{}", color, line, color::Fg(color::Reset)); } } @@ -351,7 +396,7 @@ pub fn list_note(path: &Path, num: usize, all: bool, filter: Option<&str>) -> io } /// Parse task references and perform operations on tasks/subtasks -pub fn prioritize_items(todo_path: &Path, refs: &[String]) -> io::Result<()> { +pub fn prioritize_items(todo_path: &Path, refs: &[String]) -> Result<()> { if !todo_path.exists() { println!( "{}No notes found.{}", @@ -361,9 +406,10 @@ pub fn prioritize_items(todo_path: &Path, refs: &[String]) -> io::Result<()> { return Ok(()); } - let content = fs::read_to_string(todo_path)?; - let mut todo_file = parse_todo_file(&content) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; + let content = fs::read_to_string(todo_path) + .with_context(|| format!("Failed to read file: {}", todo_path.display()))?; + let mut todo_file = + parse_todo_file(&content).map_err(|e| anyhow!("Failed to parse file: {}", e))?; if todo_file.is_empty() { println!( @@ -432,41 +478,44 @@ pub fn prioritize_items(todo_path: &Path, refs: &[String]) -> io::Result<()> { } } - // Move tasks to front in the order specified - let mut new_tasks = Vec::new(); - let mut moved_tasks = Vec::new(); - + // Create new task order by swapping moved tasks to front + let old_tasks = std::mem::take(&mut todo_file.tasks); + let mut new_tasks = Vec::with_capacity(old_tasks.len()); + let mut moved_task_names = Vec::new(); + + // First add the moved tasks in the order specified for &task_idx in &tasks_to_move { - moved_tasks.push(todo_file.tasks[task_idx].clone()); + if task_idx < old_tasks.len() { + new_tasks.push(old_tasks[task_idx].clone()); + moved_task_names.push(old_tasks[task_idx].text.clone()); + } } - // Add moved tasks first - new_tasks.extend(moved_tasks.clone()); - - // Add remaining tasks - for (idx, task) in todo_file.tasks.iter().enumerate() { + // Then add all non-moved tasks + for (idx, task) in old_tasks.into_iter().enumerate() { if !moved_task_indices.contains(&idx) { - new_tasks.push(task.clone()); + new_tasks.push(task); } } todo_file.tasks = new_tasks; let new_content = generate_todo_file(&todo_file); - fs::write(todo_path, new_content)?; + fs::write(todo_path, new_content) + .with_context(|| format!("Failed to write file: {}", todo_path.display()))?; println!( "{}✓ Prioritized {} task(s){}", color::Fg(color::Green), - moved_tasks.len(), + moved_task_names.len(), color::Fg(color::Reset) ); - - for task in moved_tasks { + + for task_name in moved_task_names { println!( " {}{}{}", color::Fg(color::Magenta), - task.text, + task_name, color::Fg(color::Reset) ); } @@ -474,8 +523,13 @@ pub fn prioritize_items(todo_path: &Path, refs: &[String]) -> io::Result<()> { Ok(()) } -/// Archive specified tasks or subtasks -pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> io::Result<()> { +/// Internal helper to process items for removal or archiving +fn process_items_for_removal( + todo_path: &Path, + refs: &[String], + archive_path: Option<&Path>, +) -> Result<()> { + let should_archive = archive_path.is_some(); if !todo_path.exists() { println!( "{}No notes found.{}", @@ -485,14 +539,17 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> return Ok(()); } - let content = fs::read_to_string(todo_path)?; - let mut todo_file = parse_todo_file(&content) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; + let content = fs::read_to_string(todo_path) + .with_context(|| format!("Failed to read file: {}", todo_path.display()))?; + let mut todo_file = + parse_todo_file(&content).map_err(|e| anyhow!("Failed to parse file: {}", e))?; if todo_file.is_empty() { + let action = if should_archive { "archive" } else { "remove" }; println!( - "{}No notes to archive.{}", + "{}No notes to {}.{}", color::Fg(color::Yellow), + action, color::Fg(color::Reset) ); return Ok(()); @@ -567,40 +624,52 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> } } - // Load archive file - let mut archive_file = if archive_path.exists() { - let archive_content = fs::read_to_string(archive_path)?; - parse_archive_file(&archive_content) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))? + // Load archive file if we're archiving + let mut archive_file = if let Some(archive_path) = archive_path { + if archive_path.exists() { + let archive_content = fs::read_to_string(archive_path) + .with_context(|| format!("Failed to read archive: {}", archive_path.display()))?; + parse_archive_file(&archive_content) + .map_err(|e| anyhow!("Failed to parse file: {}", e))? + } else { + ArchiveFile::new() + } } else { ArchiveFile::new() }; - // Add items to archive - if !tasks_to_archive.is_empty() { - archive_file.add_items_for_today("Default", tasks_to_archive.clone()); - let archive_content = generate_archive_file(&archive_file); - fs::write(archive_path, archive_content)?; + // Add items to archive if we're archiving + if should_archive && !tasks_to_archive.is_empty() { + if let Some(archive_path) = archive_path { + archive_file.add_items_for_today("Default", tasks_to_archive.clone()); + let archive_content = generate_archive_file(&archive_file); + fs::write(archive_path, archive_content) + .with_context(|| format!("Failed to write archive: {}", archive_path.display()))?; + } } // Remove items from todo file // Remove subtasks first (in reverse order to maintain indices) - let mut subtasks_by_task: std::collections::HashMap> = std::collections::HashMap::new(); + let mut subtasks_by_task: std::collections::HashMap> = + std::collections::HashMap::new(); for &(task_idx, subtask_idx) in &subtasks_to_remove { if !whole_tasks_to_remove.contains(&task_idx) { - subtasks_by_task.entry(task_idx).or_default().push(subtask_idx); + subtasks_by_task + .entry(task_idx) + .or_default() + .push(subtask_idx); } } // Track tasks that might need auto-completion let mut tasks_to_auto_complete = Vec::new(); - + for (task_idx, mut subtask_indices) in subtasks_by_task { subtask_indices.sort_by(|a, b| b.cmp(a)); // Sort in reverse order for subtask_idx in subtask_indices { todo_file.tasks[task_idx].subtasks.remove(subtask_idx); } - + // Check if this task now has no subtasks left and should be auto-completed if todo_file.tasks[task_idx].subtasks.is_empty() { tasks_to_auto_complete.push(task_idx); @@ -613,12 +682,16 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> for &task_idx in &tasks_to_auto_complete { auto_completed_tasks.push(todo_file.tasks[task_idx].clone()); } - - // Add auto-completed tasks to archive - if !auto_completed_tasks.is_empty() { - archive_file.add_items_for_today("Default", auto_completed_tasks.clone()); - let archive_content = generate_archive_file(&archive_file); - fs::write(archive_path, archive_content)?; + + // Add auto-completed tasks to archive if we're archiving + if should_archive && !auto_completed_tasks.is_empty() { + if let Some(archive_path) = archive_path { + archive_file.add_items_for_today("Default", auto_completed_tasks.clone()); + let archive_content = generate_archive_file(&archive_file); + fs::write(archive_path, archive_content).with_context(|| { + format!("Failed to write archive: {}", archive_path.display()) + })?; + } } } @@ -627,23 +700,30 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> whole_task_indices.extend(tasks_to_auto_complete); whole_task_indices.sort_by(|a, b| b.cmp(a)); whole_task_indices.dedup(); // Remove duplicates in case a task was both manually selected and auto-completed - + for task_idx in whole_task_indices { todo_file.tasks.remove(task_idx); } // Save updated todo file let new_content = generate_todo_file(&todo_file); - fs::write(todo_path, new_content)?; + fs::write(todo_path, new_content) + .with_context(|| format!("Failed to write file: {}", todo_path.display()))?; - let total_archived = tasks_to_archive.len() + auto_completed_tasks.len(); + let total_processed = tasks_to_archive.len() + auto_completed_tasks.len(); + let action_verb = if should_archive { + "Archived" + } else { + "Removed" + }; println!( - "{}✓ Archived {} item(s){}", + "{}✓ {} {} item(s){}", color::Fg(color::Green), - total_archived, + action_verb, + total_processed, color::Fg(color::Reset) ); - + for task in tasks_to_archive { println!( " {}{}{}", @@ -652,7 +732,7 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> color::Fg(color::Reset) ); } - + // Show auto-completed tasks if !auto_completed_tasks.is_empty() { for task in auto_completed_tasks { @@ -668,55 +748,41 @@ pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> Ok(()) } -/// Remove items without archiving (same logic as archive but don't save to archive) -pub fn remove_items(todo_path: &Path, refs: &[String]) -> io::Result<()> { - // Implementation is similar to archive_items but without the archiving part - // For brevity, I'll implement a simplified version that reuses archive logic - - // Create a temporary path that we won't actually use - let temp_dir = std::env::temp_dir(); - let temp_archive = temp_dir.join("temp_archive_unused.md"); - - // Call archive_items but then delete the temp archive file - let result = archive_items(todo_path, &temp_archive, refs); - - // Clean up temp file if it was created - if temp_archive.exists() { - let _ = fs::remove_file(&temp_archive); - } - - // Change the success message to indicate removal instead of archiving - if result.is_ok() { - // The archive_items function already printed success, so we need to override - // For simplicity in this implementation, we'll leave the message as is - // In a full implementation, we'd refactor to avoid this duplication - } - - result +/// Archive specified tasks or subtasks +pub fn archive_items(todo_path: &Path, archive_path: &Path, refs: &[String]) -> Result<()> { + process_items_for_removal(todo_path, refs, Some(archive_path)) } +/// Remove items without archiving +pub fn remove_items(todo_path: &Path, refs: &[String]) -> Result<()> { + process_items_for_removal(todo_path, refs, None) +} /// Opens the todo file in the user's preferred editor -pub fn edit_note(todo_path: &Path) -> io::Result<()> { +pub fn edit_note(todo_path: &Path) -> Result<()> { let editor = env::var("EDITOR").unwrap_or_else(|_| "nano".to_string()); // Create the file if it doesn't exist if !todo_path.exists() { let empty_file = TodoFile::new("TODOs".to_string()); let content = generate_todo_file(&empty_file); - fs::write(todo_path, content)?; + fs::write(todo_path, content) + .with_context(|| format!("Failed to write file: {}", todo_path.display()))?; } - let status = Command::new(&editor).arg(todo_path).status()?; + let status = Command::new(&editor) + .arg(todo_path) + .status() + .with_context(|| format!("Failed to run editor: {}", editor))?; if !status.success() { println!( "{}Editor exited with error code: {}{}", color::Fg(color::Red), - status.code().unwrap_or(-1), + status.code().unwrap_or(1), color::Fg(color::Reset) ); } Ok(()) -} \ No newline at end of file +} diff --git a/src/content.rs b/src/content.rs index 8b4c233..7500c88 100644 --- a/src/content.rs +++ b/src/content.rs @@ -112,4 +112,4 @@ pub fn archive_items_in_content( }; Ok((new_content, to_archive)) -} \ No newline at end of file +} diff --git a/src/main.rs b/src/main.rs index 37f482d..5e17e8d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,8 +3,8 @@ //! A command-line todo application that emphasizes adding items to the top //! and provides an interactive review mode for processing items. +use anyhow::{Context, Result}; use clap::{Parser, Subcommand}; -use std::io; use xdg::BaseDirectories; mod commands; @@ -74,42 +74,49 @@ enum Commands { /// Entry point that parses CLI arguments and dispatches to appropriate command handlers. /// Sets up XDG-compliant data directory paths and handles migration from plain text format. -fn main() -> io::Result<()> { +fn main() -> Result<()> { let cli = Cli::parse(); let base = BaseDirectories::with_prefix("ldr"); - + // Old plain text file paths let note_path = base .place_data_file("note.txt") - .expect("Failed to create data directory"); + .context("Failed to create data directory for note.txt")?; let archive_path = base .place_data_file("archive.txt") - .expect("Failed to create data directory"); + .context("Failed to create data directory for archive.txt")?; // New Markdown file paths let todo_md_path = base .place_data_file("todos.md") - .expect("Failed to create data directory"); + .context("Failed to create data directory for todos.md")?; let archive_md_path = base .place_data_file("archive.md") - .expect("Failed to create data directory"); + .context("Failed to create data directory for archive.md")?; // Check if migration is needed and perform it if migration::needs_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path) { - if let Err(e) = migration::perform_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path) { - eprintln!("Migration failed: {}", e); - return Err(io::Error::other(e)); - } + migration::perform_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path) + .map_err(|e| anyhow::anyhow!("Migration from plain text to Markdown failed: {}", e))?; } match cli.command { - Commands::Add { text, under } => commands::add_entry(&todo_md_path, &text, under), + Commands::Add { text, under } => { + commands::add_entry(&todo_md_path, &text, under).context("Failed to add entry")? + } Commands::Ls { num, all, filter } => { commands::list_note(&todo_md_path, num, all, filter.as_deref()) + .context("Failed to list notes")? } - Commands::Up { refs } => commands::prioritize_items(&todo_md_path, &refs), - Commands::Do { refs } => commands::archive_items(&todo_md_path, &archive_md_path, &refs), - Commands::Rm { refs } => commands::remove_items(&todo_md_path, &refs), - Commands::Edit => commands::edit_note(&todo_md_path), + Commands::Up { refs } => commands::prioritize_items(&todo_md_path, &refs) + .context("Failed to prioritize items")?, + Commands::Do { refs } => commands::archive_items(&todo_md_path, &archive_md_path, &refs) + .context("Failed to archive items")?, + Commands::Rm { refs } => { + commands::remove_items(&todo_md_path, &refs).context("Failed to remove items")? + } + Commands::Edit => commands::edit_note(&todo_md_path).context("Failed to edit note")?, } + + Ok(()) } diff --git a/src/markdown.rs b/src/markdown.rs index 88b5826..99424db 100644 --- a/src/markdown.rs +++ b/src/markdown.rs @@ -94,7 +94,10 @@ impl TaskRef { task_part.push(ch); } else if ch.is_ascii_lowercase() { if i == 0 { - return Err(format!("Task reference must start with a number: {}", input)); + return Err(format!( + "Task reference must start with a number: {}", + input + )); } if subtask_char.is_some() { return Err(format!("Multiple subtask letters not allowed: {}", input)); @@ -109,9 +112,24 @@ impl TaskRef { return Err(format!("No task number found: {}", input)); } - let task_index = task_part.parse::().map_err(|_| { - format!("Invalid task number: {}", task_part) - })? - 1; // Convert to 0-based + let task_num = task_part + .parse::() + .map_err(|_| format!("Invalid task number: {}", task_part))?; + + // Validate task number range + if task_num == 0 { + return Err("Task number must be at least 1".to_string()); + } + + const MAX_TASK_NUM: usize = 10000; + if task_num > MAX_TASK_NUM { + return Err(format!( + "Task number too large: {}. Maximum is {}", + task_num, MAX_TASK_NUM + )); + } + + let task_index = task_num - 1; // Convert to 0-based let subtask_index = subtask_char.map(|ch| (ch as usize) - ('a' as usize)); @@ -136,8 +154,9 @@ pub fn parse_todo_file(content: &str) -> Result { let mut todo_file = TodoFile::new("TODOs".to_string()); let mut current_task: Option = None; + let mut warned_about_deep_nesting = false; - for line in lines.iter() { + for (line_num, line) in lines.iter().enumerate() { let trimmed = line.trim(); if trimmed.is_empty() { @@ -156,7 +175,33 @@ pub fn parse_todo_file(content: &str) -> Result { else if trimmed.starts_with("##") { continue; } - // Handle subtasks - be flexible with indentation (2, 3, or 4 spaces, or tab) + // Check for deep nesting first - warn and convert to level 1 subtask + else if (line.starts_with(" ") || line.starts_with("\t\t")) + && (line.trim_start().starts_with("- ") + || line.trim_start().starts_with("* ") + || line.trim_start().starts_with("+ ")) + { + // This is a deeply nested item - warn once and treat as level 1 subtask + if !warned_about_deep_nesting { + eprintln!("Warning: Deep nesting detected (line {}). Converting to level 1 subtask. Only single-level subtasks are supported.", line_num + 1); + warned_about_deep_nesting = true; + } + + let item_text = line + .trim_start() + .strip_prefix("- ") + .or_else(|| line.trim_start().strip_prefix("* ")) + .or_else(|| line.trim_start().strip_prefix("+ ")) + .unwrap_or(line.trim_start()); + + if let Some(ref mut task) = current_task { + task.add_subtask(item_text.trim().to_string()); + } else { + // If no current task, treat as main task + current_task = Some(Task::new(item_text.trim().to_string())); + } + } + // Handle subtasks - be flexible with indentation (2, 3, or 4 spaces, or single tab) else if let Some(subtask_text) = line.strip_prefix(" - ") { if let Some(ref mut task) = current_task { task.add_subtask(subtask_text.trim().to_string()); @@ -172,14 +217,14 @@ pub fn parse_todo_file(content: &str) -> Result { current_task = Some(Task::new(subtask_text.trim().to_string())); } } else if let Some(subtask_text) = line.strip_prefix(" - ") { - // Handle 4-space indentation (but not deeper nesting) + // Handle 4-space indentation if let Some(ref mut task) = current_task { task.add_subtask(subtask_text.trim().to_string()); } else { current_task = Some(Task::new(subtask_text.trim().to_string())); } } else if let Some(subtask_text) = line.strip_prefix("\t- ") { - // Handle tab indentation + // Handle single tab indentation if let Some(ref mut task) = current_task { task.add_subtask(subtask_text.trim().to_string()); } else { @@ -202,18 +247,13 @@ pub fn parse_todo_file(content: &str) -> Result { current_task = Some(Task::new(task_text.trim().to_string())); } else if let Some(task_text) = trimmed.strip_prefix("+ ") { - // Handle plus bullet points + // Handle plus bullet points if let Some(task) = current_task.take() { todo_file.add_task(task); } current_task = Some(Task::new(task_text.trim().to_string())); } - // Check for overly deep nesting (5+ spaces or multiple tabs) - else if line.starts_with(" - ") || line.starts_with("\t\t") { - // Skip deep nesting but don't error - just ignore these lines - continue; - } // Handle non-markdown lines gracefully - ignore unknown formatting else if !trimmed.is_empty() { // If it looks like it might be a task without proper formatting, treat it as one @@ -275,26 +315,22 @@ impl ArchiveFile { pub fn add_items_for_today(&mut self, list_name: &str, tasks: Vec) { let today = chrono::Local::now().format("%Y-%m-%d").to_string(); - - // Find or create today's entry - let entry = self.entries.iter_mut() - .find(|e| e.date == today) - .map(|e| e as *mut ArchiveEntry); - if let Some(entry) = entry { - unsafe { - (*entry).lists - .entry(list_name.to_string()) - .or_insert_with(Vec::new) - .extend(tasks); - } + // Find today's entry position + let entry_pos = self.entries.iter().position(|e| e.date == today); + + if let Some(pos) = entry_pos { + // Entry exists, add tasks to it + self.entries[pos] + .lists + .entry(list_name.to_string()) + .or_default() + .extend(tasks); } else { + // Create new entry for today let mut lists = HashMap::new(); lists.insert(list_name.to_string(), tasks); - self.entries.insert(0, ArchiveEntry { - date: today, - lists, - }); + self.entries.insert(0, ArchiveEntry { date: today, lists }); } } } @@ -323,9 +359,10 @@ pub fn parse_archive_file(content: &str) -> Result { } else if let Some(date) = trimmed.strip_prefix("## ") { // Save previous task and entry if let (Some(mut entry), Some(task)) = (current_entry.take(), current_task.take()) { - entry.lists + entry + .lists .entry(current_list_name.clone()) - .or_insert_with(Vec::new) + .or_default() .push(task); archive.entries.push(entry); } else if let Some(entry) = current_entry.take() { @@ -339,20 +376,24 @@ pub fn parse_archive_file(content: &str) -> Result { current_list_name = "Default".to_string(); } else if let Some(list_name) = trimmed.strip_prefix("### ") { // Save previous task - if let (Some(ref mut entry), Some(task)) = (current_entry.as_mut(), current_task.take()) { - entry.lists + if let (Some(ref mut entry), Some(task)) = (current_entry.as_mut(), current_task.take()) + { + entry + .lists .entry(current_list_name.clone()) - .or_insert_with(Vec::new) + .or_default() .push(task); } current_list_name = list_name.to_string(); } else if let Some(task_text) = trimmed.strip_prefix("- ") { // Save previous task - if let (Some(ref mut entry), Some(task)) = (current_entry.as_mut(), current_task.take()) { - entry.lists + if let (Some(ref mut entry), Some(task)) = (current_entry.as_mut(), current_task.take()) + { + entry + .lists .entry(current_list_name.clone()) - .or_insert_with(Vec::new) + .or_default() .push(task); } @@ -379,10 +420,7 @@ pub fn parse_archive_file(content: &str) -> Result { // Save final task and entry if let Some(mut entry) = current_entry { if let Some(task) = current_task { - entry.lists - .entry(current_list_name) - .or_insert_with(Vec::new) - .push(task); + entry.lists.entry(current_list_name).or_default().push(task); } archive.entries.push(entry); } @@ -461,6 +499,11 @@ mod tests { assert!(TaskRef::parse("1A").is_err()); assert!(TaskRef::parse("1ab").is_err()); assert!(TaskRef::parse("1-2").is_err()); + + // Test validation edge cases + assert!(TaskRef::parse("0").is_err()); // Zero task number + assert!(TaskRef::parse("10001").is_err()); // Too large task number + assert!(TaskRef::parse("999999999999999999999").is_err()); // Integer overflow } #[test] @@ -485,7 +528,7 @@ mod tests { #[test] fn test_generate_todo_file() { let mut todo_file = TodoFile::new("TODOs".to_string()); - + let mut task = Task::new("Task with subtasks".to_string()); task.add_subtask("Subtask 1".to_string()); task.add_subtask("Subtask 2".to_string()); @@ -544,26 +587,26 @@ This is a code block let todo_file = parse_todo_file(content).unwrap(); assert_eq!(todo_file.title, "TODOs"); - + // Now all tasks are in the single task list assert_eq!(todo_file.tasks.len(), 6); // All tasks are top-level now (including code block content) - + // First task (dash) has no subtasks assert_eq!(todo_file.tasks[0].text, "Task with dash"); assert_eq!(todo_file.tasks[0].subtasks.len(), 0); - - // Second task (asterisk) has no subtasks + + // Second task (asterisk) has no subtasks assert_eq!(todo_file.tasks[1].text, "Task with asterisk"); assert_eq!(todo_file.tasks[1].subtasks.len(), 0); - + // Third task (plus) has all the subtasks (different indentation styles) assert_eq!(todo_file.tasks[2].text, "Task with plus"); assert_eq!(todo_file.tasks[2].subtasks.len(), 4); - + // Plain text task and normal task assert_eq!(todo_file.tasks[3].text, "Plain text task without bullet"); assert_eq!(todo_file.tasks[4].text, "Normal task"); assert_eq!(todo_file.tasks[5].text, "This is a code block"); // Comments are ignored but code block content is parsed } -} \ No newline at end of file +} diff --git a/src/migration.rs b/src/migration.rs index 937ec1d..26a753a 100644 --- a/src/migration.rs +++ b/src/migration.rs @@ -3,15 +3,20 @@ //! This module handles the one-time conversion from the old plain text format //! to the new Markdown format with Default list structure. -use crate::markdown::{Task, TodoFile, ArchiveFile}; +use crate::markdown::{ArchiveFile, Task, TodoFile}; use std::fs; use std::path::Path; /// Check if migration is needed (plain text files exist but Markdown files don't) -pub fn needs_migration(note_path: &Path, archive_path: &Path, todo_md_path: &Path, archive_md_path: &Path) -> bool { +pub fn needs_migration( + note_path: &Path, + archive_path: &Path, + todo_md_path: &Path, + archive_md_path: &Path, +) -> bool { let has_old_files = note_path.exists() || archive_path.exists(); let has_new_files = todo_md_path.exists() || archive_md_path.exists(); - + has_old_files && !has_new_files } @@ -21,8 +26,8 @@ pub fn migrate_note_file(note_path: &Path) -> Result { return Ok(create_empty_todo_file()); } - let content = fs::read_to_string(note_path) - .map_err(|e| format!("Failed to read note file: {}", e))?; + let content = + fs::read_to_string(note_path).map_err(|e| format!("Failed to read note file: {}", e))?; let mut todo_file = TodoFile::new("TODOs".to_string()); @@ -105,20 +110,22 @@ pub fn perform_migration( // Migrate note file let todo_file = migrate_note_file(note_path)?; let todo_content = crate::markdown::generate_todo_file(&todo_file); - + fs::write(todo_md_path, todo_content) .map_err(|e| format!("Failed to write todos.md: {}", e))?; - // Migrate archive file + // Migrate archive file let archive_file = migrate_archive_file(archive_path)?; let archive_content = crate::markdown::generate_archive_file(&archive_file); - + fs::write(archive_md_path, archive_content) .map_err(|e| format!("Failed to write archive.md: {}", e))?; // Count migrated items let todo_count = todo_file.task_count(); - let archive_count = archive_file.entries.first() + let archive_count = archive_file + .entries + .first() .and_then(|e| e.lists.get("Default")) .map(|tasks| tasks.len()) .unwrap_or(0); @@ -149,9 +156,9 @@ mod tests { fn test_migrate_note_file() { let content = "First task\nSecond task\nThird task\n"; let file = create_test_file_with_content(content); - + let result = migrate_note_file(file.path()).unwrap(); - + assert_eq!(result.title, "TODOs"); assert_eq!(result.tasks.len(), 3); assert_eq!(result.tasks[0].text, "First task"); @@ -163,7 +170,7 @@ mod tests { fn test_migrate_empty_note_file() { let file = create_test_file_with_content(""); let result = migrate_note_file(file.path()).unwrap(); - + assert_eq!(result.title, "TODOs"); assert_eq!(result.tasks.len(), 0); } @@ -172,12 +179,12 @@ mod tests { fn test_migrate_archive_file() { let content = "Completed task 1\nCompleted task 2\n"; let file = create_test_file_with_content(content); - + let result = migrate_archive_file(file.path()).unwrap(); - + assert_eq!(result.title, "Archive"); assert_eq!(result.entries.len(), 1); - + let entry = &result.entries[0]; let tasks = entry.lists.get("Default").unwrap(); assert_eq!(tasks.len(), 2); @@ -189,10 +196,10 @@ mod tests { fn test_migrate_nonexistent_files() { let temp_dir = tempfile::tempdir().unwrap(); let nonexistent = temp_dir.path().join("nonexistent.txt"); - + let todo_result = migrate_note_file(&nonexistent).unwrap(); assert!(todo_result.is_empty()); - + let archive_result = migrate_archive_file(&nonexistent).unwrap(); assert!(archive_result.entries.is_empty()); } @@ -206,18 +213,33 @@ mod tests { let archive_md_path = temp_dir.path().join("archive.md"); // No files exist - no migration needed - assert!(!needs_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path)); + assert!(!needs_migration( + ¬e_path, + &archive_path, + &todo_md_path, + &archive_md_path + )); // Create old file fs::write(¬e_path, "test").unwrap(); - + // Old file exists, no new files - migration needed - assert!(needs_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path)); + assert!(needs_migration( + ¬e_path, + &archive_path, + &todo_md_path, + &archive_md_path + )); // Create new file too fs::write(&todo_md_path, "test").unwrap(); - + // Both old and new files exist - no migration needed - assert!(!needs_migration(¬e_path, &archive_path, &todo_md_path, &archive_md_path)); + assert!(!needs_migration( + ¬e_path, + &archive_path, + &todo_md_path, + &archive_md_path + )); } -} \ No newline at end of file +} diff --git a/src/tests.rs b/src/tests.rs index 99d3825..f1431ff 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -254,7 +254,7 @@ mod list_tests { for task in tasks { content.push_str(&format!("- {}\n", task)); } - + let mut file = NamedTempFile::new().unwrap(); write!(file, "{}", content).unwrap(); file @@ -264,7 +264,7 @@ mod list_tests { #[test] fn test_list_with_all_flag() { let file = create_markdown_test_file(&[ - "item 1", "item 2", "item 3", "item 4", "item 5", "item 6" + "item 1", "item 2", "item 3", "item 4", "item 5", "item 6", ]); // Test that all=true shows all items regardless of num @@ -278,7 +278,7 @@ mod list_tests { #[test] fn test_list_without_all_flag() { let file = create_markdown_test_file(&[ - "item 1", "item 2", "item 3", "item 4", "item 5", "item 6" + "item 1", "item 2", "item 3", "item 4", "item 5", "item 6", ]); // Test that all=false respects num limit @@ -292,7 +292,10 @@ mod list_tests { #[test] fn test_list_with_filter_and_all() { let file = create_markdown_test_file(&[ - "read: book 1", "write: article", "read: book 2", "task: cleanup" + "read: book 1", + "write: article", + "read: book 2", + "task: cleanup", ]); // Test that all=true with filter shows all matching items @@ -479,14 +482,16 @@ mod remove_tests { for task in tasks { content.push_str(&format!("- {}\n", task)); } - + let mut file = NamedTempFile::new().unwrap(); write!(file, "{}", content).unwrap(); file } /// Helper function to create Markdown todo files with subtasks - fn create_markdown_test_file_with_subtasks(tasks_with_subtasks: &[(&str, &[&str])]) -> NamedTempFile { + fn create_markdown_test_file_with_subtasks( + tasks_with_subtasks: &[(&str, &[&str])], + ) -> NamedTempFile { let mut content = String::from("# TODOs\n\n"); for (task, subtasks) in tasks_with_subtasks { content.push_str(&format!("- {}\n", task)); @@ -494,7 +499,7 @@ mod remove_tests { content.push_str(&format!(" - {}\n", subtask)); } } - + let mut file = NamedTempFile::new().unwrap(); write!(file, "{}", content).unwrap(); file @@ -519,7 +524,8 @@ mod remove_tests { /// Tests removing multiple items #[test] fn test_remove_multiple_items() { - let file = create_markdown_test_file(&["First item", "Second item", "Third item", "Fourth item"]); + let file = + create_markdown_test_file(&["First item", "Second item", "Third item", "Fourth item"]); let result = std::panic::catch_unwind(|| { remove_items(file.path(), &["1".to_string(), "3".to_string()]).unwrap(); @@ -570,7 +576,11 @@ mod remove_tests { let file = create_markdown_test_file(&["First item", "Second item", "Third item"]); let result = std::panic::catch_unwind(|| { - remove_items(file.path(), &["2".to_string(), "2".to_string(), "1".to_string()]).unwrap(); + remove_items( + file.path(), + &["2".to_string(), "2".to_string(), "1".to_string()], + ) + .unwrap(); }); assert!(result.is_ok()); @@ -636,17 +646,22 @@ mod remove_tests { #[test] fn test_auto_complete_parent_when_all_subtasks_done() { let file = create_markdown_test_file_with_subtasks(&[ - ("Task A", &[]), // Task with no subtasks - ("Task B", &["Subtask 1", "Subtask 2"]), // Task with 2 subtasks - ("Task C", &["Another subtask"]), // Task with 1 subtask + ("Task A", &[]), // Task with no subtasks + ("Task B", &["Subtask 1", "Subtask 2"]), // Task with 2 subtasks + ("Task C", &["Another subtask"]), // Task with 1 subtask ]); - + let temp_dir = tempfile::tempdir().unwrap(); let archive_path = temp_dir.path().join("archive.md"); // Archive both subtasks of Task B (2a and 2b) let result = std::panic::catch_unwind(|| { - archive_items(file.path(), &archive_path, &["2a".to_string(), "2b".to_string()]).unwrap(); + archive_items( + file.path(), + &archive_path, + &["2a".to_string(), "2b".to_string()], + ) + .unwrap(); }); assert!(result.is_ok()); @@ -658,7 +673,7 @@ mod remove_tests { // Verify both subtasks AND the parent task are in the archive assert!(archive_path.exists()); let archive_content = std::fs::read_to_string(&archive_path).unwrap(); - + // Should contain both subtasks as individual items and the parent task assert!(archive_content.contains("- Subtask 1")); assert!(archive_content.contains("- Subtask 2")); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 1a1369d..f99afc0 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1,5 +1,5 @@ //! Integration tests for the LDR CLI application -//! +//! //! These tests verify the entire application works correctly by running actual CLI commands //! with temporary data directories. Tests cover all functionality and edge cases according to POLS. @@ -21,32 +21,35 @@ impl TestEnv { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let data_dir = temp_dir.path().join("ldr"); fs::create_dir_all(&data_dir).expect("Failed to create ldr data directory"); - + // Build the binary if it doesn't exist or is out of date let binary_path = Self::ensure_binary_built(); - + Self { _temp_dir: temp_dir, data_dir, binary_path, } } - + fn ensure_binary_built() -> PathBuf { let output = Command::new("cargo") .args(&["build", "--bin", "ldr"]) .output() .expect("Failed to build ldr binary"); - + if !output.status.success() { - panic!("Failed to build binary: {}", String::from_utf8_lossy(&output.stderr)); + panic!( + "Failed to build binary: {}", + String::from_utf8_lossy(&output.stderr) + ); } - + let mut binary_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); binary_path.push("target/debug/ldr"); binary_path } - + /// Run ldr command with arguments in the isolated environment fn run_ldr(&self, args: &[&str]) -> CommandResult { let output = Command::new(&self.binary_path) @@ -56,29 +59,29 @@ impl TestEnv { .stderr(Stdio::piped()) .output() .expect("Failed to execute ldr command"); - + CommandResult { status: output.status.code().unwrap_or(-1), stdout: String::from_utf8_lossy(&output.stdout).to_string(), stderr: String::from_utf8_lossy(&output.stderr).to_string(), } } - + /// Get the path to the todos.md file fn todos_path(&self) -> PathBuf { self.data_dir.join("ldr/todos.md") } - + /// Get the path to the archive.md file fn archive_path(&self) -> PathBuf { self.data_dir.join("ldr/archive.md") } - + /// Read the contents of todos.md fn read_todos(&self) -> String { fs::read_to_string(self.todos_path()).unwrap_or_default() } - + /// Read the contents of archive.md fn read_archive(&self) -> String { fs::read_to_string(self.archive_path()).unwrap_or_default() @@ -95,13 +98,19 @@ struct CommandResult { impl CommandResult { fn assert_success(&self) { if self.status != 0 { - panic!("Command failed with status {}: stderr: {}", self.status, self.stderr); + panic!( + "Command failed with status {}: stderr: {}", + self.status, self.stderr + ); } } - + fn assert_failure(&self) { // For LDR, failure is indicated by error messages in stdout, not exit code - if !self.stdout.contains("Invalid") && !self.stderr.contains("Error") && !self.stdout.is_empty() { + if !self.stdout.contains("Invalid") + && !self.stderr.contains("Error") + && !self.stdout.is_empty() + { panic!("Command unexpectedly succeeded: stdout: {}", self.stdout); } } @@ -110,49 +119,49 @@ impl CommandResult { #[cfg(test)] mod basic_operations { use super::*; - + #[test] fn test_add_and_list_basic() { let env = TestEnv::new(); - + // Add first item let result = env.run_ldr(&["add", "First task"]); result.assert_success(); assert!(result.stdout.contains("✓ Added: First task")); - + // Add second item (should go to top) let result = env.run_ldr(&["add", "Second task"]); result.assert_success(); - + // List items let result = env.run_ldr(&["ls"]); result.assert_success(); - + // Check that second task appears first (POLS: newest items at top) // Note: Output includes ANSI color codes, so we check for task text assert!(result.stdout.contains("1. Second task")); assert!(result.stdout.contains("2. First task")); } - + #[test] fn test_empty_list() { let env = TestEnv::new(); - + let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("No notes yet.")); } - + #[test] fn test_list_with_limits() { let env = TestEnv::new(); - + // Add 10 items for i in 1..=10 { let result = env.run_ldr(&["add", &format!("Task {}", i)]); result.assert_success(); } - + // Test default limit (5) let result = env.run_ldr(&["ls"]); result.assert_success(); @@ -160,13 +169,13 @@ mod basic_operations { // Should show 5 items plus "... and X more items" line assert!(line_count >= 5); assert!(result.stdout.contains("and 5 more items")); - + // Test custom limit let result = env.run_ldr(&["ls", "-n", "3"]); result.assert_success(); let lines: Vec<&str> = result.stdout.lines().collect(); assert!(lines.len() >= 3); - + // Test show all let result = env.run_ldr(&["ls", "--all"]); result.assert_success(); @@ -177,60 +186,62 @@ mod basic_operations { #[cfg(test)] mod subtask_operations { use super::*; - + #[test] fn test_add_subtask() { let env = TestEnv::new(); - + // Add main task let result = env.run_ldr(&["add", "Main task"]); result.assert_success(); - + // Add subtask let result = env.run_ldr(&["add", "Subtask A", "--under", "1"]); result.assert_success(); - assert!(result.stdout.contains("✓ Added subtask to task 1: Subtask A")); - + assert!(result + .stdout + .contains("✓ Added subtask to task 1: Subtask A")); + // Add another subtask let result = env.run_ldr(&["add", "Subtask B", "--under", "1"]); result.assert_success(); - + // List and verify structure let result = env.run_ldr(&["ls"]); result.assert_success(); - + // Verify main task and subtasks appear in order added assert!(result.stdout.contains("1. Main task")); assert!(result.stdout.contains("a. Subtask A")); // First subtask assert!(result.stdout.contains("b. Subtask B")); // Second subtask } - + #[test] fn test_add_subtask_invalid_parent() { let env = TestEnv::new(); - + // Try to add subtask to non-existent task let result = env.run_ldr(&["add", "Orphan subtask", "--under", "1"]); result.assert_failure(); assert!(result.stderr.contains("Invalid task number: 1")); } - + #[test] fn test_subtask_numbering() { let env = TestEnv::new(); - + // Add main task env.run_ldr(&["add", "Task with many subtasks"]); - + // Add multiple subtasks for i in 1..=5 { let result = env.run_ldr(&["add", &format!("Subtask {}", i), "--under", "1"]); result.assert_success(); } - + let result = env.run_ldr(&["ls", "--all"]); result.assert_success(); - + // Verify letter sequence (in order added: a, b, c, d, e) assert!(result.stdout.contains("a. Subtask 1")); assert!(result.stdout.contains("b. Subtask 2")); @@ -243,21 +254,21 @@ mod subtask_operations { #[cfg(test)] mod prioritization_tests { use super::*; - + #[test] fn test_prioritize_basic() { let env = TestEnv::new(); - + // Add tasks env.run_ldr(&["add", "Task A"]); env.run_ldr(&["add", "Task B"]); env.run_ldr(&["add", "Task C"]); - + // Prioritize task 3 (Task A) let result = env.run_ldr(&["up", "3"]); result.assert_success(); assert!(result.stdout.contains("✓ Prioritized 1 task(s)")); - + // Verify new order let result = env.run_ldr(&["ls"]); result.assert_success(); @@ -266,21 +277,21 @@ mod prioritization_tests { assert!(lines[1].contains("2. Task C")); assert!(lines[2].contains("3. Task B")); } - + #[test] fn test_prioritize_multiple() { let env = TestEnv::new(); - + // Add tasks env.run_ldr(&["add", "Task A"]); env.run_ldr(&["add", "Task B"]); env.run_ldr(&["add", "Task C"]); env.run_ldr(&["add", "Task D"]); - + // Prioritize tasks 4, 2 (in that order) let result = env.run_ldr(&["up", "4", "2"]); result.assert_success(); - + // Verify command-line order preserved (POLS) let result = env.run_ldr(&["ls"]); result.assert_success(); @@ -290,38 +301,38 @@ mod prioritization_tests { assert!(lines[2].contains("3. Task D")); // Remaining items assert!(lines[3].contains("4. Task B")); } - + #[test] fn test_prioritize_subtask_moves_parent() { let env = TestEnv::new(); - + // Add tasks with subtasks env.run_ldr(&["add", "Task A"]); env.run_ldr(&["add", "Subtask A1", "--under", "1"]); env.run_ldr(&["add", "Task B"]); env.run_ldr(&["add", "Subtask B1", "--under", "2"]); - + // Prioritize subtask - should move entire parent task let result = env.run_ldr(&["up", "2a"]); // Task A is now at position 2 result.assert_success(); - + // Verify parent task moved (POLS: subtask reference affects parent) let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("1. Task A")); // Parent moved to top assert!(result.stdout.contains("a. Subtask A1")); } - + #[test] fn test_prioritize_invalid_reference() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task A"]); - + let result = env.run_ldr(&["up", "5"]); result.assert_failure(); - - let result = env.run_ldr(&["up", "1z"]); // Invalid subtask + + let result = env.run_ldr(&["up", "1z"]); // Invalid subtask result.assert_failure(); } } @@ -329,92 +340,92 @@ mod prioritization_tests { #[cfg(test)] mod archiving_tests { use super::*; - + #[test] fn test_archive_single_task() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task to complete"]); - + let result = env.run_ldr(&["do", "1"]); result.assert_success(); assert!(result.stdout.contains("✓ Archived 1 item(s)")); - + // Verify task removed from todos let todos = env.read_todos(); assert!(!todos.contains("Task to complete")); - + // Verify task in archive let archive = env.read_archive(); assert!(archive.contains("Task to complete")); assert!(archive.contains(&chrono::Local::now().format("%Y-%m-%d").to_string())); } - + #[test] fn test_archive_subtask() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Main task"]); env.run_ldr(&["add", "Subtask A", "--under", "1"]); env.run_ldr(&["add", "Subtask B", "--under", "1"]); - + // Archive one subtask let result = env.run_ldr(&["do", "1a"]); result.assert_success(); - + // Main task should still exist with remaining subtask let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("Main task")); assert!(result.stdout.contains("Subtask B")); assert!(!result.stdout.contains("Subtask A")); - + // Archive should contain the subtask let archive = env.read_archive(); assert!(archive.contains("Subtask A")); } - + #[test] fn test_auto_complete_parent_when_all_subtasks_done() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Main task"]); env.run_ldr(&["add", "Subtask A", "--under", "1"]); env.run_ldr(&["add", "Subtask B", "--under", "1"]); - + // Archive all subtasks let result = env.run_ldr(&["do", "1a", "1b"]); result.assert_success(); - + // Main task should be auto-completed (POLS: completing all subtasks completes parent) let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(!result.stdout.contains("Main task")); - + // Archive should contain subtasks AND parent task let archive = env.read_archive(); assert!(archive.contains("Subtask A")); assert!(archive.contains("Subtask B")); assert!(archive.contains("Main task")); } - + #[test] fn test_archive_whole_task_with_subtasks() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Complete project"]); env.run_ldr(&["add", "Write code", "--under", "1"]); env.run_ldr(&["add", "Write tests", "--under", "1"]); - + // Archive entire task (should include all subtasks) let result = env.run_ldr(&["do", "1"]); result.assert_success(); - + // Nothing should remain in todos let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("No notes yet")); - + // Archive should contain complete task structure let archive = env.read_archive(); assert!(archive.contains("Complete project")); @@ -426,52 +437,52 @@ mod archiving_tests { #[cfg(test)] mod removal_tests { use super::*; - + #[test] fn test_remove_vs_archive() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task to remove"]); env.run_ldr(&["add", "Task to archive"]); - + // Remove one task let result = env.run_ldr(&["rm", "2"]); result.assert_success(); - + // Archive another task let result = env.run_ldr(&["do", "1"]); result.assert_success(); - + // Only archived task should be in archive let archive = env.read_archive(); assert!(archive.contains("Task to archive")); assert!(!archive.contains("Task to remove")); - + // Both should be gone from todos let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("No notes yet")); } - + #[test] fn test_remove_subtask() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Main task"]); env.run_ldr(&["add", "Keep this", "--under", "1"]); env.run_ldr(&["add", "Remove this", "--under", "1"]); - + // Remove second subtask (1b) let result = env.run_ldr(&["rm", "1b"]); result.assert_success(); - + // Main task should remain with first subtask only let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("Main task")); assert!(result.stdout.contains("Keep this")); assert!(!result.stdout.contains("Remove this")); - + // Archive should be empty let archive = env.read_archive(); assert!(!archive.contains("Remove this")); @@ -481,16 +492,16 @@ mod removal_tests { #[cfg(test)] mod filtering_tests { use super::*; - + #[test] fn test_filtering_basic() { let env = TestEnv::new(); - + env.run_ldr(&["add", "read: Book about Rust"]); env.run_ldr(&["add", "write: Article about Go"]); env.run_ldr(&["add", "read: Documentation"]); env.run_ldr(&["add", "@work: Review PR"]); - + // Filter by "read:" let result = env.run_ldr(&["ls", "read:"]); result.assert_success(); @@ -498,82 +509,88 @@ mod filtering_tests { assert!(result.stdout.contains("read: Documentation")); assert!(!result.stdout.contains("write: Article")); assert!(!result.stdout.contains("@work: Review")); - + // Filter by "@work" let result = env.run_ldr(&["ls", "@work"]); result.assert_success(); assert!(result.stdout.contains("@work: Review PR")); assert!(!result.stdout.contains("read:")); } - + #[test] fn test_filtering_case_insensitive() { let env = TestEnv::new(); - + env.run_ldr(&["add", "READ: Book"]); env.run_ldr(&["add", "read: Article"]); env.run_ldr(&["add", "ReAd: Mixed"]); - + let result = env.run_ldr(&["ls", "read:"]); result.assert_success(); assert!(result.stdout.contains("READ: Book")); assert!(result.stdout.contains("read: Article")); assert!(result.stdout.contains("ReAd: Mixed")); } - + #[test] fn test_filtering_with_limits() { let env = TestEnv::new(); - + // Add many matching items for i in 1..=10 { env.run_ldr(&["add", &format!("read: Book {}", i)]); } env.run_ldr(&["add", "write: Article"]); - + // Test filtering respects limits let result = env.run_ldr(&["ls", "-n", "3", "read:"]); result.assert_success(); - let matching_lines: Vec<&str> = result.stdout.lines() + let matching_lines: Vec<&str> = result + .stdout + .lines() .filter(|line| line.contains("read:")) .collect(); assert!(matching_lines.len() <= 3); - + // Test filtering with --all let result = env.run_ldr(&["ls", "--all", "read:"]); result.assert_success(); - let matching_lines: Vec<&str> = result.stdout.lines() + let matching_lines: Vec<&str> = result + .stdout + .lines() .filter(|line| line.contains("read:")) .collect(); assert_eq!(matching_lines.len(), 10); } - + #[test] fn test_filtering_no_matches() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task A"]); env.run_ldr(&["add", "Task B"]); - + let result = env.run_ldr(&["ls", "nonexistent"]); result.assert_success(); - assert!(result.stdout.contains("No items found matching filter: \"nonexistent\"")); + assert!(result + .stdout + .contains("No items found matching filter: \"nonexistent\"")); } } #[cfg(test)] mod error_handling { use super::*; - + #[test] fn test_invalid_task_references() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task A"]); - + // Test various invalid references let invalid_refs = vec!["0", "99", "1z", "a1", "1ab", "1-2", ""]; - + for invalid_ref in invalid_refs { if !invalid_ref.is_empty() { let result = env.run_ldr(&["up", invalid_ref]); @@ -581,37 +598,37 @@ mod error_handling { } } } - + #[test] fn test_operations_on_empty_file() { let env = TestEnv::new(); - + // Try operations on empty file let result = env.run_ldr(&["up", "1"]); result.assert_success(); // Should handle gracefully assert!(result.stdout.contains("No notes found")); - + let result = env.run_ldr(&["do", "1"]); result.assert_success(); assert!(result.stdout.contains("No notes found")); - + let result = env.run_ldr(&["rm", "1"]); result.assert_success(); } - + #[test] fn test_subtask_references_without_parent() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Task without subtasks"]); - + // Try to reference non-existent subtask let result = env.run_ldr(&["up", "1a"]); result.assert_failure(); - - let result = env.run_ldr(&["do", "1a"]); + + let result = env.run_ldr(&["do", "1a"]); result.assert_failure(); - + let result = env.run_ldr(&["rm", "1a"]); result.assert_failure(); } @@ -620,34 +637,34 @@ mod error_handling { #[cfg(test)] mod edit_functionality { use super::*; - + #[test] fn test_edit_creates_file_if_not_exists() { let env = TestEnv::new(); - + // Set EDITOR to a command that just touches the file and exits let result = Command::new(&env.binary_path) .args(&["edit"]) .env("XDG_DATA_HOME", &env.data_dir) - .env("EDITOR", "touch") // Will just touch the file + .env("EDITOR", "touch") // Will just touch the file .output() .expect("Failed to execute edit command"); - + // Should succeed (touch command succeeds) assert_eq!(result.status.code().unwrap_or(-1), 0); - + // File should exist with proper structure let todos = env.read_todos(); assert!(todos.contains("# TODOs")); } - + #[test] fn test_edit_aliases() { let env = TestEnv::new(); - + // Test all aliases work by using echo to verify they're called let aliases = vec!["edit", "e", "scan", "s", "review", "r"]; - + for alias in aliases { let result = Command::new(&env.binary_path) .args(&[alias]) @@ -655,11 +672,16 @@ mod edit_functionality { .env("EDITOR", "/bin/echo") .output() .expect(&format!("Failed to execute {} command", alias)); - + // Should succeed and echo the file path assert_eq!(result.status.code().unwrap_or(-1), 0); let stdout = String::from_utf8_lossy(&result.stdout); - assert!(stdout.contains("todos.md"), "Alias '{}' didn't work: {}", alias, stdout); + assert!( + stdout.contains("todos.md"), + "Alias '{}' didn't work: {}", + alias, + stdout + ); } } } @@ -667,62 +689,62 @@ mod edit_functionality { #[cfg(test)] mod migration_tests { use super::*; - + #[test] fn test_migration_from_plain_text() { let env = TestEnv::new(); - + // Create old-style plain text files let old_note_path = env.data_dir.join("ldr/note.txt"); let old_archive_path = env.data_dir.join("ldr/archive.txt"); - + fs::create_dir_all(old_note_path.parent().unwrap()).unwrap(); - + // Write old-style content fs::write(&old_note_path, "Task A\nTask B with details\nTask C\n").unwrap(); fs::write(&old_archive_path, "Completed task 1\nCompleted task 2\n").unwrap(); - + // Run any command to trigger migration let result = env.run_ldr(&["ls"]); result.assert_success(); - + // Check that markdown files were created assert!(env.todos_path().exists()); assert!(env.archive_path().exists()); - + // Verify content was migrated correctly let todos = env.read_todos(); assert!(todos.contains("# TODOs")); assert!(todos.contains("- Task A")); assert!(todos.contains("- Task B with details")); assert!(todos.contains("- Task C")); - + let archive = env.read_archive(); assert!(archive.contains("# Archive")); assert!(archive.contains("- Completed task 1")); assert!(archive.contains("- Completed task 2")); - + // Verify old files were backed up (not deleted) assert!(old_note_path.with_extension("txt.bak").exists()); assert!(old_archive_path.with_extension("txt.bak").exists()); } - + #[test] fn test_no_migration_when_markdown_exists() { let env = TestEnv::new(); - + // Create markdown file first fs::create_dir_all(env.todos_path().parent().unwrap()).unwrap(); fs::write(&env.todos_path(), "# TODOs\n\n- Existing task\n").unwrap(); - + // Create old-style file let old_note_path = env.data_dir.join("ldr/note.txt"); fs::write(&old_note_path, "Old task\n").unwrap(); - + // Run command let result = env.run_ldr(&["ls"]); result.assert_success(); - + // Verify markdown file unchanged (no migration happened) let todos = env.read_todos(); assert!(todos.contains("Existing task")); @@ -733,34 +755,34 @@ mod migration_tests { #[cfg(test)] mod pols_compliance { use super::*; - + #[test] fn test_newest_items_at_top() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Old task"]); std::thread::sleep(std::time::Duration::from_millis(10)); env.run_ldr(&["add", "New task"]); - + let result = env.run_ldr(&["ls"]); result.assert_success(); - + assert!(result.stdout.contains("1. New task")); assert!(result.stdout.contains("2. Old task")); } - + #[test] fn test_prioritization_order_predictable() { let env = TestEnv::new(); - + env.run_ldr(&["add", "A"]); env.run_ldr(&["add", "B"]); env.run_ldr(&["add", "C"]); env.run_ldr(&["add", "D"]); - + // Test prioritization is predictable env.run_ldr(&["up", "4", "2"]); // Move A (4th) and C (2nd) to top - + let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("1. A")); @@ -768,42 +790,42 @@ mod pols_compliance { assert!(result.stdout.contains("3. D")); assert!(result.stdout.contains("4. B")); } - - #[test] + + #[test] fn test_subtask_operations_affect_parent_logically() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Project A"]); env.run_ldr(&["add", "Task 1", "--under", "1"]); env.run_ldr(&["add", "Project B"]); - + // Prioritizing subtask should move parent project env.run_ldr(&["up", "2a"]); // Project A is now at position 2 - + let result = env.run_ldr(&["ls"]); result.assert_success(); // Project A should be at top (moved because of subtask reference) assert!(result.stdout.contains("1. Project A")); assert!(result.stdout.contains("a. Task 1")); } - + #[test] fn test_completion_cascades_logically() { let env = TestEnv::new(); - + env.run_ldr(&["add", "Big project"]); env.run_ldr(&["add", "Only subtask", "--under", "1"]); - + // Completing the only subtask should complete parent env.run_ldr(&["do", "1a"]); - + let result = env.run_ldr(&["ls"]); result.assert_success(); assert!(result.stdout.contains("No notes yet")); - + // Both should be in archive let archive = env.read_archive(); assert!(archive.contains("Only subtask")); assert!(archive.contains("Big project")); } -} \ No newline at end of file +} diff --git a/tests/validation_tests.rs b/tests/validation_tests.rs new file mode 100644 index 0000000..edadaab --- /dev/null +++ b/tests/validation_tests.rs @@ -0,0 +1,130 @@ +use std::process::Command; +use tempfile::TempDir; + +fn run_ldr(dir: &TempDir, args: &[&str]) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_ldr")) + .env("XDG_DATA_HOME", dir.path()) + .args(args) + .output() + .expect("Failed to run ldr") +} + +#[test] +fn test_empty_task_rejected() { + let dir = TempDir::new().unwrap(); + + // Try to add empty task + let output = run_ldr(&dir, &["add", ""]); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("Cannot add empty task")); + + // Try to add whitespace-only task + let output = run_ldr(&dir, &["add", " "]); + assert!(!output.status.success()); +} + +#[test] +fn test_task_length_limit() { + let dir = TempDir::new().unwrap(); + + // Create a very long task (over 500 chars) + let long_text = "x".repeat(501); + let output = run_ldr(&dir, &["add", &long_text]); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("Task text too long")); + assert!(stderr.contains("Maximum length is 500")); +} + +#[test] +fn test_max_subtasks_limit() { + let dir = TempDir::new().unwrap(); + + // Add a main task + run_ldr(&dir, &["add", "Main task"]); + + // Add 26 subtasks (a-z) + for i in 0..26 { + let subtask = format!("Subtask {}", i); + let output = run_ldr(&dir, &["add", &subtask, "--under", "1"]); + assert!(output.status.success()); + } + + // Try to add 27th subtask - should fail + let output = run_ldr(&dir, &["add", "One too many", "--under", "1"]); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("already has maximum number of subtasks")); +} + +#[test] +fn test_invalid_task_references() { + let dir = TempDir::new().unwrap(); + + // Add some tasks + run_ldr(&dir, &["add", "Task 1"]); + run_ldr(&dir, &["add", "Task 2"]); + + // Test zero task number - should print error but return success + let output = run_ldr(&dir, &["up", "0"]); + // Check that error message was printed even if exit code is 0 + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Invalid task reference") + || stdout.contains("Task number must be at least 1") + ); + + // Test too large task number + let output = run_ldr(&dir, &["up", "999"]); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Invalid task number")); + + // Test invalid formats + let output = run_ldr(&dir, &["up", "1A"]); // uppercase letter + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Invalid") || stdout.contains("uppercase")); + + let output = run_ldr(&dir, &["up", "abc"]); // no number + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Invalid") || stdout.contains("must start with a number")); +} + +#[test] +fn test_subtask_under_invalid_parent() { + let dir = TempDir::new().unwrap(); + + // Try to add subtask under non-existent task + let output = run_ldr(&dir, &["add", "Subtask", "--under", "1"]); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("Invalid task number")); +} + +#[test] +fn test_deep_nesting_prevention() { + let dir = TempDir::new().unwrap(); + + // Create a markdown file with deep nesting + // ldr stores files in a subdirectory + std::fs::create_dir_all(dir.path().join("ldr")).unwrap(); + let todos_path = dir.path().join("ldr").join("todos.md"); + let content = r#"# TODOs + +- Task 1 + - Subtask 1 + - Deep subtask (should be flattened) + - Very deep subtask (should be flattened) +"#; + std::fs::write(&todos_path, content).unwrap(); + + // List tasks - deep nesting should be handled silently now + // (warnings are printed during parse but list should work) + let output = run_ldr(&dir, &["ls", "-a"]); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + + // The deep subtasks should appear as regular subtasks (flattened) + // We should see the subtasks in the output + assert!(stdout.contains("Subtask 1") || stdout.contains("a.")); +}