Allow textDocument/didChange notification to be synchronous

helix-lsp/src/client.rs

@@ -12,6 +12,7 @@ use crate::lsp::{
 };
 use helix_core::{find_workspace, syntax::LanguageServerFeature, ChangeSet, Rope};
 use helix_loader::VERSION_AND_GIT_HASH;
+use helix_lsp_types::TextDocumentContentChangeEvent;
 use helix_stdx::path;
 use parking_lot::Mutex;
 use serde::Deserialize;
@@ -482,6 +483,28 @@ impl Client {
         }
     }
 
+    /// Send a RPC notification to the language server synchronously.
+    pub fn notify_sync<R: lsp::notification::Notification>(&self, params: R::Params) -> Result<()>
+    where
+        R::Params: serde::Serialize,
+    {
+        let server_tx = self.server_tx.clone();
+
+        let params = serde_json::to_value(params)?;
+
+        let notification = jsonrpc::Notification {
+            jsonrpc: Some(jsonrpc::Version::V2),
+            method: R::METHOD.to_string(),
+            params: Self::value_into_params(params),
+        };
+
+        server_tx
+            .send(Payload::Notification(notification))
+            .map_err(|e| Error::Other(e.into()))?;
+
+        Ok(())
+    }
+
     /// Reply to a language server RPC call.
     pub fn reply(
         &self,
@@ -930,6 +953,44 @@ impl Client {
         new_text: &Rope,
         changes: &ChangeSet,
     ) -> Option<impl Future<Output = Result<()>>> {
+        if let Some(changes) = self.text_document_did_change_impl(old_text, new_text, changes) {
+            return Some(self.notify::<lsp::notification::DidChangeTextDocument>(
+                lsp::DidChangeTextDocumentParams {
+                    text_document,
+                    content_changes: changes,
+                },
+            ));
+        };
+        None
+    }
+
+    /// Will send textDocument/didChange notificaiton synchronously
+    pub fn text_document_did_change_sync(
+        &self,
+        text_document: lsp::VersionedTextDocumentIdentifier,
+        old_text: &Rope,
+        new_text: &Rope,
+        changes: &ChangeSet,
+    ) -> Option<Result<()>> {
+        if let Some(changes) = self.text_document_did_change_impl(old_text, new_text, changes) {
+            return Some(
+                self.notify_sync::<lsp::notification::DidChangeTextDocument>(
+                    lsp::DidChangeTextDocumentParams {
+                        text_document,
+                        content_changes: changes,
+                    },
+                ),
+            );
+        };
+        None
+    }
+
+    pub fn text_document_did_change_impl(
+        &self,
+        old_text: &Rope,
+        new_text: &Rope,
+        changes: &ChangeSet,
+    ) -> Option<Vec<TextDocumentContentChangeEvent>> {
         let capabilities = self.capabilities.get().unwrap();
 
         // Return early if the server does not support document sync.
@@ -961,12 +1022,7 @@ impl Client {
             kind => unimplemented!("{:?}", kind),
         };
 
-        Some(self.notify::<lsp::notification::DidChangeTextDocument>(
-            lsp::DidChangeTextDocumentParams {
-                text_document,
-                content_changes: changes,
-            },
-        ))
+        Some(changes)
     }
 
     pub fn text_document_did_close(

helix-term/src/commands/lsp.rs

@@ -551,6 +551,11 @@ struct CodeActionOrCommandItem {
     language_server_id: LanguageServerId,
 }
 
+struct CodeActionItem {
+    lsp_item: lsp::CodeAction,
+    language_server_id: LanguageServerId,
+}
+
 impl ui::menu::Item for CodeActionOrCommandItem {
     type Data = ();
     fn format(&self, _data: &Self::Data) -> Row {
@@ -712,8 +717,36 @@ pub fn code_action(cx: &mut Context) {
 
                 // always present here
                 let action = action.unwrap();
+                let Some(language_server) = editor.language_server_by_id(action.language_server_id)
+                else {
+                    editor.set_error("Language Server disappeared");
+                    return;
+                };
+                let offset_encoding = language_server.offset_encoding();
+
+                match &action.lsp_item {
+                    lsp::CodeActionOrCommand::Command(command) => {
+                        log::debug!("code action command: {:?}", command);
+                        execute_lsp_command(editor, action.language_server_id, command.clone());
+                    }
+                    lsp::CodeActionOrCommand::CodeAction(code_action) => {
+                        log::debug!("code action: {:?}", code_action);
+                        let resolved_code_action =
+                            resolve_code_action(code_action, language_server);
+
+                        if let Some(ref workspace_edit) =
+                            resolved_code_action.as_ref().unwrap_or(code_action).edit
+                        {
+                            let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit);
+                        }
 
-                apply_code_action(editor, action);
+                        // if code action provides both edit and command first the edit
+                        // should be applied and then the command
+                        if let Some(command) = &code_action.command {
+                            execute_lsp_command(editor, action.language_server_id, command.clone());
+                        }
+                    }
+                }
             });
             picker.move_down(); // pre-select the first item
 
@@ -780,7 +813,7 @@ pub fn code_actions_on_save(cx: &mut compositor::Context, doc_id: &DocumentId) {
             log::debug!("Attempting code action on save {:?}", code_action_kind);
             let doc = doc!(cx.editor, doc_id);
             let full_range = Range::new(0, doc.text().len_chars());
-            let code_actions: Vec<CodeActionOrCommandItem> =
+            let code_actions: Vec<CodeActionItem> =
                 code_actions_for_range(doc, full_range, Some(vec![code_action_kind.clone()]))
                     .into_iter()
                     .filter_map(|(future, language_server_id)| {
@@ -808,10 +841,15 @@ pub fn code_actions_on_save(cx: &mut compositor::Context, doc_id: &DocumentId) {
 
                                 // Use the first matching code action
                                 if let Some(lsp_item) = actions.first() {
-                                    return Some(CodeActionOrCommandItem {
-                                        lsp_item: lsp_item.clone(),
-                                        language_server_id,
-                                    });
+                                    return match lsp_item {
+                                        CodeActionOrCommand::CodeAction(code_action) => {
+                                            Some(CodeActionItem {
+                                                lsp_item: code_action.clone(),
+                                                language_server_id,
+                                            })
+                                        }
+                                        _ => None,
+                                    };
                                 }
                             }
                         }
@@ -831,55 +869,52 @@ pub fn code_actions_on_save(cx: &mut compositor::Context, doc_id: &DocumentId) {
                     code_action.lsp_item,
                     code_action.language_server_id
                 );
-                apply_code_action(cx.editor, &code_action);
+                let Some(language_server) = cx
+                    .editor
+                    .language_server_by_id(code_action.language_server_id)
+                else {
+                    log::error!(
+                        "Language server disappeared {:?}",
+                        code_action.language_server_id
+                    );
+                    continue;
+                };
 
-                // TODO: Find a better way to handle this
-                // Sleep to avoid race condition between next code action/auto-format
-                // and the textDocument/didChange notification
-                std::thread::sleep(std::time::Duration::from_millis(10));
+                let offset_encoding = language_server.offset_encoding();
+
+                let resolved_code_action =
+                    resolve_code_action(&code_action.lsp_item, language_server);
+
+                if let Some(ref workspace_edit) = resolved_code_action
+                    .as_ref()
+                    .unwrap_or(&code_action.lsp_item)
+                    .edit
+                {
+                    let _ = cx
+                        .editor
+                        .apply_workspace_edit_sync(offset_encoding, workspace_edit);
+                }
             }
         }
     }
 }
 
-fn apply_code_action(editor: &mut Editor, action: &CodeActionOrCommandItem) {
-    let Some(language_server) = editor.language_server_by_id(action.language_server_id) else {
-        editor.set_error("Language Server disappeared");
-        return;
-    };
-    let offset_encoding = language_server.offset_encoding();
-
-    match &action.lsp_item {
-        lsp::CodeActionOrCommand::Command(command) => {
-            log::debug!("code action command: {:?}", command);
-            execute_lsp_command(editor, action.language_server_id, command.clone());
-        }
-        lsp::CodeActionOrCommand::CodeAction(code_action) => {
-            log::debug!("code action: {:?}", code_action);
-            // we support lsp "codeAction/resolve" for `edit` and `command` fields
-            let mut resolved_code_action = None;
-            if code_action.edit.is_none() || code_action.command.is_none() {
-                if let Some(future) = language_server.resolve_code_action(code_action.clone()) {
-                    if let Ok(response) = helix_lsp::block_on(future) {
-                        if let Ok(code_action) = serde_json::from_value::<CodeAction>(response) {
-                            resolved_code_action = Some(code_action);
-                        }
-                    }
+pub fn resolve_code_action(
+    code_action: &CodeAction,
+    language_server: &Client,
+) -> Option<CodeAction> {
+    // we support lsp "codeAction/resolve" for `edit` and `command` fields
+    let mut resolved_code_action = None;
+    if code_action.edit.is_none() || code_action.command.is_none() {
+        if let Some(future) = language_server.resolve_code_action(code_action.clone()) {
+            if let Ok(response) = helix_lsp::block_on(future) {
+                if let Ok(code_action) = serde_json::from_value::<CodeAction>(response) {
+                    resolved_code_action = Some(code_action);
                 }
             }
-            let resolved_code_action = resolved_code_action.as_ref().unwrap_or(code_action);
-
-            if let Some(ref workspace_edit) = resolved_code_action.edit {
-                let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit);
-            }
-
-            // if code action provides both edit and command first the edit
-            // should be applied and then the command
-            if let Some(command) = &code_action.command {
-                execute_lsp_command(editor, action.language_server_id, command.clone());
-            }
         }
     }
+    resolved_code_action
 }
 
 pub fn execute_lsp_command(

helix-term/src/ui/completion.rs

@@ -253,7 +253,7 @@ impl Completion {
                         })
                     }
                     // if more text was entered, remove it
-                    doc.restore(view, &savepoint, false);
+                    doc.restore(view, &savepoint, None);
                     // always present here
                     let item = item.unwrap();
 
@@ -273,7 +273,7 @@ impl Completion {
                     if let Some(CompleteAction::Selected { savepoint }) =
                         editor.last_completion.take()
                     {
-                        doc.restore(view, &savepoint, false);
+                        doc.restore(view, &savepoint, None);
                     }
                     // always present here
                     let mut item = item.unwrap().clone();
@@ -289,7 +289,11 @@ impl Completion {
                         }
                     };
                     // if more text was entered, remove it
-                    doc.restore(view, &savepoint, true);
+                    doc.restore(
+                        view,
+                        &savepoint,
+                        Some(helix_view::document::EmitLspNotification::Async),
+                    );
                     // save an undo checkpoint before the completion
                     doc.append_changes_to_history(view);
                     let transaction = item_to_transaction(

helix-term/src/ui/editor.rs

@@ -974,7 +974,11 @@ impl EditorView {
                                 let (view, doc) = current!(cxt.editor);
 
                                 if let Some(last_savepoint) = last_savepoint.as_deref() {
-                                    doc.restore(view, last_savepoint, true);
+                                    doc.restore(
+                                        view,
+                                        last_savepoint,
+                                        Some(helix_view::document::EmitLspNotification::Async),
+                                    );
                                 }
 
                                 let text = doc.text().slice(..);
@@ -1063,7 +1067,7 @@ impl EditorView {
                 }),
                 CompleteAction::Selected { savepoint } => {
                     let (view, doc) = current!(editor);
-                    doc.restore(view, &savepoint, false);
+                    doc.restore(view, &savepoint, None);
                 }
             }
         }

helix-view/src/document.rs

@@ -130,6 +130,12 @@ pub enum DocumentOpenError {
     IoError(#[from] io::Error),
 }
 
+#[derive(Debug)]
+pub enum EmitLspNotification {
+    Async,
+    Sync,
+}
+
 pub struct Document {
     pub(crate) id: DocumentId,
     text: Rope,
@@ -1269,7 +1275,7 @@ impl Document {
         &mut self,
         transaction: &Transaction,
         view_id: ViewId,
-        emit_lsp_notification: bool,
+        emit_lsp_notification: Option<EmitLspNotification>,
     ) -> bool {
         use helix_core::Assoc;
 
@@ -1426,19 +1432,31 @@ impl Document {
             });
         }
 
-        if emit_lsp_notification {
-            // TODO: move to hook
-            // emit lsp notification
+        // emit lsp notification
+        if let Some(emit_lsp_notification) = emit_lsp_notification {
             for language_server in self.language_servers() {
-                let notify = language_server.text_document_did_change(
-                    self.versioned_identifier(),
-                    &old_doc,
-                    self.text(),
-                    changes,
-                );
-
-                if let Some(notify) = notify {
-                    tokio::spawn(notify);
+                match emit_lsp_notification {
+                    EmitLspNotification::Async => {
+                        // TODO: move to hook
+                        let notify = language_server.text_document_did_change(
+                            self.versioned_identifier(),
+                            &old_doc,
+                            self.text(),
+                            changes,
+                        );
+
+                        if let Some(notify) = notify {
+                            tokio::spawn(notify);
+                        }
+                    }
+                    EmitLspNotification::Sync => {
+                        let _ = language_server.text_document_did_change_sync(
+                            self.versioned_identifier(),
+                            &old_doc,
+                            self.text(),
+                            changes,
+                        );
+                    }
                 }
             }
         }
@@ -1450,7 +1468,7 @@ impl Document {
         &mut self,
         transaction: &Transaction,
         view_id: ViewId,
-        emit_lsp_notification: bool,
+        emit_lsp_notification: Option<EmitLspNotification>,
     ) -> bool {
         // store the state just before any changes are made. This allows us to undo to the
         // state just before a transaction was applied.
@@ -1473,14 +1491,20 @@ impl Document {
     }
     /// 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)
+        self.apply_inner(transaction, view_id, Some(EmitLspNotification::Async))
+    }
+
+    /// Apply a [`Transaction`] to the [`Document`] to change its text and
+    /// emit the lsp notifcation synchronously
+    pub fn apply_sync_notification(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {
+        self.apply_inner(transaction, view_id, Some(EmitLspNotification::Sync))
     }
 
     /// 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)
+        self.apply_inner(transaction, view_id, None)
     }
 
     fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool {
@@ -1492,7 +1516,7 @@ impl Document {
         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, true)
+            self.apply_impl(txn, view.id, Some(EmitLspNotification::Async))
         } else {
             false
         };
@@ -1548,7 +1572,12 @@ impl Document {
         savepoint
     }
 
-    pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint, emit_lsp_notification: bool) {
+    pub fn restore(
+        &mut self,
+        view: &mut View,
+        savepoint: &SavePoint,
+        emit_lsp_notification: Option<EmitLspNotification>,
+    ) {
         assert_eq!(
             savepoint.view, view.id,
             "Savepoint must not be used with a different view!"
@@ -1581,7 +1610,7 @@ impl Document {
         };
         let mut success = false;
         for txn in txns {
-            if self.apply_impl(&txn, view.id, true) {
+            if self.apply_impl(&txn, view.id, Some(EmitLspNotification::Async)) {
                 success = true;
             }
         }

helix-view/src/handlers/lsp.rs

@@ -93,6 +93,7 @@ impl Editor {
         version: Option<i32>,
         text_edits: Vec<lsp::TextEdit>,
         offset_encoding: OffsetEncoding,
+        sync_lsp_notification: bool,
     ) -> Result<(), ApplyEditErrorKind> {
         let uri = match Uri::try_from(url) {
             Ok(uri) => uri,
@@ -133,16 +134,37 @@ impl Editor {
 
         let transaction = generate_transaction_from_edits(doc.text(), text_edits, offset_encoding);
         let view = view_mut!(self, view_id);
-        doc.apply(&transaction, view.id);
+        if sync_lsp_notification {
+            doc.apply_sync_notification(&transaction, view.id);
+        } else {
+            doc.apply(&transaction, view.id);
+        }
         doc.append_changes_to_history(view);
         Ok(())
     }
 
-    // TODO make this transactional (and set failureMode to transactional)
     pub fn apply_workspace_edit(
         &mut self,
         offset_encoding: OffsetEncoding,
         workspace_edit: &lsp::WorkspaceEdit,
+    ) -> Result<(), ApplyEditError> {
+        self.apply_workspace_edit_impl(offset_encoding, workspace_edit, false)
+    }
+
+    pub fn apply_workspace_edit_sync(
+        &mut self,
+        offset_encoding: OffsetEncoding,
+        workspace_edit: &lsp::WorkspaceEdit,
+    ) -> Result<(), ApplyEditError> {
+        self.apply_workspace_edit_impl(offset_encoding, workspace_edit, true)
+    }
+
+    // TODO make this transactional (and set failureMode to transactional)
+    fn apply_workspace_edit_impl(
+        &mut self,
+        offset_encoding: OffsetEncoding,
+        workspace_edit: &lsp::WorkspaceEdit,
+        sync_lsp_notificaiton: bool,
     ) -> Result<(), ApplyEditError> {
         if let Some(ref document_changes) = workspace_edit.document_changes {
             match document_changes {
@@ -164,6 +186,7 @@ impl Editor {
                             document_edit.text_document.version,
                             edits,
                             offset_encoding,
+                            sync_lsp_notificaiton,
                         )
                         .map_err(|kind| ApplyEditError {
                             kind,
@@ -201,6 +224,7 @@ impl Editor {
                                     document_edit.text_document.version,
                                     edits,
                                     offset_encoding,
+                                    sync_lsp_notificaiton,
                                 )
                                 .map_err(|kind| {
                                     ApplyEditError {
@@ -221,11 +245,17 @@ impl Editor {
             log::debug!("workspace changes: {:?}", changes);
             for (i, (uri, text_edits)) in changes.iter().enumerate() {
                 let text_edits = text_edits.to_vec();
-                self.apply_text_edits(uri, None, text_edits, offset_encoding)
-                    .map_err(|kind| ApplyEditError {
-                        kind,
-                        failed_change_idx: i,
-                    })?;
+                self.apply_text_edits(
+                    uri,
+                    None,
+                    text_edits,
+                    offset_encoding,
+                    sync_lsp_notificaiton,
+                )
+                .map_err(|kind| ApplyEditError {
+                    kind,
+                    failed_change_idx: i,
+                })?;
             }
         }