Fix bad unicode calculations in do_completion (#28259)

Conrad Irwin and João Marcos created 8 months ago

Co-authored-by: João Marcos <[e-mail]>

Release Notes:

- Fixed a panic with completions around non-ASCII code

---------

Co-authored-by: João Marcos <[e-mail]>

Change summary

crates/editor/src/editor.rs     | 36 +++++++++++----------
crates/vim/src/normal/repeat.rs | 56 +++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+), 17 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -4610,14 +4610,17 @@ impl Editor {
         let lookahead = old_range
             .end
             .saturating_sub(newest_selection.end.text_anchor.to_offset(buffer));
-        let mut common_prefix_len = old_text
-            .bytes()
-            .zip(new_text.bytes())
-            .take_while(|(a, b)| a == b)
-            .count();
+        let mut common_prefix_len = 0;
+        for (a, b) in old_text.chars().zip(new_text.chars()) {
+            if a == b {
+                common_prefix_len += a.len_utf8();
+            } else {
+                break;
+            }
+        }
 
         let snapshot = self.buffer.read(cx).snapshot(cx);
-        let mut range_to_replace: Option<Range<isize>> = None;
+        let mut range_to_replace: Option<Range<usize>> = None;
         let mut ranges = Vec::new();
         let mut linked_edits = HashMap::<_, Vec<_>>::default();
         for selection in &selections {
@@ -4625,10 +4628,7 @@ impl Editor {
                 let start = selection.start.saturating_sub(lookbehind);
                 let end = selection.end + lookahead;
                 if selection.id == newest_selection.id {
-                    range_to_replace = Some(
-                        ((start + common_prefix_len) as isize - selection.start as isize)
-                            ..(end as isize - selection.start as isize),
-                    );
+                    range_to_replace = Some(start + common_prefix_len..end);
                 }
                 ranges.push(start + common_prefix_len..end);
             } else {
@@ -4636,12 +4636,7 @@ impl Editor {
                 ranges.clear();
                 ranges.extend(selections.iter().map(|s| {
                     if s.id == newest_selection.id {
-                        range_to_replace = Some(
-                            old_range.start.to_offset_utf16(&snapshot).0 as isize
-                                - selection.start as isize
-                                ..old_range.end.to_offset_utf16(&snapshot).0 as isize
-                                    - selection.start as isize,
-                        );
+                        range_to_replace = Some(old_range.clone());
                         old_range.clone()
                     } else {
                         s.start..s.end
@@ -4667,8 +4662,15 @@ impl Editor {
         }
         let text = &new_text[common_prefix_len..];
 
+        let utf16_range_to_replace = range_to_replace.map(|range| {
+            let newest_selection = self.selections.newest::<OffsetUtf16>(cx).range();
+            let selection_start_utf16 = newest_selection.start.0 as isize;
+
+            range.start.to_offset_utf16(&snapshot).0 as isize - selection_start_utf16
+                ..range.end.to_offset_utf16(&snapshot).0 as isize - selection_start_utf16
+        });
         cx.emit(EditorEvent::InputHandled {
-            utf16_range_to_replace: range_to_replace,
+            utf16_range_to_replace,
             text: text.into(),
         });
 

crates/vim/src/normal/repeat.rs

@@ -482,6 +482,62 @@ mod test {
         );
     }
 
+    #[gpui::test]
+    async fn test_repeat_completion_unicode_bug(cx: &mut gpui::TestAppContext) {
+        VimTestContext::init(cx);
+        let cx = EditorLspTestContext::new_rust(
+            lsp::ServerCapabilities {
+                completion_provider: Some(lsp::CompletionOptions {
+                    trigger_characters: Some(vec![".".to_string(), ":".to_string()]),
+                    resolve_provider: Some(true),
+                    ..Default::default()
+                }),
+                ..Default::default()
+            },
+            cx,
+        )
+        .await;
+        let mut cx = VimTestContext::new_with_lsp(cx, true);
+
+        cx.set_state(
+            indoc! {"
+                ĩлˇк
+                ĩлк
+            "},
+            Mode::Normal,
+        );
+
+        let mut request = cx.set_request_handler::<lsp::request::Completion, _, _>(
+            move |_, params, _| async move {
+                let position = params.text_document_position.position;
+                let mut to_the_left = position;
+                to_the_left.character -= 2;
+                Ok(Some(lsp::CompletionResponse::Array(vec![
+                    lsp::CompletionItem {
+                        label: "oops".to_string(),
+                        text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+                            range: lsp::Range::new(to_the_left, position),
+                            new_text: "к!".to_string(),
+                        })),
+                        ..Default::default()
+                    },
+                ])))
+            },
+        );
+        cx.simulate_keystrokes("i .");
+        request.next().await;
+        cx.condition(|editor, _| editor.context_menu_visible())
+            .await;
+        cx.simulate_keystrokes("enter escape");
+        cx.assert_state(
+            indoc! {"
+                ĩкˇ!к
+                ĩлк
+            "},
+            Mode::Normal,
+        );
+    }
+
     #[gpui::test]
     async fn test_repeat_visual(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;