Skip to content

fix: incorrect whitespace removal in concise mode html blocks#216

Merged
DylanPiercey merged 1 commit intomainfrom
concise-html-block-whitespace-fix
Mar 2, 2026
Merged

fix: incorrect whitespace removal in concise mode html blocks#216
DylanPiercey merged 1 commit intomainfrom
concise-html-block-whitespace-fix

Conversation

@DylanPiercey
Copy link
Contributor

@DylanPiercey DylanPiercey commented Mar 2, 2026

Fixes a regression caused by #199 which incorrectly trimmed concise mode html block newlines between nodes (should have just been leading / trailing whitespace for the block.

Eg

---
<a/>
<b/>
<c/>
---

Would yield <a/>, <b/>, <c/>, instead of <a/>\n, <b/>\n, <c/>.

@changeset-bot
Copy link

changeset-bot bot commented Mar 2, 2026

🦋 Changeset detected

Latest commit: 1118b07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
htmljs-parser Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

Walkthrough

This pull request addresses a regression in the htmljs-parser where whitespace was incorrectly removed in concise mode HTML blocks. The fix includes a patch release changelog entry, a new test fixture file for mixed block root scenarios, and a modification to the delimiter HTML block state handler that enables proper text parsing after multiline delimited HTML block endings by starting a new text segment when appropriate conditions are met.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main fix: a regression in whitespace handling for concise mode HTML blocks.
Description check ✅ Passed The description clearly relates to the changeset, explaining the regression from PR #199 and providing concrete examples.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch concise-html-block-whitespace-fix

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/__tests__/fixtures/mixed-block-root/input.marko (1)

1-11: Consider adding one fixture that mirrors the exact self-closing repro.

This mixed-root fixture is good, but a minimal --- <a/> <b/> <c/> --- case would guard the precise regression described in the PR.

Suggested additional fixture
+// src/__tests__/fixtures/concise-html-block-self-closing/input.marko
+---
+<a/>
+<b/>
+<c/>
+---
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/fixtures/mixed-block-root/input.marko` around lines 1 - 11, Add
a new minimal fixture that reproduces the self-closing regression alongside the
existing mixed-block-root fixture: create a test fixture (e.g., under
src/__tests__/fixtures/self-closing-root/) containing a single document with the
exact self-closing pattern `--- <a/> <b/> <c/> ---` so the parser is exercised
on three adjacent self-closing tags; ensure the new fixture filename and content
follow the same layout conventions as
src/__tests__/fixtures/mixed-block-root/input.marko so it is picked up by the
test runner.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/__tests__/fixtures/mixed-block-root/input.marko`:
- Around line 1-11: Add a new minimal fixture that reproduces the self-closing
regression alongside the existing mixed-block-root fixture: create a test
fixture (e.g., under src/__tests__/fixtures/self-closing-root/) containing a
single document with the exact self-closing pattern `--- <a/> <b/> <c/> ---` so
the parser is exercised on three adjacent self-closing tags; ensure the new
fixture filename and content follow the same layout conventions as
src/__tests__/fixtures/mixed-block-root/input.marko so it is picked up by the
test runner.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2389dd and 1118b07.

⛔ Files ignored due to path filters (4)
  • src/__tests__/fixtures/argument-tag-nested-parens/__snapshots__/argument-tag-nested-parens.expected.txt is excluded by !**/__snapshots__/** and included by **
  • src/__tests__/fixtures/mixed-block-root/__snapshots__/mixed-block-root.expected.txt is excluded by !**/__snapshots__/** and included by **
  • src/__tests__/fixtures/multiline-html-block/__snapshots__/multiline-html-block.expected.txt is excluded by !**/__snapshots__/** and included by **
  • src/__tests__/fixtures/open-tag-only-with-body/__snapshots__/open-tag-only-with-body.expected.txt is excluded by !**/__snapshots__/** and included by **
📒 Files selected for processing (3)
  • .changeset/metal-bobcats-bathe.md
  • src/__tests__/fixtures/mixed-block-root/input.marko
  • src/states/BEGIN_DELIMITED_HTML_BLOCK.ts

@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.00%. Comparing base (b2389dd) to head (1118b07).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #216      +/-   ##
==========================================
+ Coverage   91.98%   92.00%   +0.01%     
==========================================
  Files          28       28              
  Lines        1448     1450       +2     
  Branches      327      328       +1     
==========================================
+ Hits         1332     1334       +2     
  Misses         54       54              
  Partials       62       62              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DylanPiercey DylanPiercey merged commit 5d9c02c into main Mar 2, 2026
11 checks passed
@DylanPiercey DylanPiercey deleted the concise-html-block-whitespace-fix branch March 2, 2026 15:56
@github-actions github-actions bot mentioned this pull request Mar 2, 2026
@github-project-automation github-project-automation bot moved this to Todo in Roadmap Mar 2, 2026
@github-project-automation github-project-automation bot moved this from Todo to Done in Roadmap Mar 2, 2026
@DylanPiercey DylanPiercey self-assigned this Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant