Skip to content

Commit f10b3bd

Browse files
authored
fix(add): stop inserting a task between the previous task's title and its body (#49)
The parser records a task's checkbox and annotation lines but not its free-text body, so the emitter now walks the source past the whole block before inserting. Closes #48.
1 parent 43ddacf commit f10b3bd

5 files changed

Lines changed: 429 additions & 8 deletions

File tree

‎crates/lash-cli/tests/add_command_test.rs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,143 @@ Prose that must stay under its heading.
795795
);
796796
}
797797

798+
// ---------------------------------------------------------------------
799+
// A task's body must stay with its task (GitHub issue #48)
800+
//
801+
// The parser records a task's checkbox line and its annotation lines and
802+
// nothing else, so free-text body under a task — prose, numbered steps,
803+
// acceptance criteria — was invisible when the insertion point was
804+
// computed. `lash add` landed the new task between the previous task's
805+
// title and its own body, silently reassigning that body to the new task.
806+
// `lash lint` passes on the result and `lash show` does not print bodies,
807+
// so nothing surfaced the damage.
808+
// ---------------------------------------------------------------------
809+
810+
#[test]
811+
fn test_add_does_not_steal_the_previous_tasks_body() {
812+
let project = TestProject::builder()
813+
.with_index("test-project", "Test Project")
814+
.with_file(
815+
"polish.md",
816+
r#"# Repro
817+
818+
@id: polish
819+
820+
## Tasks
821+
822+
- [x] First task @id:rep-first
823+
A one-line body belonging to the first task.
824+
825+
- [x] Second task @id:rep-second
826+
This body belongs to the SECOND task and must stay attached to it.
827+
1. First numbered point.
828+
2. Second numbered point.
829+
Acceptance: this paragraph still reads as part of the second task.
830+
"#,
831+
)
832+
.build();
833+
index(&project);
834+
835+
let stdout = add_task(&project, "New task", "polish.md");
836+
837+
let content = fs::read_to_string(project.file_path("polish.md")).unwrap();
838+
839+
let task = task_line_number(&content, "- [ ] New task");
840+
assert!(
841+
task > task_line_number(&content, "Acceptance: this paragraph"),
842+
"the new task split the second task from its body:\n{content}"
843+
);
844+
assert!(
845+
content.contains(
846+
"- [x] Second task @id:rep-second\n This body belongs to the SECOND task and must stay attached to it."
847+
),
848+
"the second task lost its body:\n{content}"
849+
);
850+
851+
// The reported line must be where the task actually is, not where the
852+
// resolver first guessed.
853+
let reported: usize = stdout
854+
.lines()
855+
.find_map(|line| line.rsplit_once(':'))
856+
.and_then(|(_, line_num)| line_num.trim().parse().ok())
857+
.unwrap_or_else(|| panic!("no line number in:\n{stdout}"));
858+
assert_eq!(reported, task, "reported line disagrees with the file");
859+
}
860+
861+
#[test]
862+
fn test_add_does_not_steal_a_body_split_into_paragraphs() {
863+
// A blank line inside a body does not end the block; indented content
864+
// resumes after it.
865+
let project = TestProject::builder()
866+
.with_index("test-project", "Test Project")
867+
.with_file(
868+
"tasks.md",
869+
r#"# Tasks
870+
871+
@id: tasks
872+
873+
## Tasks
874+
875+
- [ ] Only task
876+
First paragraph of the body.
877+
878+
Second paragraph of the body.
879+
880+
## Notes
881+
882+
Prose that must stay under its heading.
883+
"#,
884+
)
885+
.build();
886+
index(&project);
887+
888+
add_task(&project, "Second task", "tasks.md");
889+
890+
let content = fs::read_to_string(project.file_path("tasks.md")).unwrap();
891+
892+
let task = task_line_number(&content, "- [ ] Second task");
893+
assert!(
894+
task > task_line_number(&content, "Second paragraph of the body."),
895+
"the new task split the body's paragraphs:\n{content}"
896+
);
897+
assert!(
898+
task < task_line_number(&content, "## Notes"),
899+
"task escaped the Tasks section:\n{content}"
900+
);
901+
}
902+
903+
#[test]
904+
fn test_add_does_not_steal_a_tasks_contextual_notes() {
905+
// Indented plain bullets are contextual notes on the task above them.
906+
let project = TestProject::builder()
907+
.with_index("test-project", "Test Project")
908+
.with_file(
909+
"tasks.md",
910+
r#"# Tasks
911+
912+
@id: tasks
913+
914+
## Tasks
915+
916+
- [ ] Only task
917+
- a contextual note
918+
- another contextual note
919+
"#,
920+
)
921+
.build();
922+
index(&project);
923+
924+
add_task(&project, "Second task", "tasks.md");
925+
926+
let content = fs::read_to_string(project.file_path("tasks.md")).unwrap();
927+
928+
assert!(
929+
task_line_number(&content, "- [ ] Second task")
930+
> task_line_number(&content, "- another contextual note"),
931+
"the new task was spliced into the note bullets:\n{content}"
932+
);
933+
}
934+
798935
// ---------------------------------------------------------------------
799936
// The reported task ID must be the indexed one
800937
//

‎crates/lash-core/src/creation/emitter.rs‎

Lines changed: 172 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::path::{Path, PathBuf};
1212

1313
use super::placement::{InsertAnchor, PlacementInfo};
1414
use super::validation::ValidationContext;
15-
use crate::parser::header;
15+
use crate::parser::{checkbox, header};
1616

1717
/// Markdown emitter for task creation
1818
///
@@ -161,6 +161,17 @@ impl MarkdownEmitter {
161161
// Copy lines before insertion point
162162
new_lines.extend_from_slice(&lines[..insert_idx]);
163163

164+
// Separate the new task from a preceding task's body. Landing directly
165+
// under the last line of someone else's prose reads as a continuation
166+
// of it, which is the confusion this insertion point exists to avoid.
167+
// Tasks with no body keep butting up against each other as before.
168+
let follows_body = insert_idx
169+
.checked_sub(1)
170+
.is_some_and(|prev| Self::continues_task_block(lines[prev]));
171+
if follows_body {
172+
new_lines.push("");
173+
}
174+
164175
// Insert task line
165176
new_lines.push(task_line);
166177

@@ -181,7 +192,11 @@ impl MarkdownEmitter {
181192
updated.push('\n');
182193
Self::write_file_atomic(file_path, &updated)?;
183194

184-
Ok(insert_idx + 1)
195+
Ok(if follows_body {
196+
insert_idx + 2
197+
} else {
198+
insert_idx + 1
199+
})
185200
}
186201

187202
/// Translate a [`InsertAnchor`] into a 0-indexed position in `lines`
@@ -195,6 +210,10 @@ impl MarkdownEmitter {
195210
fn resolve_insert_index(content: &str, lines: &[&str], anchor: InsertAnchor) -> usize {
196211
match anchor {
197212
InsertAnchor::Line(line_number) => line_number.saturating_sub(1).min(lines.len()),
213+
InsertAnchor::AfterTaskBlock(line_number) => {
214+
let start = line_number.saturating_sub(1).min(lines.len());
215+
Self::skip_task_body(lines, start)
216+
}
198217
InsertAnchor::EndOfTasksSection => {
199218
// No `## Tasks` heading at all: append at the end of the file
200219
// rather than guessing where the section would have been.
@@ -217,6 +236,65 @@ impl MarkdownEmitter {
217236
}
218237
}
219238

239+
/// Advance past the body lines that belong to the preceding task
240+
///
241+
/// A parsed task records its checkbox line and its annotation lines and
242+
/// nothing else, so the free-text body a task may carry underneath —
243+
/// prose, numbered steps, acceptance criteria, indented contextual note
244+
/// bullets — is invisible to the placement resolver. Anchoring on parsed
245+
/// data alone therefore lands "after this task" between the task's title
246+
/// and its own body, which silently reassigns that body to the newly
247+
/// inserted task (GitHub issue #48). Only the source text says where the
248+
/// block actually ends.
249+
///
250+
/// A line continues the block when it is indented and is not itself a
251+
/// checkbox; a checkbox at any depth starts a new task, and an
252+
/// unindented line has left the block. Blank lines are consumed only when
253+
/// indented content resumes after them, so a body split into paragraphs
254+
/// stays whole while a blank line that genuinely ends the block still
255+
/// stops the scan.
256+
///
257+
/// `start` is a 0-indexed position in `lines`; the return value is the
258+
/// 0-indexed position to insert at.
259+
fn skip_task_body(lines: &[&str], start: usize) -> usize {
260+
let mut idx = start;
261+
262+
while idx < lines.len() {
263+
if lines[idx].trim().is_empty() {
264+
// A blank line only belongs to the block if the block resumes
265+
// after it. Otherwise the block ended at this blank line.
266+
let resumes = lines[idx..]
267+
.iter()
268+
.position(|line| !line.trim().is_empty())
269+
.map(|offset| idx + offset)
270+
.filter(|&next| Self::continues_task_block(lines[next]));
271+
272+
match resumes {
273+
Some(next) => idx = next,
274+
None => break,
275+
}
276+
} else if Self::continues_task_block(lines[idx]) {
277+
idx += 1;
278+
} else {
279+
break;
280+
}
281+
}
282+
283+
idx
284+
}
285+
286+
/// Does this line continue the block that a task's checkbox line opened?
287+
///
288+
/// Indented non-checkbox content belongs to the task above it: body prose,
289+
/// folded annotation values, contextual note bullets. A checkbox at any
290+
/// depth starts a new task, and an unindented line has left the block
291+
/// entirely.
292+
fn continues_task_block(line: &str) -> bool {
293+
!line.trim().is_empty()
294+
&& line.starts_with(char::is_whitespace)
295+
&& checkbox::CheckboxLine::parse(line, 1).is_none()
296+
}
297+
220298
/// Create a new task file with proper structure
221299
///
222300
/// Generates a complete task file with header, metadata, and the first task.
@@ -986,6 +1064,98 @@ mod tests {
9861064
assert_eq!(insert_index(content, InsertAnchor::Line(99)), 3);
9871065
}
9881066

1067+
#[test]
1068+
fn test_after_task_block_steps_over_the_previous_tasks_body() {
1069+
// GitHub issue #48: the parser records the checkbox line and nothing
1070+
// about the body under it, so the resolver anchors on line 6 — between
1071+
// the task's title and its own body. The body must stay with its task.
1072+
let content = "# T\n\n## Tasks\n\n- [x] Second task\n Body prose.\n 1. A step.\n Acceptance: still the second task's.\n";
1073+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 8);
1074+
}
1075+
1076+
#[test]
1077+
fn test_after_task_block_keeps_a_body_split_into_paragraphs_whole() {
1078+
// A blank line inside a body is not the end of the block: indented
1079+
// content resumes after it.
1080+
let content =
1081+
"# T\n\n## Tasks\n\n- [x] One\n First paragraph.\n\n Second paragraph.\n\n## Notes\n";
1082+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 8);
1083+
}
1084+
1085+
#[test]
1086+
fn test_after_task_block_stops_at_a_blank_line_that_ends_the_block() {
1087+
// Nothing indented follows the blank, so the block really did end.
1088+
let content = "# T\n\n## Tasks\n\n- [x] One\n Body.\n\nUnindented prose.\n";
1089+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 6);
1090+
}
1091+
1092+
#[test]
1093+
fn test_after_task_block_stops_at_the_next_checkbox() {
1094+
// An indented checkbox is a child task, not body text.
1095+
let content = "# T\n\n## Tasks\n\n- [x] One\n - [ ] Child\n- [ ] Two\n";
1096+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 5);
1097+
}
1098+
1099+
#[test]
1100+
fn test_after_task_block_leaves_bodyless_tasks_where_they_were() {
1101+
// The common case must resolve exactly like `Line` did before.
1102+
let content = "# T\n\n## Tasks\n\n- [ ] One\n- [ ] Two\n";
1103+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 5);
1104+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(7)), 6);
1105+
}
1106+
1107+
#[test]
1108+
fn test_after_task_block_absorbs_indented_note_bullets() {
1109+
// Contextual note bullets belong to the task above them.
1110+
let content = "# T\n\n## Tasks\n\n- [x] One\n - a note\n - another note\n";
1111+
assert_eq!(insert_index(content, InsertAnchor::AfterTaskBlock(6)), 7);
1112+
}
1113+
1114+
/// Insert `task_line` into `content` at `anchor`, returning the new file
1115+
/// text and the 1-indexed line the task landed on.
1116+
fn insert_at(content: &str, anchor: InsertAnchor, task_line: &str) -> (String, usize) {
1117+
let temp_dir =
1118+
std::env::temp_dir().join(format!("lash-test-insert-{:p}", content.as_ptr()));
1119+
let _ = fs::remove_dir_all(&temp_dir);
1120+
fs::create_dir_all(&temp_dir).unwrap();
1121+
let file_path = temp_dir.join("tasks.md");
1122+
fs::write(&file_path, content).unwrap();
1123+
1124+
let placement = PlacementInfo {
1125+
anchor,
1126+
order_index: 0,
1127+
indent_level: 0,
1128+
};
1129+
let line = MarkdownEmitter::insert_into_existing(&file_path, task_line, &[], &placement)
1130+
.expect("insert must succeed");
1131+
let updated = fs::read_to_string(&file_path).unwrap();
1132+
let _ = fs::remove_dir_all(&temp_dir);
1133+
(updated, line)
1134+
}
1135+
1136+
#[test]
1137+
fn test_insert_after_a_body_separates_the_new_task_from_it() {
1138+
// GitHub issue #48's expected output: the new task sits below the whole
1139+
// block, with a blank line so it does not read as more of that prose.
1140+
let content = "# T\n\n## Tasks\n\n- [x] One\n Body prose.\n Acceptance: mine.\n";
1141+
let (updated, line) = insert_at(content, InsertAnchor::AfterTaskBlock(6), "- [ ] New task");
1142+
1143+
assert_eq!(
1144+
updated,
1145+
"# T\n\n## Tasks\n\n- [x] One\n Body prose.\n Acceptance: mine.\n\n- [ ] New task\n"
1146+
);
1147+
assert_eq!(line, 9, "reported line must be where the task really is");
1148+
}
1149+
1150+
#[test]
1151+
fn test_insert_after_a_bodyless_task_adds_no_separator() {
1152+
let content = "# T\n\n## Tasks\n\n- [ ] One\n";
1153+
let (updated, line) = insert_at(content, InsertAnchor::AfterTaskBlock(6), "- [ ] New task");
1154+
1155+
assert_eq!(updated, "# T\n\n## Tasks\n\n- [ ] One\n- [ ] New task\n");
1156+
assert_eq!(line, 6);
1157+
}
1158+
9891159
/// Emit `note` as annotation lines, then read it back the way the parser
9901160
/// would when it encounters those lines under a task.
9911161
fn round_trip_agent_note(indent: usize, note: &str) -> Option<String> {

0 commit comments

Comments
 (0)