feat: shopping list checked file support and comment syntax update

[dubadub]

Summary

• Add .shopping-checked append-only log support per proposal 0016: parse/write + name (checked) and - name (unchecked) entries with case-insensitive last-entry-wins semantics
• Add compaction logic to prune stale checked entries against the current shopping list
• Change comment syntax from // to -- for line/inline comments, add [- ... -] block comment support
• New public API: CheckEntry, parse_checked(), checked_set(), write_check_entry(), compact_checked()

Test plan

• 7 new tests for checked file parsing, set computation, compaction, and roundtrip
• 2 new tests for block comments (multi-line and inline)
• Existing comment tests updated for -- syntax
• All 29 tests passing

[dubadub]

@claude review

[pkg-pr-new]

• cooklang-playground

  npm i https://pkg.pr.new/@cooklang/cooklang@93
  

commit: 93fb44d

[dubadub]

@claude review

[claude]

Claude finished @dubadub's task in 3m 56s —— View job

---

Code Review

• Gather context and read changed files
• Review src/shopping_list.rs changes
• Review bindings/src/shopping_list.rs and bindings/src/lib.rs
• Review parser changes (src/parser/step.rs, src/parser/text_block.rs)
• Review other changes (src/aisle.rs, src/ingredient_list.rs, src/pantry.rs)
• Compile and summarize review feedback

---

Overall this is a well-designed feature addition. The API surface is clean, tests are thorough, and the docstrings clearly communicate the contracts (especially the compact_checked iterator-based API). A few issues worth addressing before merging:

Breaking change: // → -- comment syntax

src/shopping_list.rs:117

The old delimiter // is replaced with -- silently — any user with an existing .shopping-list file using // comments will have those comments parsed as ingredient names without any error or warning. This should be clearly documented in the changelog/release notes. A migration hint in the parse error (or at least a CHANGELOG entry) would help users.

Bug: Block comments inside -- line comments cause spurious errors

src/shopping_list.rs:144 — strip_block_comments runs before collect_lines, so it processes the entire input before line comments are stripped. A [- that appears after a -- on the same line is parsed as a block comment opener:

salt -- [- this looks commented out
pepper

This would error with "unterminated block comment" even though the [- is logically inside a -- line comment. The fix is to either process both comment types in a single pass, or strip line comments first and then scan for block comments. Fix this →

compact_checked loses original casing

src/shopping_list.rs:422-446

checked_set() lowercases all names into its HashMap, so the output of compact_checked always contains lowercase names — even when the user originally wrote + Olive Oil. This is partially documented but the doc says "preserve the lowercased form", which might surprise callers that want to re-display items. Consider either preserving the last-seen casing alongside the bool state in checked_set, or making the casing-loss more prominent in the docs.

// Currently state stores only bool, losing original casing:
let mut state: HashMap<String, bool> = HashMap::new();
// Could store the original name:
let mut state: HashMap<String, (bool, String)> = HashMap::new();

write_check_entry does not validate names

src/shopping_list.rs:402-407

A name containing \n (or \r\n) would produce multiple log lines and break parsing on the next read. A cheap guard would help:

pub fn write_check_entry(entry: &CheckEntry, mut w: impl std::io::Write) -> std::io::Result<()> {
    match entry {
        CheckEntry::Checked(name) => {
            debug_assert!(!name.contains('\n'), "name must not contain newlines");
            writeln!(w, "+ {name}")
        }
        CheckEntry::Unchecked(name) => {
            debug_assert!(!name.contains('\n'), "name must not contain newlines");
            writeln!(w, "- {name}")
        }
    }
}

Or return an Err for invalid names (though that changes the signature).

Minor: parse_checked_impl in bindings — needless .iter() with immediate deref

bindings/src/shopping_list.rs:144-149

// Current — collects into a Vec, then iterates by reference:
sl::parse_checked(input)
    .iter()
    .map(CheckEntry::from)  // From<&OriginalCheckEntry>
    .collect()

// Cleaner — consumes directly:
sl::parse_checked(input)
    .into_iter()
    .map(|e| CheckEntry::from(&e))
    .collect()

Not a bug, just a style nit.

Minor: collect_ingredient_names docstring is slightly misleading

src/shopping_list.rs:449-457

The function does recurse into recipe children to find Ingredient items. The doc "Lists that only store recipe references will produce an empty result" is technically correct (if all children are more recipe references with no ingredient items), but on a first read it sounds like the function doesn't recurse, which it does. Worth a small clarification.

---

Positives

• The compact_checked<'a, I> generic-iterator design is excellent — makes the contract about expanded vs. on-disk ingredients explicit and avoids tying callers to ShoppingList.
• The strip_block_comments space-replacement strategy (rather than empty-string removal) to prevent token gluing is a thoughtful edge case.
• Pantry round-trip fix (WithAttributes-with-no-attributes → Simple) is a good correctness improvement.
• Test coverage is solid: last-wins semantics, case-insensitivity, compaction stale-removal, and write roundtrip are all tested.
• UniFFI binding layer cleanly separates wrapper types from core types and the conversions are symmetric.