[diffs/edit] Fix edit stack coalescing#1000
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Fixes EditStack undo/redo coalescing behavior in packages/diffs so new edits don’t incorrectly merge into stale history after undo/redo, and so backspace vs forward-delete direction changes introduce an undo stop (instead of being merged into one step).
Changes:
- Close the active coalescing group on undo/redo via
EditStack.peekUndoForCoalescing()+ internal state, preventing merges into newly exposed (stale/boundary-hidden) undo entries. - Track and persist delete direction using
coalescingModeso backspace/delete flips don’t coalesce when geometry is ambiguous. - Promote previously
test.failingcoalescing scenarios to passing tests; remove the corresponding “known bugs” note; fix thefindNextMatchnaming typo across code/tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/diffs/test/README.md | Removes the “known bugs” entry now covered/fixed by the updated coalescing behavior. |
| packages/diffs/test/editorSelection.test.ts | Renames test usage from findNexMatch to findNextMatch. |
| packages/diffs/test/editorEditStack.test.ts | Converts previously failing undo/redo + delete-direction coalescing scenarios into normal passing tests; adds coverage for direction flips. |
| packages/diffs/src/editor/textDocument.ts | Switches coalescing comparison to peekUndoForCoalescing() so undo/redo closes coalescing groups. |
| packages/diffs/src/editor/selection.ts | Renames exported helper to findNextMatch. |
| packages/diffs/src/editor/editStack.ts | Implements coalescing-group gating (#canCoalesce) and adds coalescingMode to preserve delete direction across coalesced runs. |
| packages/diffs/src/editor/editor.ts | Updates import/call site to use findNextMatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0185a5ed3a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if ( | ||
| selection.start.line !== selection.end.line || | ||
| selection.start.character !== selection.end.character | ||
| ) { | ||
| break; |
There was a problem hiding this comment.
When Backspace/Delete is invoked through the editor path, applyDeleteCharacterToSelections passes the expanded delete ranges as selectionsBefore, not the original collapsed carets. This branch therefore breaks on the non-collapsed selection and leaves coalescingMode unset, so a Backspace followed by forward Delete at the same pivot still coalesces in production UI; on abc, Backspace after b then Delete causes one undo to restore both b and c instead of just c.
Useful? React with 👍 / 👎.
* fix typo * fix editStack coalescing bugs.
Uh oh!
There was an error while loading. Please reload this page.