language: Fix auto-indent overwriting manual indentation when replacing a line's contents - #62644
Open
lingyaochu wants to merge 2 commits into
Open
language: Fix auto-indent overwriting manual indentation when replacing a line's contents#62644lingyaochu wants to merge 2 commits into
lingyaochu wants to merge 2 commits into
Conversation
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.
Objective
Closes #62617
Turns out #62617 is just a special trigger point of a more general issue: replacing a line's contents can silently rewrite the line's indentation with the auto-indent suggestion.
Consider the following Rust code, where the line has an extra tab, making its indent 8 spaces instead of the default 4:
If we select and replace the line's contents (without the indentation):
with
let a = 8;, the result is:The extra indent has been stripped.
Tracing this down to
Buffer::edit_internal()incrates/language/src/buffer.rs, the code decides whether the edited line needs an indent update via thefirst_line_is_newflag, which ends up as theold_rowof anAutoindentRequestEntry. One of these checks is:zed/crates/language/src/buffer.rs
Lines 2913 to 2918 in cdc537c
When replacing a line's contents, the edit range ends exactly at the end of the line, so
old_start.column + (range_len as u32) == old_line_end. Because the check uses<, this case meets none of the these conditions,first_line_is_newstaystrue, and an indent update is triggered. If the manual indent differs from the suggested indent, it gets overwritten — exactly as in the example above.For IME input, composition updates replace the previously marked preedit text, which sits at the end of the line — the same geometry as a full line-content replacement. In some environments (observed on KDE Wayland with fcitx), a single keystroke delivers the preedit update twice, so the replacement happens on the very first keystroke, which is what #62617 reports. On other platforms, the replacement may happens once the composition changes, i.e. on the second keystroke, so it takes at least two characters to trigger.
Solution
Simply change the guard from
(old_start.column + (range_len as u32) < old_line_endto(old_start.column + (range_len as u32) <= old_line_end.Testing
Two new tests are added:
test_ime_composition_keeps_manual_indentcovers the IME input path, andtest_replacing_line_content_keeps_manual_indentcovers a plain line-content replacement.Self-Review Checklist:
Release Notes: