From fd9af64c4141c2a768d8d1747293f99c59c8a0be Mon Sep 17 00:00:00 2001 From: "Taylor C. Richberger" Date: Tue, 10 Jan 2023 12:21:44 -0700 Subject: [PATCH 1/6] allow LSP insert text to replace non-matching prefixes (#5469) Most LSPs will complete case-insensitive matches, particularly from lowercase to uppercase. In some cases, notably Pyright, this is given as a simple insert text instead of TextEdit. When this happens, the prefix text was left unedited. --- helix-term/src/ui/completion.rs | 42 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index ef88938fe33a..336b75cb46a8 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -124,6 +124,8 @@ impl Completion { ) -> Transaction { use helix_lsp::snippet; let selection = doc.selection(view_id); + let text = doc.text().slice(..); + let primary_cursor = selection.primary().cursor(text); let (start_offset, end_offset, new_text) = if let Some(edit) = &item.text_edit { let edit = match edit { @@ -133,8 +135,6 @@ 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) { @@ -149,24 +149,26 @@ impl Completion { (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()) + let new_text = item + .insert_text + .clone() + .unwrap_or_else(|| item.label.clone()); + + // check that we are still at the correct savepoint + // we can still generate a transaction regardless but if the + // document changed (and not just the selection) then we will + // likely delete the wrong text (same if we applied an edit sent by the LS) + debug_assert!(primary_cursor == trigger_offset); + + // TODO: Respect editor.completion_replace? + // Would require detecting the end of the word boundary for every cursor individually. + // We don't do the same for normal `edits, to be consistent we would have to do it for those too + + ( + start_offset as i128 - primary_cursor as i128, + trigger_offset as i128 - primary_cursor as i128, + new_text, + ) }; if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET)) From 83bdd48f7e401f0dea547e32ba9d1fd92303a509 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 9 Mar 2023 22:09:12 +0100 Subject: [PATCH 2/6] Add IntoIterator implementation for Selection --- helix-core/src/selection.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 0eb2b755ec4f..8e93c633e4ad 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -661,6 +661,15 @@ impl<'a> IntoIterator for &'a Selection { } } +impl IntoIterator for Selection { + type Item = Range; + type IntoIter = smallvec::IntoIter<[Range; 1]>; + + fn into_iter(self) -> smallvec::IntoIter<[Range; 1]> { + self.ranges.into_iter() + } +} + // TODO: checkSelection -> check if valid for doc length && sorted pub fn keep_or_remove_matches( From 0e436a5802d32fc6cd580de621ee3ed44d4798d0 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 9 Mar 2023 22:17:12 +0100 Subject: [PATCH 3/6] avoid allocations during snippet rendering --- helix-lsp/src/snippet.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/helix-lsp/src/snippet.rs b/helix-lsp/src/snippet.rs index b27077e7068c..4713ad8bbe80 100644 --- a/helix-lsp/src/snippet.rs +++ b/helix-lsp/src/snippet.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use anyhow::{anyhow, Result}; -use helix_core::{smallvec, SmallVec}; +use helix_core::{smallvec, SmallVec, Tendril}; #[derive(Debug, PartialEq, Eq)] pub enum CaseChange { @@ -57,10 +57,10 @@ pub fn parse(s: &str) -> Result> { fn render_elements( snippet_elements: &[SnippetElement<'_>], - insert: &mut String, + insert: &mut Tendril, offset: &mut usize, tabstops: &mut Vec<(usize, (usize, usize))>, - newline_with_offset: &String, + newline_with_offset: &str, include_placeholer: bool, ) { use SnippetElement::*; @@ -121,10 +121,10 @@ fn render_elements( #[allow(clippy::type_complexity)] // only used one time pub fn render( snippet: &Snippet<'_>, - newline_with_offset: String, + newline_with_offset: &str, include_placeholer: bool, -) -> (String, Vec>) { - let mut insert = String::new(); +) -> (Tendril, Vec>) { + let mut insert = Tendril::new(); let mut tabstops = Vec::new(); let mut offset = 0; @@ -133,7 +133,7 @@ pub fn render( &mut insert, &mut offset, &mut tabstops, - &newline_with_offset, + newline_with_offset, include_placeholer, ); From 96649aba6f0b538427ac8c0f5c6736a6b7c6c50f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 9 Mar 2023 22:19:14 +0100 Subject: [PATCH 4/6] Add API to create a Transaction from potentially overlapping changes This commit adds new functions to `Transaction` that allow creating edits that might potentially overlap. Any change that overlaps previous changes is ignored. Furthermore, a utility method is added that also drops selections associated with dropped changes (for transactions that are created from a selection). This is needed to avoid crashes when applying multicursor autocompletions, as the edit from a previous cursor may overlap with the next cursor/edit. --- helix-core/src/transaction.rs | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index d2f4de07dbe7..d8e581aae12f 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -1,3 +1,5 @@ +use smallvec::SmallVec; + use crate::{Range, Rope, Selection, Tendril}; use std::borrow::Cow; @@ -466,6 +468,33 @@ impl Transaction { self } + /// Generate a transaction from a set of potentially overlapping changes. The `change_ranges` + /// iterator yield the range (of removed text) in the old document for each edit. If any change + /// overlaps with a range overlaps with a previous range then that range is ignored. + /// + /// The `process_change` callback is called for each edit that is not ignored (in the order + /// yielded by `changes`) and should return the new text that the associated range will be + /// replaced with. + /// + /// To make this function more flexible the iterator can yield additional data for each change + /// that is passed to `process_change` + pub fn change_ignore_overlapping( + doc: &Rope, + change_ranges: impl Iterator, + mut process_change: impl FnMut(usize, usize, T) -> Option, + ) -> Self { + let mut last = 0; + let changes = change_ranges.filter_map(|(from, to, data)| { + if from < last { + return None; + } + let tendril = process_change(from, to, data); + last = to; + Some((from, to, tendril)) + }); + Self::change(doc, changes) + } + /// Generate a transaction from a set of changes. pub fn change(doc: &Rope, changes: I) -> Self where @@ -513,6 +542,44 @@ impl Transaction { Self::change(doc, selection.iter().map(f)) } + pub fn change_by_selection_ignore_overlapping( + doc: &Rope, + selection: &Selection, + mut change_range: impl FnMut(&Range) -> (usize, usize), + mut create_tendril: impl FnMut(usize, usize) -> Option, + ) -> (Transaction, Selection) { + let mut last_selection_idx = None; + let mut new_primary_idx = None; + let mut ranges: SmallVec<[Range; 1]> = SmallVec::new(); + let process_change = |change_start, change_end, (idx, range): (usize, &Range)| { + // update the primary idx + if idx == selection.primary_index() { + new_primary_idx = Some(idx); + } else if new_primary_idx.is_none() { + if idx > selection.primary_index() { + new_primary_idx = last_selection_idx; + } else { + last_selection_idx = Some(idx); + } + } + ranges.push(*range); + create_tendril(change_start, change_end) + }; + let transaction = Self::change_ignore_overlapping( + doc, + selection.iter().enumerate().map(|range| { + let (change_start, change_end) = change_range(range.1); + (change_start, change_end, range) + }), + process_change, + ); + + ( + transaction, + Selection::new(ranges, new_primary_idx.unwrap_or(0)), + ) + } + /// Insert text at each selection head. pub fn insert(doc: &Rope, selection: &Selection, text: Tendril) -> Self { Self::change_by_selection(doc, selection, |range| { From ecce1f7b94c393bd9df73d5beb8f2ebf309d4ad4 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 9 Mar 2023 23:08:55 +0100 Subject: [PATCH 5/6] fix snippet bugs and multicursor completion edgecases Multicursor completions may overlap and therefore overlapping completions must be dropped to avoid crashes. Furthermore, multicursor edits might simply be out of range if the word before/after the cursor is shorter. This currently leads to crashes, instead these selections are now also removed for completions. This commit also significantly refactors snippet transaction generation so that tabstops behave correctly with the above rules. Furthermore, snippet tabstops need to be carefully mapped to ensure their position is correct and consistent with our selection semantics. Finally, we now keep a partially updated Rope while creating snippet transactions so that we can fill information into snippets that depends on the position in the document. --- helix-lsp/src/lib.rs | 241 +++++++++++++++++++++++--------- helix-term/src/ui/completion.rs | 18 +-- 2 files changed, 180 insertions(+), 79 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 147b381c2a4e..58e8d83dc386 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -59,8 +59,8 @@ pub enum OffsetEncoding { pub mod util { use super::*; use helix_core::line_ending::{line_end_byte_index, line_end_char_index}; + use helix_core::{chars, RopeSlice, SmallVec}; use helix_core::{diagnostic::NumberOrString, Range, Rope, Selection, Tendril, Transaction}; - use helix_core::{smallvec, SmallVec}; /// Converts a diagnostic in the document to [`lsp::Diagnostic`]. /// @@ -247,13 +247,46 @@ pub mod util { Some(Range::new(start, end)) } + /// If the LS did not provide a range for the completion or the range of the + /// primary cursor can not be used for the secondary cursor, this function + /// can be used to find the completion range for a cursor + fn find_completion_range(text: RopeSlice, cursor: usize) -> (usize, usize) { + let start = cursor + - text + .chars_at(cursor) + .reversed() + .take_while(|ch| chars::char_is_word(*ch)) + .count(); + (start, cursor) + } + fn completion_range( + text: RopeSlice, + edit_offset: Option<(i128, i128)>, + cursor: usize, + ) -> Option<(usize, usize)> { + let res = match edit_offset { + Some((start_offset, end_offset)) => { + let start_offset = cursor as i128 + start_offset; + if start_offset < 0 { + return None; + } + let end_offset = cursor as i128 + end_offset; + if end_offset > text.len_chars() as i128 { + return None; + } + (start_offset as usize, end_offset as usize) + } + None => find_completion_range(text, cursor), + }; + Some(res) + } + /// Creates a [Transaction] from the [lsp::TextEdit] in a completion response. /// The transaction applies the edit to all cursors. pub fn generate_transaction_from_completion_edit( doc: &Rope, selection: &Selection, - start_offset: i128, - end_offset: i128, + edit_offset: Option<(i128, i128)>, new_text: String, ) -> Transaction { let replacement: Option = if new_text.is_empty() { @@ -263,83 +296,163 @@ pub mod util { }; let text = doc.slice(..); + let (removed_start, removed_end) = + completion_range(text, edit_offset, selection.primary().cursor(text)) + .expect("transaction must be valid for primary selection"); + let removed_text = text.slice(removed_start..removed_end); - Transaction::change_by_selection(doc, selection, |range| { - let cursor = range.cursor(text); - ( - (cursor as i128 + start_offset) as usize, - (cursor as i128 + end_offset) as usize, - replacement.clone(), - ) - }) + let (transaction, mut selection) = Transaction::change_by_selection_ignore_overlapping( + doc, + selection, + |range| { + let cursor = range.cursor(text); + completion_range(text, edit_offset, cursor) + .filter(|(start, end)| text.slice(start..end) == removed_text) + .unwrap_or_else(|| find_completion_range(text, cursor)) + }, + |_, _| replacement.clone(), + ); + if transaction.changes().is_empty() { + return transaction; + } + selection = selection.map(transaction.changes()); + transaction.with_selection(selection) } /// Creates a [Transaction] from the [snippet::Snippet] in a completion response. /// The transaction applies the edit to all cursors. + #[allow(clippy::too_many_arguments)] pub fn generate_transaction_from_snippet( doc: &Rope, selection: &Selection, - start_offset: i128, - end_offset: i128, + edit_offset: Option<(i128, i128)>, snippet: snippet::Snippet, line_ending: &str, include_placeholder: bool, + tab_width: usize, ) -> Transaction { let text = doc.slice(..); - // For each cursor store offsets for the first tabstop - let mut cursor_tabstop_offsets = Vec::>::new(); - let transaction = Transaction::change_by_selection(doc, selection, |range| { - let cursor = range.cursor(text); - let replacement_start = (cursor as i128 + start_offset) as usize; - let replacement_end = (cursor as i128 + end_offset) as usize; - let newline_with_offset = format!( - "{line_ending}{blank:width$}", - line_ending = line_ending, - width = replacement_start - doc.line_to_char(doc.char_to_line(replacement_start)), - blank = "" - ); - - let (replacement, tabstops) = - snippet::render(&snippet, newline_with_offset, include_placeholder); - - let replacement_len = replacement.chars().count(); - cursor_tabstop_offsets.push( - tabstops - .first() - .unwrap_or(&smallvec![(replacement_len, replacement_len)]) - .iter() - .map(|(from, to)| -> (i128, i128) { - ( - *from as i128 - replacement_len as i128, - *to as i128 - replacement_len as i128, - ) - }) - .collect(), - ); - - (replacement_start, replacement_end, Some(replacement.into())) - }); + let mut off = 0i128; + let mut mapped_doc = doc.clone(); + let mut selection_tabstops: SmallVec<[_; 1]> = SmallVec::new(); + let (removed_start, removed_end) = + completion_range(text, edit_offset, selection.primary().cursor(text)) + .expect("transaction must be valid for primary selection"); + let removed_text = text.slice(removed_start..removed_end); - // Create new selection based on the cursor tabstop from above - let mut cursor_tabstop_offsets_iter = cursor_tabstop_offsets.iter(); - let selection = selection - .clone() - .map(transaction.changes()) - .transform_iter(|range| { - cursor_tabstop_offsets_iter - .next() - .unwrap() - .iter() - .map(move |(from, to)| { - Range::new( - (range.anchor as i128 + *from) as usize, - (range.anchor as i128 + *to) as usize, - ) - }) - }); + let (transaction, selection) = Transaction::change_by_selection_ignore_overlapping( + doc, + selection, + |range| { + let cursor = range.cursor(text); + completion_range(text, edit_offset, cursor) + .filter(|(start, end)| text.slice(start..end) == removed_text) + .unwrap_or_else(|| find_completion_range(text, cursor)) + }, + |replacement_start, replacement_end| { + let mapped_replacement_start = (replacement_start as i128 + off) as usize; + let mapped_replacement_end = (replacement_end as i128 + off) as usize; + + let line_idx = mapped_doc.char_to_line(mapped_replacement_start); + let pos_on_line = mapped_replacement_start - mapped_doc.line_to_char(line_idx); + + // we only care about the actual offset here (not virtual text/softwrap) + // so it's ok to use the deprecated function here + #[allow(deprecated)] + let width = helix_core::visual_coords_at_pos( + mapped_doc.line(line_idx), + pos_on_line, + tab_width, + ) + .col; + let newline_with_offset = format!( + "{line_ending}{blank:width$}", + line_ending = line_ending, + blank = "" + ); + + let (replacement, tabstops) = + snippet::render(&snippet, &newline_with_offset, include_placeholder); + selection_tabstops.push((mapped_replacement_start, tabstops)); + mapped_doc.remove(mapped_replacement_start..mapped_replacement_end); + mapped_doc.insert(mapped_replacement_start, &replacement); + off += + replacement_start as i128 - replacement_end as i128 + replacement.len() as i128; + + Some(replacement) + }, + ); - transaction.with_selection(selection) + let changes = transaction.changes(); + if changes.is_empty() { + return transaction; + } + + let mut mapped_selection = SmallVec::with_capacity(selection.len()); + let mut mapped_primary_idx = 0; + let primary_range = selection.primary(); + for (range, (tabstop_anchor, tabstops)) in selection.into_iter().zip(selection_tabstops) { + if range == primary_range { + mapped_primary_idx = mapped_selection.len() + } + + let range = range.map(changes); + let tabstops = tabstops.first().filter(|tabstops| !tabstops.is_empty()); + let Some(tabstops) = tabstops else{ + // no tabstop normal mapping + mapped_selection.push(range); + continue; + }; + + // expand the selection to cover the tabstop to retain the helix selection semantic + // the tabstop closest to the range simply replaces `head` while anchor remains in place + // the remaining tabstops receive their own single-width cursor + if range.head < range.anchor { + let first_tabstop = tabstop_anchor + tabstops[0].1; + + // if selection is forward but was moved to the right it is + // contained entirely in the replacement text, just do a point + // selection (fallback below) + if range.anchor >= first_tabstop { + let range = Range::new(range.anchor, first_tabstop); + mapped_selection.push(range); + let rem_tabstops = tabstops[1..] + .iter() + .map(|tabstop| Range::point(tabstop_anchor + tabstop.1)); + mapped_selection.extend(rem_tabstops); + continue; + } + } else { + let last_idx = tabstops.len() - 1; + let last_tabstop = tabstop_anchor + tabstops[last_idx].1; + + // if selection is forward but was moved to the right it is + // contained entirely in the replacement text, just do a point + // selection (fallback below) + if range.anchor <= last_tabstop { + // we can't properly compute the the next grapheme + // here because the transaction hasn't been applied yet + // that is not a problem because the range gets grapheme aligned anyway + // tough so just adding one will always cause head to be grapheme + // aligned correctly when applied to the document + let range = Range::new(range.anchor, last_tabstop + 1); + mapped_selection.push(range); + let rem_tabstops = tabstops[..last_idx] + .iter() + .map(|tabstop| Range::point(tabstop_anchor + tabstop.0)); + mapped_selection.extend(rem_tabstops); + continue; + } + }; + + let tabstops = tabstops + .iter() + .map(|tabstop| Range::point(tabstop_anchor + tabstop.0)); + mapped_selection.extend(tabstops); + } + + transaction.with_selection(Selection::new(mapped_selection, mapped_primary_idx)) } pub fn generate_transaction_from_edits( diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 336b75cb46a8..99c337811741 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -118,7 +118,6 @@ impl Completion { view_id: ViewId, item: &CompletionItem, offset_encoding: helix_lsp::OffsetEncoding, - start_offset: usize, trigger_offset: usize, include_placeholder: bool, ) -> Transaction { @@ -147,28 +146,18 @@ impl Completion { None => return Transaction::new(doc.text()), }; - (start_offset, end_offset, edit.new_text) + (Some((start_offset, end_offset)), edit.new_text) } else { let new_text = item .insert_text .clone() .unwrap_or_else(|| item.label.clone()); - // check that we are still at the correct savepoint // we can still generate a transaction regardless but if the // document changed (and not just the selection) then we will // likely delete the wrong text (same if we applied an edit sent by the LS) debug_assert!(primary_cursor == trigger_offset); - - // TODO: Respect editor.completion_replace? - // Would require detecting the end of the word boundary for every cursor individually. - // We don't do the same for normal `edits, to be consistent we would have to do it for those too - - ( - start_offset as i128 - primary_cursor as i128, - trigger_offset as i128 - primary_cursor as i128, - new_text, - ) + (None, Some(0), new_text) }; if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET)) @@ -186,6 +175,7 @@ impl Completion { snippet, doc.line_ending.as_str(), include_placeholder, + doc.tab_width(), ), Err(err) => { log::error!( @@ -232,7 +222,6 @@ impl Completion { view.id, item, offset_encoding, - start_offset, trigger_offset, true, ); @@ -254,7 +243,6 @@ impl Completion { view.id, item, offset_encoding, - start_offset, trigger_offset, false, ); From 3b88d7405aa4aaca25a86f2f640de965805a0e7f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 9 Mar 2023 23:21:02 +0100 Subject: [PATCH 6/6] treat replace/insertmode consistently, default to insert --- book/src/configuration.md | 1 + helix-lsp/src/lib.rs | 45 +++++++++++++++++++++++---------- helix-term/src/ui/completion.rs | 24 ++++++++++++------ helix-view/src/editor.rs | 4 +++ 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/book/src/configuration.md b/book/src/configuration.md index aebf5ff0f9be..4b62ca521e6b 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -50,6 +50,7 @@ signal to the Helix process on Unix operating systems, such as by using the comm | `auto-save` | Enable automatic saving on the focus moving away from Helix. Requires [focus event support](https://github.com/helix-editor/helix/wiki/Terminal-Support) from your terminal | `false` | | `idle-timeout` | Time in milliseconds since last keypress before idle timers trigger. Used for autocompletion, set to 0 for instant | `400` | | `completion-trigger-len` | The min-length of word under cursor to trigger autocompletion | `2` | +| `completion-replace` | Set to `true` to make completions always replace the entire word and not just the part before the cursor | `false` | | `auto-info` | Whether to display info boxes | `true` | | `true-color` | Set to `true` to override automatic detection of terminal truecolor support in the event of a false negative | `false` | | `rulers` | List of column positions at which to display the rulers. Can be overridden by language specific `rulers` in `languages.toml` file | `[]` | diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 58e8d83dc386..1463ccb3ccf8 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -250,18 +250,27 @@ pub mod util { /// If the LS did not provide a range for the completion or the range of the /// primary cursor can not be used for the secondary cursor, this function /// can be used to find the completion range for a cursor - fn find_completion_range(text: RopeSlice, cursor: usize) -> (usize, usize) { + fn find_completion_range(text: RopeSlice, replace_mode: bool, cursor: usize) -> (usize, usize) { let start = cursor - text .chars_at(cursor) .reversed() .take_while(|ch| chars::char_is_word(*ch)) .count(); - (start, cursor) + let mut end = cursor; + if replace_mode { + end += text + .chars_at(cursor) + .skip(1) + .take_while(|ch| chars::char_is_word(*ch)) + .count(); + } + (start, end) } fn completion_range( text: RopeSlice, edit_offset: Option<(i128, i128)>, + replace_mode: bool, cursor: usize, ) -> Option<(usize, usize)> { let res = match edit_offset { @@ -276,7 +285,7 @@ pub mod util { } (start_offset as usize, end_offset as usize) } - None => find_completion_range(text, cursor), + None => find_completion_range(text, replace_mode, cursor), }; Some(res) } @@ -287,6 +296,7 @@ pub mod util { doc: &Rope, selection: &Selection, edit_offset: Option<(i128, i128)>, + replace_mode: bool, new_text: String, ) -> Transaction { let replacement: Option = if new_text.is_empty() { @@ -296,9 +306,13 @@ pub mod util { }; let text = doc.slice(..); - let (removed_start, removed_end) = - completion_range(text, edit_offset, selection.primary().cursor(text)) - .expect("transaction must be valid for primary selection"); + let (removed_start, removed_end) = completion_range( + text, + edit_offset, + replace_mode, + selection.primary().cursor(text), + ) + .expect("transaction must be valid for primary selection"); let removed_text = text.slice(removed_start..removed_end); let (transaction, mut selection) = Transaction::change_by_selection_ignore_overlapping( @@ -306,9 +320,9 @@ pub mod util { selection, |range| { let cursor = range.cursor(text); - completion_range(text, edit_offset, cursor) + completion_range(text, edit_offset, replace_mode, cursor) .filter(|(start, end)| text.slice(start..end) == removed_text) - .unwrap_or_else(|| find_completion_range(text, cursor)) + .unwrap_or_else(|| find_completion_range(text, replace_mode, cursor)) }, |_, _| replacement.clone(), ); @@ -326,6 +340,7 @@ pub mod util { doc: &Rope, selection: &Selection, edit_offset: Option<(i128, i128)>, + replace_mode: bool, snippet: snippet::Snippet, line_ending: &str, include_placeholder: bool, @@ -336,9 +351,13 @@ pub mod util { let mut off = 0i128; let mut mapped_doc = doc.clone(); let mut selection_tabstops: SmallVec<[_; 1]> = SmallVec::new(); - let (removed_start, removed_end) = - completion_range(text, edit_offset, selection.primary().cursor(text)) - .expect("transaction must be valid for primary selection"); + let (removed_start, removed_end) = completion_range( + text, + edit_offset, + replace_mode, + selection.primary().cursor(text), + ) + .expect("transaction must be valid for primary selection"); let removed_text = text.slice(removed_start..removed_end); let (transaction, selection) = Transaction::change_by_selection_ignore_overlapping( @@ -346,9 +365,9 @@ pub mod util { selection, |range| { let cursor = range.cursor(text); - completion_range(text, edit_offset, cursor) + completion_range(text, edit_offset, replace_mode, cursor) .filter(|(start, end)| text.slice(start..end) == removed_text) - .unwrap_or_else(|| find_completion_range(text, cursor)) + .unwrap_or_else(|| find_completion_range(text, replace_mode, cursor)) }, |replacement_start, replacement_end| { let mapped_replacement_start = (replacement_start as i128 + off) as usize; diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 99c337811741..6303793b4a11 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -108,6 +108,7 @@ impl Completion { start_offset: usize, trigger_offset: usize, ) -> Self { + let replace_mode = editor.config().completion_replace; // Sort completion items according to their preselect status (given by the LSP server) items.sort_by_key(|item| !item.preselect.unwrap_or(false)); @@ -120,18 +121,23 @@ impl Completion { offset_encoding: helix_lsp::OffsetEncoding, trigger_offset: usize, include_placeholder: bool, + replace_mode: bool, ) -> Transaction { use helix_lsp::snippet; let selection = doc.selection(view_id); let text = doc.text().slice(..); let primary_cursor = selection.primary().cursor(text); - let (start_offset, end_offset, new_text) = if let Some(edit) = &item.text_edit { + let (edit_offset, new_text) = if let Some(edit) = &item.text_edit { let edit = match edit { lsp::CompletionTextEdit::Edit(edit) => edit.clone(), lsp::CompletionTextEdit::InsertAndReplace(item) => { - // TODO: support using "insert" instead of "replace" via user config - lsp::TextEdit::new(item.replace, item.new_text.clone()) + let range = if replace_mode { + item.replace + } else { + item.insert + }; + lsp::TextEdit::new(range, item.new_text.clone()) } }; @@ -157,7 +163,7 @@ impl Completion { // document changed (and not just the selection) then we will // likely delete the wrong text (same if we applied an edit sent by the LS) debug_assert!(primary_cursor == trigger_offset); - (None, Some(0), new_text) + (None, new_text) }; if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET)) @@ -170,8 +176,8 @@ impl Completion { Ok(snippet) => util::generate_transaction_from_snippet( doc.text(), selection, - start_offset, - end_offset, + edit_offset, + replace_mode, snippet, doc.line_ending.as_str(), include_placeholder, @@ -190,8 +196,8 @@ impl Completion { util::generate_transaction_from_completion_edit( doc.text(), selection, - start_offset, - end_offset, + edit_offset, + replace_mode, new_text, ) } @@ -224,6 +230,7 @@ impl Completion { offset_encoding, trigger_offset, true, + replace_mode, ); // initialize a savepoint @@ -245,6 +252,7 @@ impl Completion { offset_encoding, trigger_offset, false, + replace_mode, ); doc.apply(&transaction, view.id); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c6541105a2e5..1b4664ffb39f 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -251,6 +251,9 @@ pub struct Config { )] pub idle_timeout: Duration, pub completion_trigger_len: u8, + /// Whether to instruct the LSP to replace the entire word when applying a completion + /// or to only insert new text + pub completion_replace: bool, /// Whether to display infoboxes. Defaults to true. pub auto_info: bool, pub file_picker: FilePickerConfig, @@ -738,6 +741,7 @@ impl Default for Config { color_modes: false, soft_wrap: SoftWrap::default(), text_width: 80, + completion_replace: false, } } }