Repository navigation
Conversation
## Description Copy as Markdown includes an entire code block even when only part of its text is selected. Make the CODE transformer slice the selected raw text before adding the fence, preserving the language and whitespace. Whole-document exports stay unchanged. ## Changeset `.changeset/markdown-selected-code.md` ## Test plan ### Before `pnpm run test-unit packages/lexical-markdown/src/__tests__/unit/LexicalMarkdown.test.ts --maxWorkers=1 -t 'code block partial selection'` ```text Test Files 1 failed (1) Tests 1 failed | 659 skipped (660) ``` The regression copies unselected lines and text outside both selection endpoints. ### After `pnpm run test-unit packages/lexical-markdown --maxWorkers=2` ```text Test Files 9 passed (9) Tests 925 passed (925) ``` The test also checks that export leaves editor state unchanged and that importing the result preserves the selected code. Full unit suite, `ci-check`, `build`, `build-www`, `build-prod` and `build-types` passed. Browser suites were not run; this change is covered through the headless public API.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good — thanks for the fix! Copy as Markdown now copies only the selected part of a code block. The CODE transformer honours the selection argument that MultilineElementTransformer.export already documents ("only selected content should be included"). Whole-document export is unchanged, and the fence length is chosen from the sliced text, so a slice that contains backticks still gets a fence that closes it. The changeset follows .changeset/README.md, including the Verify: paragraph.
A few optional, non-blocking suggestions:
- Cover the NodeSelection branch (
packages/lexical-markdown/src/MarkdownTransformers.ts:867-869). No test exercises it: without it, a NodeSelection of a code block would export an empty fence (```js\n```), and the current suite would still pass. CONTRIBUTING.md asks for tests for new code ("If you've added code that should be tested, add tests"). A NodeSelection case, plus a backward or paragraph-into-code selection, next to the new test inLexicalMarkdown.test.tswould pin both paths. - Possible follow-up, outside this PR:
@lexical/mdasthas its own code handler (packages/lexical-mdast/src/handlers.ts:534) that still usesnode.getTextContent(). Its$convertSelectionToMarkdownStringtherefore still copies the whole block. A matching slice there would make the two exporters agree.
FYI only: a selection that merely touches the edge of a code block (from the end of its last line into the next paragraph) now copies an empty fence. That matches how headings (# ) and quotes (> ) already behave in the selection exporter, so I don't think it needs to change here.
| if ($isNodeSelection(selection) && node.isSelected(selection)) { | ||
| return node.getTextContent(); | ||
| } |
There was a problem hiding this comment.
Optional: no test exercises this NodeSelection branch. Without it, a NodeSelection of a code block would export an empty fence and the current suite would still pass. A NodeSelection case next to the new test in LexicalMarkdown.test.ts would pin it (CONTRIBUTING.md: "If you've added code that should be tested, add tests").
There was a problem hiding this comment.
Added a NodeSelection case in 402c381 that expects the whole two-line code block, with its language and whitespace, and not the paragraphs around it. It fails with an empty fence when the NodeSelection branch is removed.
## Description
The code block NodeSelection branch in the CODE transformer had no test: with it removed, a NodeSelection of a code block exports an empty fence and the suite still passed. Add a case beside the partial-selection test that selects a two-line code block between unselected paragraphs and expects the whole fenced block, including language and whitespace. No production code changes.
## Test plan
### Before
With only the NodeSelection early return removed:
`node_modules/.bin/vitest --project unit --no-watch packages/lexical-markdown/src/__tests__/unit/LexicalMarkdown.test.ts --maxWorkers=1 -t 'code block node selection'`
```text
AssertionError: expected '```js\n```' to be '```js\nconst value = "*";\n\treturn v…' // Object.is equality
Test Files 1 failed (1)
Tests 1 failed | 660 skipped (661)
```
### After
`node_modules/.bin/vitest --project unit --project scripts-unit --no-watch packages/lexical-markdown --maxWorkers=2`
```text
Test Files 9 passed (9)
Tests 926 passed (926)
```
`pnpm run ci-check`, `pnpm run build` and the full unit suite passed. Browser suites were not run; the case uses the headless public API.
There was a problem hiding this comment.
This looks good — thanks for the fix, and for adding the NodeSelection case. Copy as Markdown now copies only the selected part of a code block: CODE.export honours the selection argument that MultilineElementTransformer.export already documents ("When provided, only selected content should be included"). Whole-document export is unchanged, and the fence length is chosen from the sliced text, so a slice that contains three backticks still gets a fence that closes it (CommonMark 0.31.2 §4.5). The new NodeSelection test pins the branch at packages/lexical-markdown/src/MarkdownTransformers.ts:867-869, and the changeset follows .changeset/README.md, including the Verify: paragraph.
Optional, non-blocking suggestions:
- Pin a backward or paragraph-into-code selection (
packages/lexical-markdown/src/__tests__/unit/LexicalMarkdown.test.ts, next to the two new tests). Both endpoints of the partial-selection test sit inside the code block and the selection runs forward. A selection that starts in the paragraph above the block, or one made backward, exports correctly today, but no test covers it. CONTRIBUTING.md asks: "If you've added code that should be tested, add tests." - Possible follow-up, outside this PR:
@lexical/mdast's$exportCode(packages/lexical-mdast/src/handlers.ts:527-535) still usesnode.getTextContent(), so that package's$convertSelectionToMarkdownStringstill copies the whole block.
FYI only, no change needed: a selection that only touches the edge of a code block (from the end of its last line into the next paragraph) copies an empty fence, the same way headings (# ) and quotes (> ) already behave in this exporter.
Description
Copy as Markdown includes an entire code block even when only part of its text is selected. Make the CODE transformer slice the selected raw text before adding the fence, preserving the language and whitespace. Whole-document exports stay unchanged.
Changeset
.changeset/markdown-selected-code.mdTest plan
Before
pnpm run test-unit packages/lexical-markdown/src/__tests__/unit/LexicalMarkdown.test.ts --maxWorkers=1 -t 'code block partial selection'The regression copies unselected lines and text outside both selection endpoints.
After
pnpm run test-unit packages/lexical-markdown --maxWorkers=2The test also checks that export leaves editor state unchanged and that importing the result preserves the selected code. Full unit suite,
ci-check,build,build-www,build-prodandbuild-typespassed. Browser suites were not run; this change is covered through the headless public API.