From 8fccfa98318220c86c1dee19454c6d0a18c15eac Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Mon, 30 Jan 2023 03:13:42 +0100 Subject: [PATCH] make replace completions optional, fix issues with multicursors --- book/src/configuration.md | 1 + helix-core/src/transaction.rs | 85 ++++++++++++++ helix-lsp/src/lib.rs | 201 ++++++++++++++++++++++---------- helix-term/src/ui/completion.rs | 29 ++--- helix-view/src/editor.rs | 4 + 5 files changed, 240 insertions(+), 80 deletions(-) diff --git a/book/src/configuration.md b/book/src/configuration.md index aebf5ff0f9beb..4b62ca521e6b7 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-core/src/transaction.rs b/helix-core/src/transaction.rs index d2f4de07dbe74..729b45ac13871 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,56 @@ impl Transaction { self } + /// Generate a transaction from a set of potentially overallping changes. + /// If a change overlaps a previous changes it's ignored + /// + /// The `process_change` callback is called for each change (in the order + /// yielded by `changes`). This callback can be used to process additional + /// information (usually selections) associated with each change. This + /// function passes `None` to the `process_changes` function if the + /// `changes` iterator yields `None` **or if the change overlaps with a + /// previous change** and is ignored. + pub fn change_ignore_overlapping( + doc: &Rope, + changes: I, + mut process_change: impl FnMut(Option), + ) -> Self + where + I: Iterator>, + { + let len = doc.len_chars(); + + let (lower, upper) = changes.size_hint(); + let size = upper.unwrap_or(lower); + let mut changeset = ChangeSet::with_capacity(2 * size + 1); // rough estimate + + let mut last = 0; + for mut change in changes { + change = change.filter(|&((from, _, _), _)| last <= from); + let Some(((from, to, tendril), data)) = change else { + process_change(None); + continue; + }; + process_change(Some(data)); + + // Retain from last "to" to current "from" + changeset.retain(from - last); + let span = to - from; + match tendril { + Some(text) => { + changeset.insert(text); + changeset.delete(span); + } + None => changeset.delete(span), + } + last = to; + } + + changeset.retain(len - last); + + Self::from(changeset) + } + /// Generate a transaction from a set of changes. pub fn change(doc: &Rope, changes: I) -> Self where @@ -513,6 +565,39 @@ impl Transaction { Self::change(doc, selection.iter().map(f)) } + pub fn change_by_selection_ignore_overlapping( + doc: &Rope, + selection: &Selection, + mut create_change: impl FnMut(&Range) -> Option<(Change, T)>, + mut process_applied_change: impl FnMut(&Range, T), + ) -> (Transaction, usize) { + let mut last_selection_idx = None; + let mut new_primary_idx = 0; + let mut idx = 0; + let mut applied_changes = 0; + let process_change = |change: Option<_>| { + if let Some((range, data)) = change { + last_selection_idx = Some(applied_changes); + process_applied_change(range, data); + applied_changes += 1; + } + if idx == selection.primary_index() { + new_primary_idx = last_selection_idx.unwrap_or(0); + } + idx += 1; + }; + let transaction = Self::change_ignore_overlapping( + doc, + selection.iter().map(|range| { + let (change, data) = create_change(range)?; + Some((change, (range, data))) + }), + process_change, + ); + // map the transaction trough changes + ((transaction, new_primary_idx)) + } + /// Insert text at each selection head. pub fn insert(doc: &Rope, selection: &Selection, text: Tendril) -> Self { Self::change_by_selection(doc, selection, |range| { diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 147b381c2a4e1..1e46251920b4e 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, Assoc, 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,56 @@ pub mod util { Some(Range::new(start, end)) } + fn completion_pos( + text: RopeSlice, + start_offset: Option, + end_offset: Option, + cursor: usize, + ) -> Option<(usize, usize)> { + let replacement_start = match start_offset { + Some(start_offset) => { + let start_offset = cursor as i128 + start_offset; + if start_offset < 0 { + return None; + } + start_offset as usize + } + None => { + cursor + + text + .chars_at(cursor) + .skip(1) + .take_while(|ch| chars::char_is_word(*ch)) + .count() + } + }; + let replacement_end = match end_offset { + Some(end_offset) => { + let replacement_end = cursor as i128 + end_offset; + if replacement_end > text.len_chars() as i128 { + return None; + } + replacement_end as usize + } + None => { + cursor + - text + .chars_at(cursor) + .reversed() + .take_while(|ch| chars::char_is_word(*ch)) + .count() + } + }; + Some((replacement_start, replacement_end)) + } + /// 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, + start_offset: Option, + end_offset: Option, new_text: String, ) -> Transaction { let replacement: Option = if new_text.is_empty() { @@ -263,15 +306,23 @@ pub mod util { }; let text = doc.slice(..); + let mut new_selection = SmallVec::new(); - 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, primary_idx) = Transaction::change_by_selection_ignore_overlapping( + doc, + selection, + |range| { + let cursor = range.cursor(text); + let (start, end) = completion_pos(text, start_offset, end_offset, cursor)?; + Some(((start as usize, end, replacement.clone()), ())) + }, + |range, _| new_selection.push(*range), + ); + if transaction.changes().is_empty() { + return transaction; + } + let selection = Selection::new(new_selection, primary_idx).map(transaction.changes()); + transaction.with_selection(selection) } /// Creates a [Transaction] from the [snippet::Snippet] in a completion response. @@ -279,67 +330,91 @@ pub mod util { pub fn generate_transaction_from_snippet( doc: &Rope, selection: &Selection, - start_offset: i128, - end_offset: i128, + start_offset: Option, + end_offset: Option, snippet: snippet::Snippet, line_ending: &str, include_placeholder: bool, ) -> Transaction { let text = doc.slice(..); + let mut new_selection: SmallVec<[_; 1]> = SmallVec::new(); - // 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 (transaction, primary_idx) = Transaction::change_by_selection_ignore_overlapping( + doc, + selection, + |range| { + let cursor = range.cursor(text); + let (replacement_start, replacement_end) = + completion_pos(text, start_offset, end_offset, cursor)?; + 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); + + Some(( + (replacement_start, replacement_end, Some(replacement.into())), + (replacement_start, tabstops), + )) + }, + |range, tabstops| new_selection.push((*range, tabstops)), + ); + + let changes = transaction.changes(); + if changes.is_empty() { + return transaction; + } + + let mut mapped_selection = SmallVec::with_capacity(new_selection.len()); + let mut mapped_primary_idx = 0; + for (i, (range, (tabstop_anchor, tabstops))) in new_selection.into_iter().enumerate() { + if i == primary_idx { + mapped_primary_idx = mapped_selection.len() + } - // 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() + 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 remainaing tabstops recive their own single-width cursor + if range.head < range.anchor { + let primary_tabstop = tabstop_anchor + tabstops[0].1; + debug_assert!(primary_tabstop <= range.anchor); + let range = Range::new(range.anchor, primary_tabstop); + mapped_selection.push(range); + let rem_tabstops = tabstops[1..] .iter() - .map(move |(from, to)| { - Range::new( - (range.anchor as i128 + *from) as usize, - (range.anchor as i128 + *to) as usize, - ) - }) - }); + .map(|tabstop| Range::point(tabstop_anchor + tabstop.1)); + mapped_selection.extend(rem_tabstops); + } else { + let last_idx = tabstops.len() - 1; + let primary_tabstop = tabstop_anchor + tabstops[last_idx].1; + debug_assert!(primary_tabstop >= range.anchor); + // we can't properly compute the the next grapheme + // here because the transaction hasn't been applied yet + // that is not a probleme 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, primary_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); + }; + } - transaction.with_selection(selection) + 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 c8c14a6567983..ade951f2c41e7 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -106,6 +106,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)); @@ -116,9 +117,9 @@ impl Completion { view_id: ViewId, item: &CompletionItem, offset_encoding: helix_lsp::OffsetEncoding, - start_offset: usize, trigger_offset: usize, include_placeholder: bool, + replace_mode: bool, ) -> Transaction { use helix_lsp::snippet; let selection = doc.selection(view_id); @@ -129,8 +130,12 @@ impl Completion { 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()) } }; @@ -145,28 +150,18 @@ impl Completion { None => return Transaction::new(doc.text()), }; - (start_offset, end_offset, edit.new_text) + (Some(start_offset), Some(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, None, new_text) }; if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET)) @@ -231,9 +226,9 @@ impl Completion { view.id, item, offset_encoding, - start_offset, trigger_offset, true, + replace_mode, ); // initialize a savepoint @@ -254,9 +249,9 @@ impl Completion { view.id, item, offset_encoding, - start_offset, 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 5b819b33354e0..097c0551cafb1 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, } } }