🐛 fix: prevent nesting of code nodes in selection processing#119
Open
🐛 fix: prevent nesting of code nodes in selection processing#119
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the markdown text-format transformer pipeline to allow transformer-specific cancellation, and uses that capability in the code plugin to prevent applying the inline code shortcut when the current selection already includes a code node (avoiding nested code nodes). Sequence diagram for markdown text-format processing with cancellable transformerssequenceDiagram
actor User
participant Editor
participant MarkdownService
participant TextFormatPipeline
participant CodeShortcutTransformer
participant Selection
User->>Editor: Type inline code shortcut
Editor->>MarkdownService: handleKeyPress
MarkdownService->>TextFormatPipeline: $runTextFormatTransformers(selection)
TextFormatPipeline->>CodeShortcutTransformer: match inline code pattern
TextFormatPipeline->>Selection: create nextSelection with open/close tags removed
TextFormatPipeline->>CodeShortcutTransformer: process(nextSelection)
CodeShortcutTransformer->>Selection: getNodes()
Selection-->>CodeShortcutTransformer: nodes including CodeNode
CodeShortcutTransformer-->>TextFormatPipeline: return false
TextFormatPipeline->>Editor: $setSelection(anchorNode.selectEnd())
TextFormatPipeline-->>MarkdownService: transform cancelled
MarkdownService-->>Editor: no inline code applied
Editor-->>User: Selection restored without nested code node
Class diagram for updated TextFormatTransformer and CodePlugin behaviorclassDiagram
class TextFormatTransformer {
<<type>>
+format: ReadonlyArray~TextFormatType~
+intraword: boolean
+process(selection: RangeSelection) boolean void
+tag: string
+type: string
}
class TextFormatPipeline {
+$runTextFormatTransformers(editor: LexicalEditor, selection: RangeSelection) boolean
}
class RangeSelection {
+getNodes() Array~LexicalNode~
+getTextContent() string
+removeText() void
+insertNodes(nodes: Array~LexicalNode~) void
+focus: SelectionPoint
}
class SelectionPoint {
+set(key: NodeKey, offset: number, type: string) void
}
class CodePlugin {
+registerMarkdownShortCuts() void
}
class CodeShortcutTransformer {
+process(selection: RangeSelection) boolean void
}
class CodeNode {
+getType() string
+getTextContent() string
}
class CursorNode {
+$createCursorNode() CursorNode
}
class MarkdownService {
+registerMarkdownShortCuts(transformers: Array~TextFormatTransformer~) void
}
TextFormatPipeline --> TextFormatTransformer : uses
TextFormatPipeline --> RangeSelection : manipulates
MarkdownService --> TextFormatPipeline : invokes
MarkdownService --> TextFormatTransformer : registers
CodePlugin --> MarkdownService : configures
CodePlugin --> CodeShortcutTransformer : provides
CodeShortcutTransformer ..|> TextFormatTransformer : conforms
RangeSelection --> CodeNode : getNodes returns
RangeSelection --> CursorNode : insertNodes
CodeShortcutTransformer --> CodeNode : checks getType
CodeShortcutTransformer --> CursorNode : inserts
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
|
👍 @ilimei |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
process?: (selection) => boolean | voidcontract combined withif (!matcher.process(nextSelection))will treat existingprocessimplementations that returnvoidas a cancellation, effectively disabling those transformers; consider checking explicitly forfalse(e.g.if (matcher.process(nextSelection) === false)) so that legacyvoidreturn handlers continue to behave as before.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `process?: (selection) => boolean | void` contract combined with `if (!matcher.process(nextSelection))` will treat existing `process` implementations that return `void` as a cancellation, effectively disabling those transformers; consider checking explicitly for `false` (e.g. `if (matcher.process(nextSelection) === false)`) so that legacy `void` return handlers continue to behave as before.
## Individual Comments
### Comment 1
<location> `src/plugins/markdown/service/transformers.ts:328` </location>
<code_context>
if (matcher.process) {
- matcher.process(nextSelection);
+ if (!matcher.process(nextSelection)) {
+ // If process function returns false, cancel the transform and set selection to original position
+ $setSelection(anchorNode.selectEnd());
</code_context>
<issue_to_address>
**issue (bug_risk):** Distinguish explicit `false` from `undefined` when handling `process` return value to avoid breaking existing transformers.
With the new type `process?: (selection: RangeSelection) => boolean | void`, existing `process` functions that return nothing now yield `undefined`, which is falsy and will cancel the transform. Previously, the return value was ignored and all `process` handlers ran, so this change breaks backward compatibility. To preserve existing behavior while adding an explicit cancel signal, capture the return value and only cancel when it is strictly `false`, for example:
```ts
if (matcher.process) {
const result = matcher.process(nextSelection);
if (result === false) {
$setSelection(/* original selection */);
continue;
}
return true;
}
```
This keeps `void` returns as "proceed" and uses `false` to opt into cancellation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
commit: |
…false to cancel transformation
added 2 commits
February 10, 2026 07:12
…ormers when process function cancels transformation
…when process function cancels transformation
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
2 participants
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.
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
📝 补充信息 | Additional Information
Summary by Sourcery
Prevent markdown inline code shortcut from creating nested code nodes and allow text format transformers to cancel transformations via a boolean return value.
Bug Fixes:
Enhancements: