Skip to content

Make template assertions newline agnostic - #1909

Merged
mre merged 1 commit into
analysis-tools-dev:masterfrom
dajiaohuang:fix/windows-comment-newlines
Sep 21, 2026
Merged

mre merged 1 commit into
analysis-tools-dev:masterfrom
dajiaohuang:fix/windows-comment-newlines

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Summary

  • Normalize rendered template line endings in assertions so tests pass with Windows core.autocrlf=true.
  • Keep production rendering behavior unchanged.

Validation

  • cargo test --manifest-path ci/Cargo.toml --workspace --all-targets --all-features --locked
  • cargo fmt --manifest-path ci/Cargo.toml --all --check
  • cargo clippy --manifest-path ci/Cargo.toml --workspace --all-targets --all-features --locked -- -D warnings
  • render --skip-deprecated against all catalog entries

@mre

mre commented Sep 21, 2026

Copy link
Copy Markdown
Member

Could we use str::lines() at the affected assertions instead of normalizing the whole rendered string? It handles both LF and CRLF. For the source assertion, we could use rendered.lines().any(|line| line == expected), with expected = format!("Source: {source}"). For the multiline renderer assertions, collecting the lines and checking adjacent windows() would let us keep asserting the blank lines needed between HTML and Markdown without depending on line endings. No extra dependency or helper needed.

@dajiaohuang
dajiaohuang force-pushed the fix/windows-comment-newlines branch from a981af4 to 058d671 Compare September 21, 2026 22:52
@dajiaohuang

Copy link
Copy Markdown
Contributor Author

Implemented the requested newline-agnostic assertions in 058d671: source checks now use str::lines(), and the renderer tests collect lines and use adjacent windows() for the required blank-line structure. Validation: git diff --check, cargo fmt --check for both crates, targeted pr-check test (1 passed), and render deprecated tests (3 passed). CI checks are pending.

@mre
mre merged commit 029d415 into analysis-tools-dev:master Sep 21, 2026
4 checks passed
@mre

mre commented Sep 21, 2026

Copy link
Copy Markdown
Member

Thanks!

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