Strip the presentational trailing newline when importing code blocks - #1216
Open
wintan1418 wants to merge 2 commits into
Open
Strip the presentational trailing newline when importing code blocks#1216wintan1418 wants to merge 2 commits into
wintan1418 wants to merge 2 commits into
Conversation
Markdown renderers like Redcarpet emit <pre> blocks whose text content ends with a newline before the closing tag. Lexical imports that newline as an extra code line, so the editor showed a spurious blank line at the end of every imported code block. Trim a single trailing newline from the last text node of each <pre> before generating Lexical nodes, mirroring how the HTML spec disregards a leading newline right after <pre>. Interior newlines and deliberate blank lines beyond the final one are unaffected. Fixes basecamp#917
wintan1418
marked this pull request as ready for review
July 30, 2026 18:40
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an import artifact where HTML <pre> blocks that end with a trailing newline (common from Markdown renderers) were being imported into Lexical as an extra empty code line, producing a spurious blank line at the end of code blocks in Lexxy.
Changes:
- Added
stripTrailingCodeBlockNewlines(doc)DOM pre-processing helper to trim a single trailing newline from each<pre>’s last text node before Lexical import. - Wired the helper into the shared import choke point (
editor.$generateNodesFromDOM) so it applies to initial value,setValue, paste, and templates. - Added Playwright + Vitest coverage for trailing-newline
<pre>imports, including nested markup cases.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/helpers/html_helper.js |
Introduces DOM preprocessing to strip a single presentational trailing newline from <pre> blocks. |
src/elements/editor.js |
Applies the preprocessing during $generateNodesFromDOM so all import paths benefit. |
test/javascript/unit/helpers/html_helper.test.js |
Adds unit tests for the new helper across several edge cases. |
test/browser/tests/formatting/code_block_trailing_newline.test.js |
Adds Playwright regression tests verifying no spurious blank code line is imported. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+68
to
+70
| function lastTextNodeIn(element) { | ||
| const walker = element.ownerDocument.createTreeWalker(element, NodeFilter.SHOW_TEXT) | ||
| let lastTextNode = null |
Author
There was a problem hiding this comment.
Good catch — switched to the numeric constant with the inline comment, matching the pattern in selection.js and code_highlighting_helper.js. Fixed in 6409ee1.
Match the convention used by the other tree-walker helpers, which avoid referencing the NodeFilter global so the helper keeps working where it is not defined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #917
Markdown renderers like Redcarpet emit
<pre>blocks whose text content ends with a newline before the closing tag. Lexical imports that newline as an extra code line, so the editor showed a spurious blank line (<br><br>) at the end of every imported code block.The fix
stripTrailingCodeBlockNewlines(doc)(inhtml_helper.js, alongside the existingaddBlockSpacingDOM-preprocessing helper) trims a single trailing newline from the last text node of each<pre>in the parsed document. It runs ineditor.$generateNodesFromDOM, the single choke point all import paths share (initial value,setValue, paste, prompt templates).This mirrors how the HTML spec disregards a leading newline right after
<pre>— the trailing one is presentational, not content. Only one newline is trimmed, so interior line breaks and deliberate blank lines beyond the final one survive unchanged.Tests
test/browser/tests/formatting/code_block_trailing_newline.test.js): importing a<pre>with a trailing newline yields no blank last line; interior line breaks are kept; a<pre>without a trailing newline is untouched. The first two fail onmainand pass with this change.test/javascript/unit/helpers/html_helper.test.js): unit coverage of the helper, including nested markup inside<pre>, the keep-deliberate-blank-lines case, and empty code blocks.Full Playwright suite passes on Chromium (621) and the new tests on Firefox too; WebKit couldn't launch locally (missing system libraries), so relying on CI for that project.
yarn lintandyarn testare clean.