[lexical-code-core][lexical-code-shiki][lexical-code-prism] Feature: Outdent space-indented code lines#8445
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
etrepum
left a comment
There was a problem hiding this comment.
It looks like the code is substantially the same between the prism and shiki support for this, maybe there should be some common functionality here like a CodeIndentExtension or something lower-level (register function) that they both depend on?
0f8dfc6 to
a44af6c
Compare
|
Pushed the extraction as About 1000 lines of duplicated handlers and helpers collapsed into one module ( |
etrepum
left a comment
There was a problem hiding this comment.
Love the code consolidation here, much nicer to have it in one place instead of two!
What I don't think we should do is break backwards compatibility, by making the exported registerCodeHighlighting functions a wrapper that does all of the legacy initialization (highlighting + indentation) we can add this configurability to the extensions without changing anything for legacy code.
a44af6c to
5a5e611
Compare
|
Reworked per your suggestion.
Tests updated accordingly. tsc / flow / vitest green locally. |
|
That’s not the refactoring I was suggesting, there should be no worry of double registration or any configuration forwarding. The extensions should simply not use the same register functions that are exported for legacy |
…Outdent space-indented code lines Shift+Tab in a code block currently only removes a leading TabNode, so prettier-formatted code (which uses spaces for indentation) cannot be outdented by the keyboard shortcut. This PR adds an optional tabSize configuration; when set, the OUTDENT_CONTENT_COMMAND also strips up to that many leading spaces from a code line. When tabSize is undefined (the default), the original TabNode-only outdent behavior is preserved. Per line in the selection, with tabSize set: - TabNode at the start: remove it (existing behavior takes precedence). - CodeHighlightNode starting with spaces: strip min(tabSize, leadingSpaces). Best-effort, matching VS Code. - Otherwise: no-op. Selection is preserved relative to line content. When stripping K characters, anchor/focus offsets pointing into the affected node shift left by K (clamped to 0), so the cursor stays on the same character rather than snapping to the start of the line. Tab and INSERT_TAB_COMMAND continue to insert a TabNode regardless of tabSize. The keyboard / move handlers (Tab / Shift+Tab, alt+arrow line shifts, Home/End) were duplicated verbatim between CodeHighlighterShiki and CodeHighlighterPrism. This change moves them to a new @lexical/code-core CodeIndentation module exposed in two ways: - registerCodeIndentation(editor, tabSize?): a low-level register function. Used internally by the legacy registerCodeHighlighting wrapper, and exported standalone for callers who want indent without a highlighter. - CodeIndentExtension: an Extension wrapping registerCodeIndentation with signal-based config (disabled, tabSize). Both CodeShikiExtension and CodePrismExtension declare it as a dependency, so Extension users configure tabSize on CodeIndentExtension. To preserve backwards compatibility, the legacy registerCodeHighlighting function keeps its existing signature and continues to register both the highlighter transforms and the indent / Tab handlers as it always did. The Extension flow uses a separate internal helper (registerHighlightingOnly) for the highlighter side and depends on CodeIndentExtension for the indent side, so the legacy export and the Extensions are wired independently. The space-stripping helper $outdentLeadingSpaces lives in @lexical/code-core too, called from $handleMultilineIndent. Both extensions' test suites use a shared $runOutdentScenario helper and OUTDENT_SCENARIOS data table from @lexical/code-core/__tests__/outdentTestUtils, driven through test.for. Cases cover tabSize=2/4 with full and partial leading spaces, the TabNode-precedence behavior, the tabSize=undefined backward-compat path, cursor preservation, and rejection of non-positive / non-integer tabSize. Closes facebook#8410
5a5e611 to
bbf43e8
Compare
|
Got it now, sorry for the round trip. Restructured so the two paths are wired independently:
tsc / flow / 225 unit tests pass locally. |
…ActionsPlugin (html & markdown mode) (facebook#8444)
…stNode and use it for children normalization (facebook#8427) Co-authored-by: Bob Ippolito <bob@redivi.com>
…opt-in format escape at text node boundaries (facebook#8383) Co-authored-by: Sergey Gorbachev <grbchv.s@gmail.com> Co-authored-by: Sherry <potatowagon@meta.com> Co-authored-by: Bob Ippolito <bob@redivi.com> Co-authored-by: Agyei Holy <agyeiholy978@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
…k#8448) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… update Links test for adjacent-link merge The npm→pnpm migration regressed the e2e workflow so the flaky-matrix job no longer ran the @flaky-tagged tests. The reusable workflow had hardcoded `--grep-invert "@flaky"` for both the regular and collab test steps, so the `flaky: true` matrix job ran the same suite as the non-flaky job (with continue-on-error: true), making it always green. Make the grep argument depend on `inputs.flaky` so the flaky-matrix job actually runs the @flaky-tagged tests. Re-running the flaky tests locally surfaces: - Links.spec.mjs:410 ("Can create a link with some text after, insert paragraph, then backspace, it should merge correctly"): updated the post-backspace assertion to match the current adjacent-link merge behavior (the two split <a> nodes are now merged back into one when the paragraphs join, which is what shouldMergeAdjacentLink does). - Tab.spec.mjs:22 (Tab + IME) and Toolbar.spec.mjs:43 (Insert image caption + table in collab) still fail. They are real bugs that need separate fixes; left tagged @flaky so the flaky-matrix job surfaces them without blocking CI.
…YJS sync; surface IME keydown for Tab+IME flaky test Tab.spec.mjs "can tab + IME": call enableCompositionKeyEvents (the same helper used by Composition/History IME tests) after focusEditor. Without it, the second CDP imeSetComposition after a TabNode insertion silently dropped its insertText and the second `すし` never appeared. lexical-yjs $syncPropertiesFromYjs: when a property already holds a LexicalEditor (e.g. ImageNode.__caption, built with buildEditorFromExtensions in the constructor) and YJS delivers a Doc, reuse the existing editor instead of replacing it with a bare createEditor(). Replacing it dropped the LexicalBuilder symbol and every registered extension, so any subsequent render through LexicalExtensionEditorComposer or getExtensionDependencyFromEditor would throw "LexicalBuilder.fromEditor: The given editor was not created with LexicalBuilder" and crash the right frame's React tree. This unblocks Toolbar.spec.mjs:43 "Insert image caption + table" in collab mode through the React render path; the remaining caption-text sync between frames hits a separate dependency-version mismatch in package.json (yjs@13 client vs @y/websocket-server@0.1.5 needing yjs@14) and is left for a follow-up.
…et; simplify nested-editor reuse with isLexicalEditor - Drop @y/websocket-server (its server bin uses yjs@14, which can't decode updates from this codebase's yjs@13 client and surfaces as "TypeError: store.getClock is not a function" on every WS message, silently breaking subdoc sync). Pin y-websocket to ^2.1.0, which re-introduces the server bin and stays on yjs@13. - Update the docs install snippet to drop @y/websocket-server. - Simplify the nested-editor preservation guard in $syncPropertiesFromYjs to use isLexicalEditor(prevValue) (per review). Status: Tab+IME and Links flaky tests pass; the Toolbar "Insert image caption + table" collab test no longer crashes the right frame and the caption editor is now mounted, but the typed caption text still does not propagate from left to right. Investigation continues.
…g to an empty Yjs doc When CollaborationPlugin binds an editor that was built with a non-empty initial state (e.g. a nested editor like ImageNode's caption editor, which is constructed via buildEditorFromExtensions and starts with a default empty paragraph) to a fresh empty Yjs doc, bootstrap's initializeEditor was a no-op because the Lexical root was already non-empty. The CollabElementNode root then never picked up the pre-existing paragraph, leaving binding.root._children = [] while $getRoot() reported [paragraphKey]. On the next edit, syncChildrenFromLexical saw prevChildren=nextChildren (matching keys), took the "no move/create/remove" branch, and called _syncChildFromLexical which silently no-oped because this._children[nextIndex] was undefined and none of the instanceof branches matched. Result: every transaction ran, the wrapping binding.doc.transact wrote nothing, no Yjs structs were produced, and the caption text never propagated to other clients. Detect that case and create the missing CollabNode (which recursively exports the Lexical subtree to Yjs via $createCollabNodeFromLexicalNode), then splice it into the binding. Subsequent typings then sync normally. This fixes the Toolbar.spec.mjs:43 "Insert image caption + table" test in collab mode (the third and last @flaky-tagged test that was deterministically failing once the flaky-matrix CI job was wired up).
…n binding to an empty Yjs doc" This reverts commit 2ceefb7.
…caption editor in collab mode CollaborationPlugin's bootstrap only runs initializeEditor when the Lexical root is empty. The CaptionEditor was built via buildEditorFromExtensions, where InitialStateExtension's $defaultInitializer eagerly appends an empty paragraph during afterRegistration. With that paragraph already there, bootstrap never runs, so the caption editor's state never gets exported to Yjs and typed text never propagates to the other client. Set \$initialEditorState: null on the CaptionEditorExtension so the caption editor starts with a truly empty root. Now bootstrap runs in collab mode and adds the paragraph through the normal sync path. In non-collab mode RichText's normalization handles the empty root by adding a paragraph as soon as the editor mounts. This was the last @flaky-tagged test (Toolbar.spec.mjs:43 "Insert image caption + table") that was deterministically failing once the flaky matrix CI job was wired up.
…' into feat/code-block-shift-tab-spaces
There was a problem hiding this comment.
I did some additional cleanup to the tests. When verifying them I stumbled upon an issue with the test CI workflow, which led to a few unrelated tests that needed to be fixed because they were broken but weren't previously running in CI, and a yjs dependency that needed to be rolled back because it was pulling in the unstable yjs 14.
The playground is now using tabSize: 2 so this extension is easier to exercise in QA
|
Thanks for picking up the rest of the inline-comment cleanup — those should have been on me. The internal/legacy split with |
Description
Shift+Tab in a code block currently only removes a leading
TabNode, so prettier-formatted code (which uses spaces for indentation) cannot be outdented by the keyboard shortcut. This PR adds an optionaltabSizeconfig toCodeShikiExtensionandCodePrismExtensionthat, when set, makes theOUTDENT_CONTENT_COMMANDhandler also strip up to that many leading spaces from a code line.When
tabSizeisundefined(the default), the originalTabNode-only behavior is preserved.Demo
2026-05-02.4.37.13.mov
Behavior
Per line in the selection, with
tabSizeset:TabNodeat the start: remove it (existing behavior takes precedence).CodeHighlightNodestarting with spaces: stripmin(tabSize, leadingSpaces). Best-effort (same as VS Code): a line with fewer thantabSizeleading spaces is fully outdented in one keypress.Selection is preserved relative to line content. When stripping
Kcharacters, anchor/focus offsets pointing into the affected node shift left byK(clamped to 0), so the cursor stays on the same character rather than snapping to the start of the line.The space-stripping logic lives in
@lexical/code-coreas$outdentLeadingSpacesand is called from both highlighter extensions.Out of scope
TabandINSERT_TAB_COMMANDcontinue to insert aTabNoderegardless oftabSize. This keeps the change focused on the bug reported in #8410.Design question
When
tabSizeis set, shouldTab/INSERT_TAB_COMMANDalso switch to insertingtabSizespaces (instead of aTabNode), for consistency? Three possible scopes:Shift+Tabstrips spaces;Tabstill inserts aTabNode. Mixed indentation possible after editing.TabinsertsNspaces whentabSizeis set; new indentation is space-only. ExistingTabNodes left alone.Tabinserts spaces, plus a transform converts existingTabNodes to spaces whentabSizeis set.(3) is probably its own feature, since auto-conversion mid-editing would be unexpected. I went with (1) for now. If you think (2) is the right scope, I'll extend this PR to cover it.
Test plan
pnpm test-unit— 2412 pass. NewOUTDENT_SCENARIOSdata table runs viatest.foragainst both@lexical/code-shiki(registerCodeHighlighting) and@lexical/code(which uses@lexical/code-prism'sregisterCodeHighlighting). Cases cover:tabSize=2/tabSize=4with full and partial leading spacesTabNode-precedence behaviortabSize=undefinedbackward-compat|hello→|hello, not| hello)tabSize=0) and non-integer (tabSize=2.5) valuespnpm run ci-check— typecheck, lint, prettier all clean.tabSize: 2onCodeShikiExtension, paste prettier-formatted code,Shift+Taboutdents one indent level per keypress and the cursor stays on the same character.Closes #8410