Skip to content

feat: support backslash line breaks for formatting in steps and notes#256

Merged
dubadub merged 2 commits intomainfrom
claude/cooklang-backslash-formatting-cbn7H
Feb 6, 2026
Merged

feat: support backslash line breaks for formatting in steps and notes#256
dubadub merged 2 commits intomainfrom
claude/cooklang-backslash-formatting-cbn7H

Conversation

@dubadub
Copy link
Member

@dubadub dubadub commented Feb 6, 2026

The Cooklang parser converts backslash-newline sequences into literal
newline characters in text items. This change makes both the CLI and
web UI honor those newlines for precise formatting control.

CLI (human-readable output):

  • Step text containing newlines is split into paragraphs, each properly
    indented under the step number
  • Note text containing newlines preserves line breaks with consistent
    indentation

Web UI:

  • Added LineBreak variant to StepItem enum, rendered as
    in HTML
  • Text items are split on newline characters into separate Text and
    LineBreak items during template data construction
  • Note paragraphs use white-space: pre-line for newline rendering

https://claude.ai/code/session_01YUd1SFaAakySfqdq4qBYnD

@claude

This comment has been minimized.

The Cooklang parser converts backslash-newline sequences into literal
newline characters in text items. This change makes both the CLI and
web UI honor those newlines for precise formatting control.

CLI (human-readable output):
- Step text containing newlines is split into paragraphs, each properly
  indented under the step number
- Note text containing newlines preserves line breaks with consistent
  indentation

Web UI:
- Added LineBreak variant to StepItem enum, rendered as <br> in HTML
- Text items are split on newline characters into separate Text and
  LineBreak items during template data construction
- Note paragraphs use white-space: pre-line for newline rendering

https://claude.ai/code/session_01YUd1SFaAakySfqdq4qBYnD
@dubadub dubadub force-pushed the claude/cooklang-backslash-formatting-cbn7H branch from 7f10161 to 40f4987 Compare February 6, 2026 13:43
@claude

This comment has been minimized.

Fixes clippy::unnecessary_unwrap warnings in latex, markdown, and typst
output formatters.

https://claude.ai/code/session_01YUd1SFaAakySfqdq4qBYnD
@claude
Copy link

claude bot commented Feb 6, 2026

Pull Request Review: Backslash Line Break Support

Summary

This PR adds support for backslash-newline sequences in recipe steps and notes, enabling precise formatting control in both CLI and web UI. The implementation is generally well-executed with good attention to detail.


✅ Strengths

  1. Consistent Implementation: The feature is implemented consistently across all output formats (CLI, Web UI, LaTeX, Markdown, Typst)

  2. Clean Refactoring: The clippy fixes (is_some() + unwrap()if let) are excellent improvements that reduce code fragility

  3. Proper Separation of Concerns: Web UI uses a new LineBreak enum variant while CLI uses string processing - appropriate for each context

  4. Good Documentation: The commit message clearly explains the implementation approach for both CLI and Web UI


🔍 Code Quality Issues

1. Potential Issue: Empty Paragraph Handling (CLI)

Location: src/util/cooklang_to_human.rs:419-428

The CLI implementation doesn't filter empty paragraphs after splitting on \n. If a recipe contains consecutive newlines (e.g., "First line\n\nThird line"), this will call print_wrapped_with_options() with empty strings, potentially creating unwanted blank lines.

Current code:

let paragraphs: Vec<&str> = step_text.trim().split('\n').collect();
for (i, paragraph) in paragraphs.iter().enumerate() {
    if i == 0 {
        let first = format\!("{:>2}. {}", step.number, paragraph.trim_start());
        print_wrapped_with_options(w, &first, |o| o.subsequent_indent("    "))?;
    } else {
        print_wrapped_with_options(w, paragraph.trim_start(), |o| {
            o.initial_indent("    ").subsequent_indent("    ")
        })?;
    }
}

Suggested fix:

let paragraphs: Vec<&str> = step_text.trim().split('\n').filter(|p| \!p.trim().is_empty()).collect();

Same issue exists at line 453 for notes.

2. Inconsistency: Web UI Empty Text Check

Location: src/server/ui.rs:344

The Web UI filters empty parts (if \!part.is_empty()), but this creates an asymmetry:

  • Empty strings between newlines are skipped
  • But LineBreak is still inserted before them

This means "text\n\nmore" becomes [Text("text"), LineBreak, LineBreak, Text("more")], which may render as two <br> tags.

Suggested approach:
Either filter empty parts consistently, or handle multiple consecutive newlines as a design decision. Consider restructuring the logic to only add line breaks between non-empty text segments.

3. Template Readability

Location: templates/recipe.html:302

The entire step rendering logic is on a single 900+ character line, making it very difficult to maintain. While this is a pre-existing issue, adding LineBreak makes it worse.

Recommendation: Consider breaking the template into multiple lines or extracting a partial template for step items.


🔒 Security Considerations

XSS Risk Assessment: ✅ SAFE

  • Text content is properly escaped by Askama's default HTML escaping
  • <br> tag is hardcoded, not user-supplied
  • No raw HTML injection vectors introduced

⚡ Performance Considerations

  1. String Allocation: Each split('\n') creates a new Vec allocation. For recipes with many steps, this could add up. However, given typical recipe sizes (10-50 steps), this is likely negligible.

  2. Web UI Processing: The split operation happens during template data construction (once per page load), not during rendering, which is good.

  3. CLI Processing: Similar - happens once per output generation.

Verdict: Performance impact is minimal and acceptable.


🧪 Test Coverage

Issue: No tests were added for this feature.

Given the scope of changes across multiple output formats, I recommend adding:

  1. Unit test for CLI output with newlines in steps/notes
  2. Integration test verifying Web UI template rendering
  3. Snapshot test comparing output with/without newlines
  4. Edge case tests: consecutive newlines, trailing newlines, empty paragraphs

The repository has existing test infrastructure in tests/ that could be extended:

  • tests/output_formats_test.rs - perfect place for CLI output tests
  • tests/edge_cases_test.rs - good for boundary conditions

📋 Minor Observations

  1. CSS Choice: Using white-space: pre-line for notes (line 321) is appropriate - it preserves newlines while collapsing other whitespace.

  2. Trim Behavior: The use of both trim() and trim_start() is intentional and correct - trim() cleans the full text first, then trim_start() handles paragraph indentation.

  3. Consistent Style: The indentation (4 spaces for step continuations, 11 spaces for note continuations) maintains visual consistency with existing output.


📊 Best Practices Alignment

Per CLAUDE.md:

  • ✅ Follows workspace structure conventions
  • ✅ Uses proper error handling with Result
  • ✅ Consistent with existing output formatting patterns
  • ⚠️ Missing tests (noted in CONTRIBUTING.md as a known gap)
  • ✅ Semantic commit messages used

🎯 Recommendations

Must Fix (before merge):

  1. Add empty paragraph filtering to CLI implementation to prevent blank line issues

Should Consider (optional):
2. Add basic test coverage for the feature
3. Document the newline behavior in user-facing docs or help text
4. Refactor template to improve readability (separate PR)

Nice to Have:
5. Consider whether consecutive newlines should be collapsed or preserved (design decision)


📝 Overall Assessment

This is a solid implementation of a useful feature. The code quality is good, and the clippy fixes are a welcome bonus. The main concerns are:

  1. Potential blank line issues with empty paragraphs
  2. Lack of test coverage
  3. Minor inconsistency between CLI and Web UI empty text handling

With the empty paragraph filtering fix, this PR would be ready to merge. The test coverage gap is consistent with the project's current state (as noted in CONTRIBUTING.md) but should be addressed eventually.

Recommendation: ✅ Approve with minor fixes

Great work on this feature! The backslash-newline support will give recipe authors much better control over formatting.

@dubadub dubadub merged commit 04fd52e into main Feb 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants