Conversation
eaglethrost
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for the extensive testing!
…s-cause-render-failure-on Resolve merge conflict in escapeProblematicBraces by combining: - Paragraph-spanning expression detection (CX-2955) - HTML element protection to prevent backslash in output (CX-2978) Updated test to reflect that braces inside HTML elements are protected and should NOT be escaped (to avoid backslashes appearing in rendered output). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@claude review this pr |
rafegoldberg
left a comment
There was a problem hiding this comment.
Maybe the Claude bot isn't set up in this repo yet? Either way, I reviewed and QAed this locally and all looks good to me!
| // Check if this newline creates a blank line (only whitespace since last newline) | ||
| if (lastNewlinePos >= 0) { | ||
| const between = chars.slice(lastNewlinePos + 1, i).join(''); | ||
| if (/^[ \t]*$/.test(between)) { |
There was a problem hiding this comment.
The only thing my local Claude said that I thought was worth mentioning:
The blank-line detection checks only for
\nand the whitespace regex/^[ \t]*$/doesn't include\r. On content with Windows-style \r\n line endings, a blank line (\r\n\r\n) would have a\rbetween the two\ns, failing the whitespace check. If the upstream pipeline normalizes line endings to\nbefore this function runs (which is likely), this is a non-issue. But if not, it could miss the exact scenario this PR aims to fix. Worth verifying.Suggestion: Either confirm normalization happens upstream, or change the regex to
/^[\s]*$/(or/^[ \t\r]*$/) to be safe.
No idea if these line endings get normalized anywhere in our system? Via the in-product editor I'd imagine? But could maybe get funky for docs coming in via bidi sync?
|
One thing I did notice during testing is that these normalizations don't work with the strip comments flag! So things like this will still break where/whenever comments are being stripped:
This is a pretty niche case though, so (unless we think that's gonna be a blocker for Akamai for some reason) I don't think this is a blocker! |
## Version 13.6.0 ### ✨ New & Improved * correctly render block level elements in callout ([#1362](#1362)) ([32040cb](32040cb)) ### 🛠 Fixes & Updates * **mdxish-editor:** built in anchor component not deserializing to to a link in mdxish editor ([#1361](#1361)) ([30e037d](30e037d)) * **mdxish:** callout end tag rendering ([#1373](#1373)) ([cad7594](cad7594)) * **mdxish:** combine code tabs if separated by CLRF token \r\n ([#1372](#1372)) ([2d8d267](2d8d267)) * **mdxish:** Curly braces on separate lines cause render failure on MDXish ([#1364](#1364)) ([d85e106](d85e106)) * **mdxished:** Preserve embed type and dimensions when copy/pasting <Embed> blocks ([#1369](#1369)) ([5af9623](5af9623)) ### 📘 Tests & Docs * **xish:** add CLAUDE.md + processor flow overview docs ([#1370](#1370)) ([b8d9e4c](b8d9e4c)) <!--SKIP CI-->
|
This PR was released!🚀 Changes included in v13.6.0 |
🧰 Changes
Problem
When using the mdxish engine with content like:
The page crashes with: Unexpected end of file in expression, expected a corresponding closing brace for
{Root cause
When a blank line separates { and }, markdown splits them into different paragraphs, so the parser sees { without its matching } and crashes
The Fix
Enhanced
escapeUnbalanceBraces()(escapeProblematicBraces) to detect and escape expressions that span blank lines (paragraph boundaries). The function now handles both:Describe your changes in detail.
🧬 QA & Testing