Skip to content

[lexical-code-core] Bug Fix: Alt+Arrow no longer merges two lines when a code block line is blank - #8986

Closed
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix/code-shift-lines-blank
Closed

[lexical-code-core] Bug Fix: Alt+Arrow no longer merges two lines when a code block line is blank#8986
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix/code-shift-lines-blank

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

$handleShiftLines moves the selected code-block line(s) past the adjacent
line. It anchors the move on sibling — the node just past the linebreak that
separates the moving line from its neighbour — and resolves that to the
neighbour line's boundary node:

const maybeInsertionPoint =
  $isCodeHighlightNode(sibling) || $isTabNode(sibling) || $isLineBreakNode(sibling)
    ? arrowIsUp ? $getFirstCodeNodeOfLine(sibling) : $getLastCodeNodeOfLine(sibling)
    : null;

When the neighbour line is blank, sibling is itself a LineBreakNode
the one that terminates the line on the far side of the blank line.
$getFirstCodeNodeOfLine / $getLastCodeNodeOfLine only walk over
CodeHighlightNode/TabNode, so for a LineBreakNode anchor they return it
unchanged. The moving line is then spliced against a linebreak that belongs to
another line, which appends it to that line — the two lines merge and the text
is corrupted, not merely misplaced.

With a code block containing A, a blank line, and B:

  • Alt+ArrowUp on B produced AB\n\n instead of A\nB\n;
  • Alt+ArrowDown on A produced \n\nAB instead of \nA\nB.

A blank LineBreakNode sibling has no node of its own to anchor to, so treat
it as its own case: drop the moving line (and its linebreak) immediately after
that sibling, which is exactly the blank line's position. That is the same
insertion for both directions. Lines whose neighbour is not blank keep taking
the existing code path unchanged.

Test plan

Two new cases in
packages/lexical-code-core/src/__tests__/unit/CodeIndentation.test.ts.

Before

$ npx vitest run packages/lexical-code-core/src/__tests__/unit/CodeIndentation.test.ts

 FAIL  |unit| .../CodeIndentation.test.ts > CodeIndentExtension > shiftLines > moves a line up past a blank line without merging it into the line above
AssertionError: expected 'AB\n\n' to be 'A\nB\n' // Object.is equality

 FAIL  |unit| .../CodeIndentation.test.ts > CodeIndentExtension > shiftLines > moves a line down past a blank line without merging it into the line below
AssertionError: expected '\n\nAB' to be '\nA\nB' // Object.is equality

- Expected
+ Received

-
- A
- B
+
+ AB

 Test Files  1 failed (1)
      Tests  2 failed | 18 passed (20)

After

$ npx vitest run packages/lexical-code-core packages/lexical-code

 Test Files  11 passed (11)
      Tests  270 passed | 1 skipped (271)

The existing code blocks can shift lines (with tab) /
code blocks can shift multiple lines (with tab) unit tests and the
can move around code block with arrow keys e2e spec use no blank lines, so
their sibling is a CodeHighlightNode and they take the unchanged path.

…n a code block line is blank

## Description

`$handleShiftLines` moves the selected code-block line(s) past the adjacent
line. It anchors the move on `sibling` — the node just past the linebreak that
separates the moving line from its neighbour — and resolves that to the
neighbour line's boundary node:

```ts
const maybeInsertionPoint =
  $isCodeHighlightNode(sibling) || $isTabNode(sibling) || $isLineBreakNode(sibling)
    ? arrowIsUp ? $getFirstCodeNodeOfLine(sibling) : $getLastCodeNodeOfLine(sibling)
    : null;
```

When the neighbour line is **blank**, `sibling` is itself a `LineBreakNode` —
the one that terminates the line on the *far* side of the blank line.
`$getFirstCodeNodeOfLine` / `$getLastCodeNodeOfLine` only walk over
`CodeHighlightNode`/`TabNode`, so for a `LineBreakNode` anchor they return it
unchanged. The moving line is then spliced against a linebreak that belongs to
another line, which appends it to that line — the two lines merge and the text
is corrupted, not merely misplaced.

With a code block containing `A`, a blank line, and `B`:

- Alt+ArrowUp on `B` produced `AB\n\n` instead of `A\nB\n`;
- Alt+ArrowDown on `A` produced `\n\nAB` instead of `\nA\nB`.

A blank `LineBreakNode` sibling has no node of its own to anchor to, so treat
it as its own case: drop the moving line (and its linebreak) immediately after
that sibling, which is exactly the blank line's position. That is the same
insertion for both directions. Lines whose neighbour is not blank keep taking
the existing code path unchanged.

## Test plan

Two new cases in
`packages/lexical-code-core/src/__tests__/unit/CodeIndentation.test.ts`.

### Before

```
$ npx vitest run packages/lexical-code-core/src/__tests__/unit/CodeIndentation.test.ts

 FAIL  |unit| .../CodeIndentation.test.ts > CodeIndentExtension > shiftLines > moves a line up past a blank line without merging it into the line above
AssertionError: expected 'AB\n\n' to be 'A\nB\n' // Object.is equality

 FAIL  |unit| .../CodeIndentation.test.ts > CodeIndentExtension > shiftLines > moves a line down past a blank line without merging it into the line below
AssertionError: expected '\n\nAB' to be '\nA\nB' // Object.is equality

- Expected
+ Received

-
- A
- B
+
+ AB

 Test Files  1 failed (1)
      Tests  2 failed | 18 passed (20)
```

### After

```
$ npx vitest run packages/lexical-code-core packages/lexical-code

 Test Files  11 passed (11)
      Tests  270 passed | 1 skipped (271)
```

The existing `code blocks can shift lines (with tab)` /
`code blocks can shift multiple lines (with tab)` unit tests and the
`can move around code block with arrow keys` e2e spec use no blank lines, so
their `sibling` is a `CodeHighlightNode` and they take the unchanged path.
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@LeSingh1

Copy link
Copy Markdown
Contributor Author

Consolidated into #9056 with the other PRs that share this defect, per @etrepum's note on #9027 and @mayrang's on #9035. Same fix and same tests, one review.

@LeSingh1 LeSingh1 closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant