Skip to content

Commit 727f2df

Browse files
committed
fix(add): stop prepending above the H1 when the Tasks section is empty
`lash add` against an existing file whose `## Tasks` section held no tasks wrote the checkbox at line 1, above the H1. The parser never saw it as a task, so `lash index` reported 0 tasks and `lash lint` passed clean: the task was on disk and nowhere else, with exit code 0 and a normal success message pointing at `path:0`. `resolve_append` returned `line_number: 0` whenever the file had no tasks, commented "Signal for new file", and the emitter mapped 0 to insert index 0. The sentinel conflated "brand-new file" with "existing file whose Tasks section is empty", and only the first is safe to write at offset 0. `PlacementInfo` now carries an `InsertAnchor` instead of a bare line number: either a concrete `Line(n)` or `EndOfTasksSection`. The ambiguity cannot recur, because there is no longer a number that means something other than a line. Resolving `EndOfTasksSection` needs the source text. A parsed `TaskFile` records task line numbers and nothing about section boundaries, so an empty section has nothing to anchor to, which is why the old code fell back to a hardcoded guess of line 15. The emitter resolves it instead, against the content it already reads, via a new `parser::header::tasks_section_body`. That returns the section's line span using pulldown-cmark, so a `##` inside a code fence does not close the section, and H3 headings stay inside it (files group tasks under `### Subsection` headings). With no `## Tasks` heading at all it appends at end of file rather than guessing. The line reported on success now comes from where the task was actually written, for new files as well, so `path:0` is gone. Two things fixed in passing, both in code this commit rewrites: `find_end_of_tasks_section` and its line-15 estimate are deleted rather than ported, and `insert_into_existing` restores the trailing newline that `join` dropped, which had every `lash add` leaving the file ending mid-line. Tests: five integration tests in add_command_test.rs (empty section lands below the header and reports the right line, is indexed afterwards, a following `## Notes` section survives intact, the new-file path still writes a well-formed file, and files end in a newline) plus six unit tests on anchor resolution covering empty sections, trailing blank lines, a heading inside a code fence, a file with no Tasks section, and clamping. Four of the five integration tests fail without the fix.
1 parent 0305925 commit 727f2df

6 files changed

Lines changed: 459 additions & 92 deletions

File tree

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

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,3 +366,194 @@ fn test_add_preserves_multiline_agent_note_through_reindex() {
366366
);
367367
}
368368
}
369+
370+
// ---------------------------------------------------------------------
371+
// Appending to a file whose `## Tasks` section is empty
372+
//
373+
// `resolve_append` used to return the magic line number 0 whenever the
374+
// target file held no tasks, meaning "this is a brand-new file, write at
375+
// offset 0". For a file that already existed with an empty `## Tasks`
376+
// section that was catastrophically wrong: the checkbox was prepended
377+
// above the H1, so the parser never saw it as a task, `lash index` found
378+
// nothing, and `lash lint` still passed. The task was on disk and
379+
// invisible.
380+
// ---------------------------------------------------------------------
381+
382+
/// Line the task line lands on, 1-indexed.
383+
fn task_line_number(content: &str, needle: &str) -> usize {
384+
content
385+
.lines()
386+
.position(|line| line.contains(needle))
387+
.map(|idx| idx + 1)
388+
.unwrap_or_else(|| panic!("'{needle}' not found in:\n{content}"))
389+
}
390+
391+
/// Run `lash add`, returning the command's stdout.
392+
fn add_task(project: &TestProject, title: &str, file: &str) -> String {
393+
let output = run_lash_command()
394+
.arg("--root")
395+
.arg(project.path())
396+
.arg("add")
397+
.arg(title)
398+
.arg("--file")
399+
.arg(file)
400+
.assert()
401+
.success()
402+
.get_output()
403+
.stdout
404+
.clone();
405+
String::from_utf8(output).unwrap()
406+
}
407+
408+
#[test]
409+
fn test_add_to_empty_tasks_section_lands_below_the_header() {
410+
let project = TestProject::builder()
411+
.with_index("test-project", "Test Project")
412+
.with_file(
413+
"empty.md",
414+
r#"# Empty
415+
416+
@id: empty
417+
418+
## Tasks
419+
"#,
420+
)
421+
.build();
422+
index(&project);
423+
424+
let stdout = add_task(&project, "Ship it", "empty.md");
425+
426+
let content = fs::read_to_string(project.file_path("empty.md")).unwrap();
427+
428+
assert_eq!(
429+
content.lines().next(),
430+
Some("# Empty"),
431+
"the H1 must still be the first line:\n{content}"
432+
);
433+
// The old sentinel surfaced here too, as a literal `empty.md:0`.
434+
assert!(
435+
stdout.contains(&format!(
436+
"empty.md:{}",
437+
task_line_number(&content, "- [ ] Ship it")
438+
)),
439+
"reported line does not match where the task was written:\n{stdout}"
440+
);
441+
assert!(
442+
task_line_number(&content, "- [ ] Ship it") > task_line_number(&content, "## Tasks"),
443+
"task must land inside the Tasks section:\n{content}"
444+
);
445+
}
446+
447+
#[test]
448+
fn test_add_to_empty_tasks_section_is_indexed() {
449+
// The destructive part of the bug: the task was written to a place the
450+
// parser does not read, so it existed on disk and nowhere else.
451+
let project = TestProject::builder()
452+
.with_index("test-project", "Test Project")
453+
.with_file(
454+
"empty.md",
455+
r#"# Empty
456+
457+
@id: empty
458+
459+
## Tasks
460+
"#,
461+
)
462+
.build();
463+
index(&project);
464+
465+
add_task(&project, "Ship it", "empty.md");
466+
index(&project);
467+
468+
run_lash_command()
469+
.arg("--root")
470+
.arg(project.path())
471+
.arg("list")
472+
.assert()
473+
.success()
474+
.stdout(predicate::str::contains("Ship it"));
475+
}
476+
477+
#[test]
478+
fn test_add_to_empty_tasks_section_keeps_following_sections_intact() {
479+
let project = TestProject::builder()
480+
.with_index("test-project", "Test Project")
481+
.with_file(
482+
"sections.md",
483+
r#"# Sections
484+
485+
@id: sections
486+
487+
## Tasks
488+
489+
## Notes
490+
491+
Prose that must survive.
492+
"#,
493+
)
494+
.build();
495+
index(&project);
496+
497+
add_task(&project, "Ship it", "sections.md");
498+
499+
let content = fs::read_to_string(project.file_path("sections.md")).unwrap();
500+
501+
assert_eq!(
502+
content.lines().next(),
503+
Some("# Sections"),
504+
"the H1 must still be the first line:\n{content}"
505+
);
506+
assert!(
507+
content.contains("Prose that must survive."),
508+
"trailing section content was destroyed:\n{content}"
509+
);
510+
511+
let task = task_line_number(&content, "- [ ] Ship it");
512+
assert!(
513+
task > task_line_number(&content, "## Tasks")
514+
&& task < task_line_number(&content, "## Notes"),
515+
"task escaped the Tasks section:\n{content}"
516+
);
517+
}
518+
519+
#[test]
520+
fn test_add_creating_a_new_file_still_writes_a_well_formed_file() {
521+
// Guards the other half of the split: the old code used one sentinel for
522+
// both "new file" and "existing file with an empty section".
523+
let project = TestProject::builder()
524+
.with_index("test-project", "Test Project")
525+
.build();
526+
index(&project);
527+
528+
add_task(&project, "Ship it", "fresh.md");
529+
530+
let content = fs::read_to_string(project.file_path("fresh.md")).unwrap();
531+
532+
assert!(
533+
task_line_number(&content, "- [ ] Ship it") > task_line_number(&content, "## Tasks"),
534+
"task must land inside the Tasks section:\n{content}"
535+
);
536+
537+
index(&project);
538+
run_lash_command()
539+
.arg("--root")
540+
.arg(project.path())
541+
.arg("list")
542+
.assert()
543+
.success()
544+
.stdout(predicate::str::contains("Ship it"));
545+
}
546+
547+
#[test]
548+
fn test_add_leaves_the_file_ending_in_a_newline() {
549+
let project = project_with_tasks_file();
550+
index(&project);
551+
552+
add_task(&project, "Second task", "tasks.md");
553+
554+
let content = fs::read_to_string(project.file_path("tasks.md")).unwrap();
555+
assert!(
556+
content.ends_with('\n'),
557+
"file ends mid-line, every later diff shows '\\ No newline at end of file':\n{content}"
558+
);
559+
}

0 commit comments

Comments
 (0)