Handle snippets for LSPs not providing offsets for completion

helix-lsp/src/lib.rs

@@ -252,26 +252,17 @@ pub mod util {
     pub fn generate_transaction_from_completion_edit(
         doc: &Rope,
         selection: &Selection,
-        edit: lsp::TextEdit,
+        start_offset: i128,
-        offset_encoding: OffsetEncoding,
+        end_offset: i128,
+        new_text: String,
     ) -> Transaction {
-        let replacement: Option<Tendril> = if edit.new_text.is_empty() {
+        let replacement: Option<Tendril> = if new_text.is_empty() {
             None
         } else {
-            Some(edit.new_text.into())
+            Some(new_text.into())
         };
 
         let text = doc.slice(..);
-        let primary_cursor = selection.primary().cursor(text);
-
-        let start_offset = match lsp_pos_to_pos(doc, edit.range.start, offset_encoding) {
-            Some(start) => start as i128 - primary_cursor as i128,
-            None => return Transaction::new(doc),
-        };
-        let end_offset = match lsp_pos_to_pos(doc, edit.range.end, offset_encoding) {
-            Some(end) => end as i128 - primary_cursor as i128,
-            None => return Transaction::new(doc),
-        };
 
         Transaction::change_by_selection(doc, selection, |range| {
             let cursor = range.cursor(text);
@@ -288,23 +279,13 @@ pub mod util {
     pub fn generate_transaction_from_snippet(
         doc: &Rope,
         selection: &Selection,
-        edit_range: &lsp::Range,
+        start_offset: i128,
+        end_offset: i128,
         snippet: snippet::Snippet,
         line_ending: &str,
         include_placeholder: bool,
-        offset_encoding: OffsetEncoding,
     ) -> Transaction {
         let text = doc.slice(..);
-        let primary_cursor = selection.primary().cursor(text);
-
-        let start_offset = match lsp_pos_to_pos(doc, edit_range.start, offset_encoding) {
-            Some(start) => start as i128 - primary_cursor as i128,
-            None => return Transaction::new(doc),
-        };
-        let end_offset = match lsp_pos_to_pos(doc, edit_range.end, offset_encoding) {
-            Some(end) => end as i128 - primary_cursor as i128,
-            None => return Transaction::new(doc),
-        };
 
         // For each cursor store offsets for the first tabstop
         let mut cursor_tabstop_offsets = Vec::<SmallVec<[(i128, i128); 1]>>::new();

helix-term/src/ui/completion.rs

@@ -121,8 +121,9 @@ impl Completion {
                 include_placeholder: bool,
             ) -> Transaction {
                 use helix_lsp::snippet;
+                let selection = doc.selection(view_id);
 
-                if let Some(edit) = &item.text_edit {
+                let (start_offset, end_offset, new_text) = if let Some(edit) = &item.text_edit {
                     let edit = match edit {
                         lsp::CompletionTextEdit::Edit(edit) => edit.clone(),
                         lsp::CompletionTextEdit::InsertAndReplace(item) => {
@@ -130,6 +131,41 @@ impl Completion {
                             lsp::TextEdit::new(item.replace, item.new_text.clone())
                         }
                     };
+                    let text = doc.text().slice(..);
+                    let primary_cursor = selection.primary().cursor(text);
+
+                    let start_offset =
+                        match util::lsp_pos_to_pos(doc.text(), edit.range.start, offset_encoding) {
+                            Some(start) => start as i128 - primary_cursor as i128,
+                            None => return Transaction::new(doc.text()),
+                        };
+                    let end_offset =
+                        match util::lsp_pos_to_pos(doc.text(), edit.range.end, offset_encoding) {
+                            Some(end) => end as i128 - primary_cursor as i128,
+                            None => return Transaction::new(doc.text()),
+                        };
+
+                    (start_offset, end_offset, edit.new_text)
+                } else {
+                    let new_text = item.insert_text.as_ref().unwrap_or(&item.label);
+                    // Some LSPs just give you an insertText with no offset ¯\_(ツ)_/¯
+                    // in these cases we need to check for a common prefix and remove it
+                    let prefix = Cow::from(doc.text().slice(start_offset..trigger_offset));
+                    let new_text = new_text.trim_start_matches::<&str>(&prefix);
+
+                    // TODO: this needs to be true for the numbers to work out correctly
+                    // in the closure below. It's passed in to a callback as this same
+                    // formula, but can the value change between the LSP request and
+                    // response? If it does, can we recover?
+                    debug_assert!(
+                        doc.selection(view_id)
+                            .primary()
+                            .cursor(doc.text().slice(..))
+                            == trigger_offset
+                    );
+
+                    (0, 0, new_text.into())
+                };
 
                 if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET))
                     || matches!(
@@ -137,20 +173,20 @@ impl Completion {
                         Some(lsp::InsertTextFormat::SNIPPET)
                     )
                 {
-                        match snippet::parse(&edit.new_text) {
+                    match snippet::parse(&new_text) {
                         Ok(snippet) => util::generate_transaction_from_snippet(
                             doc.text(),
-                                doc.selection(view_id),
+                            selection,
-                                &edit.range,
+                            start_offset,
+                            end_offset,
                             snippet,
                             doc.line_ending.as_str(),
                             include_placeholder,
-                                offset_encoding,
                         ),
                         Err(err) => {
                             log::error!(
                                 "Failed to parse snippet: {:?}, remaining output: {}",
-                                    &edit.new_text,
+                                &new_text,
                                 err
                             );
                             Transaction::new(doc.text())
@@ -159,35 +195,12 @@ impl Completion {
                 } else {
                     util::generate_transaction_from_completion_edit(
                         doc.text(),
-                            doc.selection(view_id),
+                        selection,
-                            edit,
+                        start_offset,
-                            offset_encoding, // TODO: should probably transcode in Client
+                        end_offset,
+                        new_text,
                     )
                 }
-                } else {
-                    let text = item.insert_text.as_ref().unwrap_or(&item.label);
-                    // Some LSPs just give you an insertText with no offset ¯\_(ツ)_/¯
-                    // in these cases we need to check for a common prefix and remove it
-                    let prefix = Cow::from(doc.text().slice(start_offset..trigger_offset));
-                    let text = text.trim_start_matches::<&str>(&prefix);
-
-                    // TODO: this needs to be true for the numbers to work out correctly
-                    // in the closure below. It's passed in to a callback as this same
-                    // formula, but can the value change between the LSP request and
-                    // response? If it does, can we recover?
-                    debug_assert!(
-                        doc.selection(view_id)
-                            .primary()
-                            .cursor(doc.text().slice(..))
-                            == trigger_offset
-                    );
-
-                    Transaction::change_by_selection(doc.text(), doc.selection(view_id), |range| {
-                        let cursor = range.cursor(doc.text().slice(..));
-
-                        (cursor, cursor, Some(text.into()))
-                    })
-                }
             }
 
             fn completion_changes(transaction: &Transaction, trigger_offset: usize) -> Vec<Change> {