mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-13 00:42:42 +00:00
editor: Fix text duplication when converting case with multiple cursors in one word (#65291)
## Summary Closes #64273 Running a case conversion (or any other `manipulate_text` command) with several cursors inside the same word inserted the converted word once per cursor: | Before | Upper case result | |---|---| | `some_ˇvariˇable_naˇme` | `SOME_VARIABLE_NAMESOME_VARIABLE_NAMESOME_VARIABLE_NAME` | ### Cause `manipulate_text` expands each empty selection to its surrounding word, so several cursors in one word produce identical ranges, and a cursor's word can also overlap other selections. The buffer coalesces overlapping edits by concatenating their texts, so the converted text was inserted once per selection. ### Fix After expanding, the ranges are sorted by start and overlapping ones are merged, then each merged range is transformed once. Sorting matters because expanding a cursor can move its range's start before earlier selections (e.g. `«a»b«cd»e|f`, where the cursor's word starts at `a`). Selections that only touch are kept separate, so non-overlapping selections behave exactly as before. This applies to every command that goes through `manipulate_text`: all case conversions, ROT13/ROT47, and Base64 encode/decode. ## Testing Added `test_multicursor_manipulate_text`, which reports every failing case at once. It covers: - multiple cursors in one word, and in two different words - a cursor's word partially overlapping a selection - a cursor's word starting before earlier selections - a conversion that changes nothing, which keeps the cursors unchanged ## Self-review - [x] I've reviewed my diff for quality, security, reliability, and performance. - [x] Tests cover the new or changed behavior. Release Notes: - Fixed case conversion and other text transformation commands duplicating the word when multiple cursors are inside it.
This commit is contained in:
parent
f1a10a5227
commit
34d68ec5e6
2 changed files with 72 additions and 10 deletions
|
|
@ -7418,13 +7418,9 @@ impl Editor {
|
|||
}
|
||||
let buffer = self.buffer.read(cx).snapshot(cx);
|
||||
|
||||
let mut new_selections = Vec::new();
|
||||
let mut edits = Vec::new();
|
||||
|
||||
let mut selections = Vec::new();
|
||||
for selection in self.selections.all_adjusted(&self.display_snapshot(cx)) {
|
||||
let selection_is_empty = selection.is_empty();
|
||||
|
||||
let (start, end) = if selection_is_empty {
|
||||
let (start, end) = if selection.is_empty() {
|
||||
let (word_range, _) = buffer.surrounding_word(selection.start, None);
|
||||
(word_range.start, word_range.end)
|
||||
} else {
|
||||
|
|
@ -7433,20 +7429,48 @@ impl Editor {
|
|||
buffer.point_to_offset(selection.end),
|
||||
)
|
||||
};
|
||||
selections.push(Selection {
|
||||
id: selection.id,
|
||||
start,
|
||||
end,
|
||||
reversed: selection.reversed,
|
||||
goal: SelectionGoal::None,
|
||||
});
|
||||
}
|
||||
|
||||
let old_text = buffer.text_for_range(start..end).collect::<String>();
|
||||
// Expanding cursors to words can make selections overlap, e.g. several
|
||||
// cursors in one word. Overlapping edits would be applied once per
|
||||
// selection and duplicate the text, so transform each merged range once.
|
||||
selections.sort_by_key(|selection| selection.start);
|
||||
let mut merged_selections: Vec<Selection<MultiBufferOffset>> = Vec::new();
|
||||
for selection in selections {
|
||||
if let Some(previous) = merged_selections.last_mut()
|
||||
&& selection.start < previous.end
|
||||
{
|
||||
previous.end = cmp::max(previous.end, selection.end);
|
||||
} else {
|
||||
merged_selections.push(selection);
|
||||
}
|
||||
}
|
||||
|
||||
let mut new_selections = Vec::new();
|
||||
let mut edits = Vec::new();
|
||||
for selection in merged_selections {
|
||||
let old_text = buffer
|
||||
.text_for_range(selection.start..selection.end)
|
||||
.collect::<String>();
|
||||
let new_text = callback(&old_text);
|
||||
|
||||
new_selections.push(Selection {
|
||||
start: buffer.anchor_before(start),
|
||||
end: buffer.anchor_after(end),
|
||||
start: buffer.anchor_before(selection.start),
|
||||
end: buffer.anchor_after(selection.end),
|
||||
goal: SelectionGoal::None,
|
||||
id: selection.id,
|
||||
reversed: selection.reversed,
|
||||
});
|
||||
|
||||
if new_text != old_text {
|
||||
edits.push((start..end, new_text));
|
||||
edits.push((selection.start..selection.end, new_text));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8911,6 +8911,44 @@ async fn test_manipulate_text(cx: &mut TestAppContext) {
|
|||
"});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_multicursor_manipulate_text(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
let mut cx = EditorTestContext::new(cx).await;
|
||||
|
||||
let mut failures = Vec::new();
|
||||
for (before, expected, to_upper) in [
|
||||
("some_ˇvariˇable_naˇme", "«SOME_VARIABLE_NAMEˇ»", true),
|
||||
("some_ˇvariˇable naˇme", "«SOME_VARIABLEˇ» «NAMEˇ»", true),
|
||||
("SOME_ˇVARIˇABLE_NAˇME", "«some_variable_nameˇ»", false),
|
||||
("SOME_ˇVARIˇABLE NAˇME", "«some_variableˇ» «nameˇ»", false),
|
||||
("«word1ˇ» woˇr«d2 word3ˇ»", "«WORD1ˇ» «WORD2 WORD3ˇ»", true),
|
||||
("«aˇ»b«cdˇ»eˇf", "«ABCDEFˇ»", true),
|
||||
("hello woˇrˇld", "hello woˇrˇld", false),
|
||||
] {
|
||||
cx.set_state(before);
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
if to_upper {
|
||||
editor.convert_to_upper_case(&ConvertToUpperCase, window, cx)
|
||||
} else {
|
||||
editor.convert_to_lower_case(&ConvertToLowerCase, window, cx)
|
||||
}
|
||||
});
|
||||
let actual = cx.editor_state();
|
||||
if actual != expected {
|
||||
failures.push(format!(
|
||||
"before: {before}\nexpected: {expected}\nactual: {actual}"
|
||||
));
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
failures.is_empty(),
|
||||
"{} case(s) failed:\n\n{}",
|
||||
failures.len(),
|
||||
failures.join("\n\n")
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_duplicate_line(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue