diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ee7bc79ddf447..80f8fb8118b9c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -33,7 +33,7 @@ use helix_core::{ use helix_view::{ clipboard::ClipboardType, document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, - editor::{Action, Motion}, + editor::{Action, CompleteAction, Motion}, info::Info, input::KeyEvent, keyboard::KeyCode, @@ -4178,16 +4178,23 @@ pub fn completion(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let savepoint = if let Some(CompleteAction::Selected { savepoint }) = &cx.editor.last_completion + { + savepoint.clone() + } else { + doc.savepoint(view) + }; + let language_server = match doc.language_server() { Some(language_server) => language_server, None => return, }; let offset_encoding = language_server.offset_encoding(); - let text = doc.text().slice(..); - let cursor = doc.selection(view.id).primary().cursor(text); + let text = savepoint.text.clone(); + let cursor = savepoint.cursor(); - let pos = pos_to_lsp_pos(doc.text(), cursor, offset_encoding); + let pos = pos_to_lsp_pos(&text, cursor, offset_encoding); let future = match language_server.completion(doc.identifier(), pos, None) { Some(future) => future, @@ -4222,7 +4229,6 @@ pub fn completion(cx: &mut Context) { iter.reverse(); let offset = iter.take_while(|ch| chars::char_is_word(*ch)).count(); let start_offset = cursor.saturating_sub(offset); - let savepoint = doc.savepoint(view); let trigger_doc = doc.id(); let trigger_view = view.id; diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index bc216509f3ca6..c5c405801f288 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -16,7 +16,6 @@ use crate::commands; use crate::ui::{menu, Markdown, Menu, Popup, PromptEvent}; use helix_lsp::{lsp, util}; -use lsp::CompletionItem; impl menu::Item for CompletionItem { type Data = (); @@ -26,28 +25,29 @@ impl menu::Item for CompletionItem { #[inline] fn filter_text(&self, _data: &Self::Data) -> Cow { - self.filter_text + self.item + .filter_text .as_ref() - .unwrap_or(&self.label) + .unwrap_or(&self.item.label) .as_str() .into() } fn format(&self, _data: &Self::Data) -> menu::Row { - let deprecated = self.deprecated.unwrap_or_default() - || self.tags.as_ref().map_or(false, |tags| { + let deprecated = self.item.deprecated.unwrap_or_default() + || self.item.tags.as_ref().map_or(false, |tags| { tags.contains(&lsp::CompletionItemTag::DEPRECATED) }); menu::Row::new(vec![ menu::Cell::from(Span::styled( - self.label.as_str(), + self.item.label.as_str(), if deprecated { Style::default().add_modifier(Modifier::CROSSED_OUT) } else { Style::default() }, )), - menu::Cell::from(match self.kind { + menu::Cell::from(match self.item.kind { Some(lsp::CompletionItemKind::TEXT) => "text", Some(lsp::CompletionItemKind::METHOD) => "method", Some(lsp::CompletionItemKind::FUNCTION) => "function", @@ -88,6 +88,12 @@ impl menu::Item for CompletionItem { } } +#[derive(Debug, PartialEq, Default, Clone)] +struct CompletionItem { + item: lsp::CompletionItem, + resolved: bool, +} + /// Wraps a Menu. pub struct Completion { popup: Popup>, @@ -103,7 +109,7 @@ impl Completion { pub fn new( editor: &Editor, savepoint: Arc, - mut items: Vec, + mut items: Vec, offset_encoding: helix_lsp::OffsetEncoding, start_offset: usize, trigger_offset: usize, @@ -111,6 +117,13 @@ impl Completion { 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)); + let items = items + .into_iter() + .map(|item| CompletionItem { + item, + resolved: false, + }) + .collect(); // Then create the menu let menu = Menu::new(items, (), move |editor: &mut Editor, item, event| { @@ -128,7 +141,7 @@ impl Completion { let text = doc.text().slice(..); let primary_cursor = selection.primary().cursor(text); - let (edit_offset, new_text) = if let Some(edit) = &item.text_edit { + let (edit_offset, new_text) = if let Some(edit) = &item.item.text_edit { let edit = match edit { lsp::CompletionTextEdit::Edit(edit) => edit.clone(), lsp::CompletionTextEdit::InsertAndReplace(item) => { @@ -151,9 +164,10 @@ impl Completion { (Some((start_offset, end_offset)), edit.new_text) } else { let new_text = item + .item .insert_text .clone() - .unwrap_or_else(|| item.label.clone()); + .unwrap_or_else(|| item.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 @@ -162,9 +176,9 @@ impl Completion { (None, new_text) }; - if matches!(item.kind, Some(lsp::CompletionItemKind::SNIPPET)) + if matches!(item.item.kind, Some(lsp::CompletionItemKind::SNIPPET)) || matches!( - item.insert_text_format, + item.item.insert_text_format, Some(lsp::InsertTextFormat::SNIPPET) ) { @@ -209,14 +223,27 @@ impl Completion { let (view, doc) = current!(editor); - // if more text was entered, remove it - doc.restore(view, &savepoint); - match event { - PromptEvent::Abort => { - editor.last_completion = None; - } + PromptEvent::Abort => {} PromptEvent::Update => { + // Update creates "ghost" transactions which are not sent to the + // lsp server to avoid messing up re-requesting completions. Once a + // completion has been selected (with tab, c-n or c-p) it's always accepted whenever anything + // is typed. The only way to avoid that is to explicitly abort the completion + // with c-c. This will remove the "ghost" transaction. + // + // The ghost transaction is modeled with a transaction that is not sent to the LS. + // (apply_temporary) and a savepoint. It's extremely important this savepoint is restored + // (also without sending the transaction to the LS) *before any further transaction is applied*. + // Otherwise incremental sync breaks (since the state of the LS doesn't match the state the transaction + // is applied to). + if editor.last_completion.is_none() { + editor.last_completion = Some(CompleteAction::Selected { + savepoint: doc.savepoint(view), + }) + } + // if more text was entered, remove it + doc.restore(view, &savepoint, false); // always present here let item = item.unwrap(); @@ -229,57 +256,49 @@ impl Completion { true, replace_mode, ); - - // initialize a savepoint - doc.apply(&transaction, view.id); - - editor.last_completion = Some(CompleteAction { - trigger_offset, - changes: completion_changes(&transaction, trigger_offset), - }); + doc.apply_temporary(&transaction, view.id); } PromptEvent::Validate => { + if let Some(CompleteAction::Selected { savepoint }) = + editor.last_completion.take() + { + doc.restore(view, &savepoint, false); + } // always present here - let item = item.unwrap(); - + let mut item = item.unwrap().clone(); + + // resolve item if not yet resolved + if !item.resolved { + if let Some(resolved) = + Self::resolve_completion_item(doc, item.item.clone()) + { + item.item = resolved; + } + }; + // if more text was entered, remove it + doc.restore(view, &savepoint, true); let transaction = item_to_transaction( doc, view.id, - item, + &item, offset_encoding, trigger_offset, false, replace_mode, ); - doc.apply(&transaction, view.id); - editor.last_completion = Some(CompleteAction { + editor.last_completion = Some(CompleteAction::Applied { trigger_offset, changes: completion_changes(&transaction, trigger_offset), }); - // apply additional edits, mostly used to auto import unqualified types - let resolved_item = if item - .additional_text_edits - .as_ref() - .map(|edits| !edits.is_empty()) - .unwrap_or(false) - { - None - } else { - Self::resolve_completion_item(doc, item.clone()) - }; - - if let Some(additional_edits) = resolved_item - .as_ref() - .and_then(|item| item.additional_text_edits.as_ref()) - .or(item.additional_text_edits.as_ref()) - { + // TODO: add additional _edits to completion_changes? + if let Some(additional_edits) = item.item.additional_text_edits { if !additional_edits.is_empty() { let transaction = util::generate_transaction_from_edits( doc.text(), - additional_edits.clone(), + additional_edits, offset_encoding, // TODO: should probably transcode in Client ); doc.apply(&transaction, view.id); @@ -306,7 +325,7 @@ impl Completion { fn resolve_completion_item( doc: &Document, completion_item: lsp::CompletionItem, - ) -> Option { + ) -> Option { let language_server = doc.language_server()?; let future = language_server.resolve_completion_item(completion_item)?; @@ -359,7 +378,7 @@ impl Completion { self.popup.contents().is_empty() } - fn replace_item(&mut self, old_item: lsp::CompletionItem, new_item: lsp::CompletionItem) { + fn replace_item(&mut self, old_item: CompletionItem, new_item: CompletionItem) { self.popup.contents_mut().replace_option(old_item, new_item); } @@ -375,7 +394,7 @@ impl Completion { // > The returned completion item should have the documentation property filled in. // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion let current_item = match self.popup.contents().selection() { - Some(item) if item.documentation.is_none() => item.clone(), + Some(item) if !item.resolved => item.clone(), _ => return false, }; @@ -385,7 +404,7 @@ impl Completion { }; // This method should not block the compositor so we handle the response asynchronously. - let future = match language_server.resolve_completion_item(current_item.clone()) { + let future = match language_server.resolve_completion_item(current_item.item.clone()) { Some(future) => future, None => return false, }; @@ -403,7 +422,13 @@ impl Completion { .unwrap() .completion { - completion.replace_item(current_item, resolved_item); + completion.replace_item( + current_item, + CompletionItem { + item: resolved_item, + resolved: true, + }, + ); } }, ); @@ -457,25 +482,25 @@ impl Component for Completion { Markdown::new(md, cx.editor.syn_loader.clone()) }; - let mut markdown_doc = match &option.documentation { + let mut markdown_doc = match &option.item.documentation { Some(lsp::Documentation::String(contents)) | Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { kind: lsp::MarkupKind::PlainText, value: contents, })) => { // TODO: convert to wrapped text - markdowned(language, option.detail.as_deref(), Some(contents)) + markdowned(language, option.item.detail.as_deref(), Some(contents)) } Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { kind: lsp::MarkupKind::Markdown, value: contents, })) => { // TODO: set language based on doc scope - markdowned(language, option.detail.as_deref(), Some(contents)) + markdowned(language, option.item.detail.as_deref(), Some(contents)) } - None if option.detail.is_some() => { + None if option.item.detail.is_some() => { // TODO: set language based on doc scope - markdowned(language, option.detail.as_deref(), None) + markdowned(language, option.item.detail.as_deref(), None) } None => return, }; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 3494da07d67f0..86d88b9125f78 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -20,7 +20,7 @@ use helix_core::{ syntax::{self, HighlightEvent}, text_annotations::TextAnnotations, unicode::width::UnicodeWidthStr, - visual_offset_from_block, Position, Range, Selection, Transaction, + visual_offset_from_block, Change, Position, Range, Selection, Transaction, }; use helix_view::{ document::{Mode, SavePoint, SCRATCH_BUFFER_NAME}, @@ -49,7 +49,10 @@ pub struct EditorView { #[derive(Debug, Clone)] pub enum InsertEvent { Key(KeyEvent), - CompletionApply(CompleteAction), + CompletionApply { + trigger_offset: usize, + changes: Vec, + }, TriggerCompletion, RequestCompletion, } @@ -813,7 +816,7 @@ impl EditorView { } (Mode::Insert, Mode::Normal) => { // if exiting insert mode, remove completion - self.completion = None; + self.clear_completion(cxt.editor); cxt.editor.completion_request_handle = None; // TODO: Use an on_mode_change hook to remove signature help @@ -891,22 +894,25 @@ impl EditorView { for key in self.last_insert.1.clone() { match key { InsertEvent::Key(key) => self.insert_mode(cxt, key), - InsertEvent::CompletionApply(compl) => { + InsertEvent::CompletionApply { + trigger_offset, + changes, + } => { let (view, doc) = current!(cxt.editor); if let Some(last_savepoint) = last_savepoint.as_deref() { - doc.restore(view, last_savepoint); + doc.restore(view, last_savepoint, true); } let text = doc.text().slice(..); let cursor = doc.selection(view.id).primary().cursor(text); let shift_position = - |pos: usize| -> usize { pos + cursor - compl.trigger_offset }; + |pos: usize| -> usize { pos + cursor - trigger_offset }; let tx = Transaction::change( doc.text(), - compl.changes.iter().cloned().map(|(start, end, t)| { + changes.iter().cloned().map(|(start, end, t)| { (shift_position(start), shift_position(end), t) }), ); @@ -979,6 +985,21 @@ impl EditorView { pub fn clear_completion(&mut self, editor: &mut Editor) { self.completion = None; + if let Some(last_completion) = editor.last_completion.take() { + match last_completion { + CompleteAction::Applied { + trigger_offset, + changes, + } => self.last_insert.1.push(InsertEvent::CompletionApply { + trigger_offset, + changes, + }), + CompleteAction::Selected { savepoint } => { + let (view, doc) = current!(editor); + doc.restore(view, &savepoint, false); + } + } + } // Clear any savepoints editor.clear_idle_timer(); // don't retrigger @@ -1265,12 +1286,22 @@ impl Component for EditorView { jobs: cx.jobs, scroll: None, }; - completion.handle_event(event, &mut cx) - }; - if let EventResult::Consumed(callback) = res { - consumed = true; + if let EventResult::Consumed(callback) = + completion.handle_event(event, &mut cx) + { + consumed = true; + Some(callback) + } else if let EventResult::Consumed(callback) = + completion.handle_event(&Event::Key(key!(Enter)), &mut cx) + { + Some(callback) + } else { + None + } + }; + if let Some(callback) = res { if callback.is_some() { // assume close_fn self.clear_completion(cx.editor); @@ -1286,10 +1317,6 @@ impl Component for EditorView { // if completion didn't take the event, we pass it onto commands if !consumed { - if let Some(compl) = cx.editor.last_completion.take() { - self.last_insert.1.push(InsertEvent::CompletionApply(compl)); - } - self.insert_mode(&mut cx, key); // record last_insert key diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 4afb79d4bdca2..e717cf72f33d0 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -112,6 +112,19 @@ pub struct SavePoint { /// The view this savepoint is associated with pub view: ViewId, revert: Mutex, + pub text: Rope, +} + +impl SavePoint { + pub fn cursor(&self) -> usize { + // we always create transactions with selections + self.revert + .lock() + .selection() + .unwrap() + .primary() + .cursor(self.text.slice(..)) + } } pub struct Document { @@ -914,7 +927,12 @@ impl Document { } /// Apply a [`Transaction`] to the [`Document`] to change its text. - fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { + fn apply_impl( + &mut self, + transaction: &Transaction, + view_id: ViewId, + emit_lsp_notification: bool, + ) -> bool { use helix_core::Assoc; let old_doc = self.text().clone(); @@ -1010,25 +1028,31 @@ impl Document { apply_inlay_hint_changes(padding_after_inlay_hints); } - // emit lsp notification - if let Some(language_server) = self.language_server() { - let notify = language_server.text_document_did_change( - self.versioned_identifier(), - &old_doc, - self.text(), - changes, - ); + if emit_lsp_notification { + // emit lsp notification + if let Some(language_server) = self.language_server() { + let notify = language_server.text_document_did_change( + self.versioned_identifier(), + &old_doc, + self.text(), + changes, + ); - if let Some(notify) = notify { - tokio::spawn(notify); + if let Some(notify) = notify { + tokio::spawn(notify); + } } } } success } - /// Apply a [`Transaction`] to the [`Document`] to change its text. - pub fn apply(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { + fn apply_inner( + &mut self, + transaction: &Transaction, + view_id: ViewId, + emit_lsp_notification: bool, + ) -> bool { // store the state just before any changes are made. This allows us to undo to the // state just before a transaction was applied. if self.changes.is_empty() && !transaction.changes().is_empty() { @@ -1038,7 +1062,7 @@ impl Document { }); } - let success = self.apply_impl(transaction, view_id); + let success = self.apply_impl(transaction, view_id, emit_lsp_notification); if !transaction.changes().is_empty() { // Compose this transaction with the previous one @@ -1048,12 +1072,23 @@ impl Document { } success } + /// Apply a [`Transaction`] to the [`Document`] to change its text. + pub fn apply(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { + self.apply_inner(transaction, view_id, true) + } + + /// Apply a [`Transaction`] to the [`Document`] to change its text + /// without notifying the language servers. This is useful for temporary transactions + /// that must not influence the server. + pub fn apply_temporary(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { + self.apply_inner(transaction, view_id, false) + } fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool { let mut history = self.history.take(); let txn = if undo { history.undo() } else { history.redo() }; let success = if let Some(txn) = txn { - self.apply_impl(txn, view.id) + self.apply_impl(txn, view.id, true) } else { false }; @@ -1085,15 +1120,32 @@ impl Document { /// the state it had when this function was called. pub fn savepoint(&mut self, view: &View) -> Arc { let revert = Transaction::new(self.text()).with_selection(self.selection(view.id).clone()); + // check if there is already an existing (identical) savepoint around + if let Some(savepoint) = self + .savepoints + .iter() + .rev() + .find_map(|savepoint| savepoint.upgrade()) + { + let transaction = savepoint.revert.lock(); + if savepoint.view == view.id + && transaction.changes().is_empty() + && transaction.selection() == revert.selection() + { + drop(transaction); + return savepoint; + } + } let savepoint = Arc::new(SavePoint { view: view.id, revert: Mutex::new(revert), + text: self.text.clone(), }); self.savepoints.push(Arc::downgrade(&savepoint)); savepoint } - pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint) { + pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint, emit_lsp_notification: bool) { assert_eq!( savepoint.view, view.id, "Savepoint must not be used with a different view!" @@ -1108,7 +1160,7 @@ impl Document { let savepoint_ref = self.savepoints.remove(savepoint_idx); let mut revert = savepoint.revert.lock(); - self.apply(&revert, view.id); + self.apply_inner(&revert, view.id, emit_lsp_notification); *revert = Transaction::new(self.text()).with_selection(self.selection(view.id).clone()); self.savepoints.push(savepoint_ref) } @@ -1121,7 +1173,7 @@ impl Document { }; let mut success = false; for txn in txns { - if self.apply_impl(&txn, view.id) { + if self.apply_impl(&txn, view.id, true) { success = true; } } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 0541b2403a6c6..9e057974f7693 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -2,7 +2,7 @@ use crate::{ align_view, annotations::diagnostics::InlineDiagnosticsConfig, clipboard::{get_clipboard_provider, ClipboardProvider}, - document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode}, + document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint}, graphics::{CursorKind, Rect}, info::Info, input::KeyEvent, @@ -910,9 +910,14 @@ enum ThemeAction { } #[derive(Debug, Clone)] -pub struct CompleteAction { - pub trigger_offset: usize, - pub changes: Vec, +pub enum CompleteAction { + Applied { + trigger_offset: usize, + changes: Vec, + }, + /// A savepoint of the currently selected completion. The savepoint + /// MUST be restored before sending any event to the LSP + Selected { savepoint: Arc }, } #[derive(Debug, Copy, Clone)]