fix: keep the cursor anchored to its text through format-on-save - #2947
Merged
Conversation
When an on-save action rewrites the buffer (format-on-save, trim trailing whitespace, ensure final newline), the cursor was restored as a raw byte offset clamped to the new length. Whenever the rewrite changed lengths before the cursor, that offset landed in unrelated text — e.g. a formatter deleting three blank lines above the cursor dropped it two lines away into a different word (#2777, split off from #2706). Map the cursor and any selection anchor through a diff of the old vs. new content instead: reuse the existing line-level patience diff (fresh-core's compute_line_diff) to shift offsets by the byte delta of preceding hunks, and refine offsets inside a rewritten region by the region's common prefix/suffix — so reindenting a line keeps the cursor at its column, and an offset whose own text was removed snaps to the start of the replacement (VS Code semantics). The mapping is one line diff plus O(lines) offset tables, so large files stay cheap, and results always land on char boundaries. The whole-buffer delete+insert mechanism and its undo behavior (#2027's leading cursor-restore event) are deliberately unchanged; only the restored positions are computed differently. A selection whose cursor maps to the buffer end now also keeps its anchor (the restore event was previously skipped in that case). Fixes #2777 Refs #2706 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
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
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.
Fixes #2777
Refs #2706 (the cursor half of that issue; its on-disk-state half was handled separately)
Motivation
When an on-save action rewrites the buffer (format-on-save, trim-trailing-whitespace, ensure-final-newline),
replace_buffer_with_outputrestored the cursor as a raw byte offset clamped to the new length (old_cursor_pos.min(new_buffer_len)). Whenever the rewrite changed lengths before the cursor, that offset landed in unrelated text. Deterministic repro from #2777: a blank-line-deleting formatter with the cursor at the end ofMARKER xyz(Ln 5, Col 11) dropped the cursor intoomegaat Ln 3, Col 3 instead of following its text to Ln 2, Col 11.The fix
Map the cursor and any selection anchor through a diff of the pre- vs. post-format content, exactly as the issue proposes, reusing the existing line-level patience diff (
fresh-core'scompute_line_diff— no new dependency, no new diff machinery):fresh_core::diff::map_offset_through_diff(old, new, offset): walks the line hunks accumulating byte deltas; an offset outside every hunk shifts by the delta of preceding hunks (stays anchored to its text); an offset inside a rewritten region is refined by that region's char-level common prefix/suffix (so reindenting a line keeps the cursor at the end of its text), else snaps to the start of the replacement (VS Code semantics). Cost is one patience diff (which trims the common prefix/suffix first) plus O(lines) offset tables — no quadratic work on large files, and results always land on char boundaries.replace_buffer_with_outputnow computes the restored cursor and selection anchor with this mapping. The whole-buffer delete+insert mechanism and its undo behavior (Feature Request: Run Formatter on Save / Don't move view after undoing a format update #2027's leading cursor-restore event) are deliberately unchanged — only the restored positions differ. A selection whose cursor maps to the buffer end now also keeps its anchor (the restore event was previously skipped in that case).This intentionally stays minimal per the complexity constraints: it does not convert the formatter result into minimal per-hunk edit events (which would additionally preserve markers and improve undo granularity) — that remains a possible follow-up.
Tests
All added tests fail without the fix (verified by reverting
on_save_actions.rsand re-running) and pass with it:map_offset_through_diffinfresh-core/src/diff.rs: the Format-on-save moves the cursor to a wrong/random position (needs diff-based cursor mapping) #2777 blank-line-deletion repro, the Format On Save #2706 reindentation repro (common-suffix refinement), offsets before all edits, inside a deleted region (snap to replacement start), between two hunks (shift by the earlier delta only), no-op formatter identity, clamping, and multibyte char-boundary safety.tests/e2e/on_save_actions.rsdriving Ctrl+S with agrep -v '^$'formatter and asserting only on rendered output:test_format_on_save_keeps_cursor_anchored_to_its_text(status bar readsLn 2, Col 11after format; readLn 3, Col 3before the fix) andtest_format_on_save_cursor_before_edit_stays_put(control: cursor before the edited region does not move).Ran:
cargo test -p fresh-core --lib(253 passed),cargo test -p fresh-editor --test e2e_tests on_save(17 passed, includes all pre-existing on-save tests),cargo test -p fresh-editor --test format_on_save_undo_view_test(16 passed — #2027 undo-view regression intact),cargo fmt,cargo clippy -p fresh-coreand-p fresh-editor --all-targets(no new warnings). Also validated manually in tmux against a debug build: the exact #2777 repro now lands onLn 2, Col 11.🤖 Generated with Claude Code
https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
Generated by Claude Code