diff --git a/crates/lash-core/src/dependency/blocker_analyzer.rs b/crates/lash-core/src/dependency/blocker_analyzer.rs index 68d7cb1..aff1544 100644 --- a/crates/lash-core/src/dependency/blocker_analyzer.rs +++ b/crates/lash-core/src/dependency/blocker_analyzer.rs @@ -1044,4 +1044,129 @@ mod tests { let blockers = analyzer.find_blockers("test#task1"); assert_eq!(blockers.len(), 0); } + + /// Walking a chain past its second link. + /// + /// `test_find_blocker_chains` stops at task1 → task2 → task3, which is one + /// hop: the walk pushes task3 and then exits because task3 is not itself + /// blocked. The loop's own "did I find anything this pass?" guard is never + /// what ends it, so breaking on that guard's inverse produced the same + /// two-element chain and nothing failed. A fourth node forces a second + /// hop, which is the first case that distinguishes them. + #[test] + fn test_blocker_chain_follows_more_than_one_hop() { + let mut graph = DependencyGraph::new(); + for (id, title) in [ + ("test#task1", "Task 1"), + ("test#task2", "Task 2"), + ("test#task3", "Task 3"), + ("test#task4", "Task 4"), + ] { + graph.add_node( + id.to_string(), + create_node(title, TaskStatus::Open, "test", 0), + ); + } + for (from, to) in [ + ("test#task1", "test#task2"), + ("test#task2", "test#task3"), + ("test#task3", "test#task4"), + ] { + graph.add_edge( + from.to_string(), + to.to_string(), + EdgeData::new(DependencyKind::ExplicitId, None), + ); + } + + let computer = StatusComputer::new(&graph); + let statuses = computer.compute_all(); + let analyzer = BlockerAnalyzer::new(&graph, &statuses); + + let chains = analyzer.find_blocker_chains("test#task1"); + assert_eq!(chains.len(), 1); + + let chain = &chains[0]; + assert_eq!( + chain.len(), + 3, + "the walk should follow task2 → task3 → task4, not stop after the first hop" + ); + assert_eq!(chain.direct_blocker().unwrap().task_id, "test#task2"); + assert_eq!(chain.root_blocker().unwrap().task_id, "test#task4"); + } + + fn blocker_with_status(task_id: &str, status: ComputedStatus) -> BlockerInfo { + BlockerInfo::new( + task_id.to_string(), + format!("Title of {task_id}"), + "test".to_string(), + 0, + DependencyKind::ExplicitId, + status, + ) + } + + /// `BlockerReport::format` is rendered for the user but only its blocker + /// counts were asserted, so the per-blocker status word and the chains + /// section were unpinned. + #[test] + fn test_report_format_labels_each_blocker_status() { + let blocked = blocker_with_status("test#blocked", ComputedStatus::Blocked(Vec::new())); + let incomplete = blocker_with_status("test#incomplete", ComputedStatus::Incomplete); + let report = BlockerReport::new( + "test#task1".to_string(), + "Task 1".to_string(), + vec![blocked.clone(), incomplete.clone()], + vec![BlockerChain::new( + "test#task1".to_string(), + vec![blocked, incomplete.clone()], + )], + vec![incomplete], + Vec::new(), + ); + + let output = report.format(); + + assert!( + output.contains("[blocked]"), + "a blocker that is itself blocked should be labelled, got:\n{output}" + ); + assert!( + output.contains("[incomplete]"), + "a root blocker should be labelled incomplete, got:\n{output}" + ); + assert!( + output.contains("Blocker Chains:"), + "a report carrying chains should show the chains section, got:\n{output}" + ); + } + + /// The other side of the chains-section condition: with no chains the + /// heading must not appear above an empty list. + #[test] + fn test_report_format_omits_chains_section_when_there_are_none() { + let report = BlockerReport::new( + "test#task1".to_string(), + "Task 1".to_string(), + vec![blocker_with_status( + "test#incomplete", + ComputedStatus::Incomplete, + )], + Vec::new(), + Vec::new(), + Vec::new(), + ); + + let output = report.format(); + + assert!( + !output.contains("Blocker Chains:"), + "no chains means no chains section, got:\n{output}" + ); + assert!( + output.contains("All Blockers:"), + "the blocker list itself should still render, got:\n{output}" + ); + } } diff --git a/crates/lash-core/src/formatter/mod.rs b/crates/lash-core/src/formatter/mod.rs index 6fdb79f..7ee5f05 100644 --- a/crates/lash-core/src/formatter/mod.rs +++ b/crates/lash-core/src/formatter/mod.rs @@ -1061,4 +1061,84 @@ mod tests { // Multiple blank lines collapsed assert!(!result.contains("\n\n\n\n")); } + + /// Format `source` with `normalize_whitespace` set either way. + /// + /// The parse is what production does — `format_file_in_place` parses the + /// same source it hands the formatter — so these tests exercise the real + /// path rather than a hand-built `TaskFile`. + fn format_with_normalize(source: &str, normalize: bool) -> String { + let config = LashConfig::default(); + let file = + crate::parser::parse_file_from_string(source, &config).expect("fixture should parse"); + let options = FormatOptions { + normalize_whitespace: normalize, + ..FormatOptions::default() + }; + Formatter::new(config, options) + .format_file(source, &file) + .expect("formatting should succeed") + } + + /// `test_whitespace_normalization` above calls `normalize_whitespace` + /// directly, so nothing pinned the flag that decides whether it runs at + /// all: inverting it left every test passing. This asserts both settings + /// produce their own result. + #[test] + fn normalize_whitespace_option_decides_whether_blank_runs_collapse() { + let source = "# Demo\n\n@id: demo\n\n## Notes\n\nalpha\n\n\n\n\nbeta\n\n## Tasks\n\n- [ ] First task\n"; + + let collapsed = format_with_normalize(source, true); + assert!( + !collapsed.contains("\n\n\n\n"), + "normalize_whitespace = true should collapse the blank run, got: {collapsed:?}" + ); + + let preserved = format_with_normalize(source, false); + assert!( + preserved.contains("\n\n\n\n"), + "normalize_whitespace = false should leave the blank run alone, got: {preserved:?}" + ); + } + + /// The trailing-newline branch only runs when the output actually ends in + /// a blank line, which `normalize_whitespace` otherwise hides by stripping + /// trailing blanks first. With it off, the generated tasks section leaves a + /// blank line at EOF and this is the only thing that trims it. + #[test] + fn format_ends_file_with_exactly_one_newline() { + let source = "# Demo\n\n@id: demo\n\n## Tasks\n\n- [ ] First task\n"; + + let formatted = format_with_normalize(source, false); + + assert!( + formatted.ends_with('\n'), + "formatted file should end with a newline, got: {formatted:?}" + ); + assert!( + !formatted.ends_with("\n\n"), + "formatted file should not end with a blank line, got: {formatted:?}" + ); + } + + /// A file with no `## Tasks` heading gets one synthesized. The description + /// must not be synthesized alongside it when the source already had a + /// `## Description` section — the walk copied that through already, so + /// emitting it again duplicates it. + #[test] + fn synthesized_tasks_section_does_not_duplicate_an_existing_description() { + let source = "# Demo\n\n@id: demo\n\n## Description\n\nProse that belongs to the file.\n"; + + let formatted = format_with_normalize(source, true); + + assert_eq!( + formatted.matches("Prose that belongs to the file.").count(), + 1, + "description should appear exactly once, got: {formatted:?}" + ); + assert!( + formatted.contains("## Tasks"), + "a missing tasks section should still be synthesized, got: {formatted:?}" + ); + } } diff --git a/crates/lash-core/src/parser/mod.rs b/crates/lash-core/src/parser/mod.rs index b28adfc..f38fb91 100644 --- a/crates/lash-core/src/parser/mod.rs +++ b/crates/lash-core/src/parser/mod.rs @@ -1903,6 +1903,31 @@ More text assert_eq!(task.metadata.docs.len(), 1); } + /// A blank line is allowed between a checkbox and its annotation block. + /// + /// The parsed *values* are not enough to pin this: when the block is not + /// consumed here, the annotations are still recovered downstream, so + /// `owner` and `labels` look correct either way. What breaks is + /// `annotation_line_count`, which drops to 0 because these lines were + /// never attributed to the task. That count is what the write paths use to + /// find where a task's annotations end, so a wrong count is how `lash add` + /// splices a new task into the middle of a previous task's block — the + /// data-loss class fixed in 0.3.1. Assert the count, not just the values. + #[test] + fn test_blank_line_before_annotations_still_counts_them() { + let content = "# Test File\n\n## Tasks\n\n- [ ] Task one\n\n@owner: bob\n@labels: x, y\n"; + let config = LashConfig::default(); + + let file = parse_file_from_string(content, &config).expect("fixture should parse"); + let task = file.tasks.tasks().first().expect("one task expected"); + + assert_eq!( + task.annotation_line_count, 2, + "both annotation lines belong to the task even across the blank line" + ); + assert_eq!(task.metadata.owner, Some("bob".to_string())); + } + // ==================== Description Section Integration Tests ==================== #[test]