From 34d68ec5e6f3c01d159539022d9153e11f0a6cbf Mon Sep 17 00:00:00 2001 From: Kashish Sahu <125540624+gitKashish@users.noreply.github.com> Date: Thu, 8 Oct 2026 11:39:56 +0000 Subject: [PATCH] editor: Fix text duplication when converting case with multiple cursors in one word (#65291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- crates/editor/src/editor.rs | 44 ++++++++++++++++++++++++------- crates/editor/src/editor_tests.rs | 38 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6051d7f4848..3839a9bb685 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -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::(); + // 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> = 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::(); 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)); } } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 09465235150..5e255697cfc 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -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, |_| {});