First attempt at sync formatting.

helix-lsp/src/lib.rs

@@ -182,6 +182,22 @@ pub mod util {
             }),
         )
     }
+
+    /// The result of asking the language server to format the document. This can be turned into a
+    /// `Transaction`, but the advantage of not doing that straight away is that this one is
+    /// `Send` and `Sync`.
+    #[derive(Clone, Debug)]
+    pub struct LspFormatting {
+        pub doc: Rope,
+        pub edits: Vec<lsp::TextEdit>,
+        pub offset_encoding: OffsetEncoding,
+    }
+
+    impl From<LspFormatting> for Transaction {
+        fn from(fmt: LspFormatting) -> Transaction {
+            generate_transaction_from_edits(&fmt.doc, fmt.edits, fmt.offset_encoding)
+        }
+    }
 }
 
 #[derive(Debug, PartialEq, Clone)]

helix-term/src/application.rs

@@ -6,6 +6,7 @@ use crate::{
     args::Args,
     compositor::Compositor,
     config::Config,
+    job::Jobs,
     keymap::Keymaps,
     ui::{self, Spinner},
 };
@@ -31,13 +32,6 @@ use crossterm::{
 
 use futures_util::{future, stream::FuturesUnordered};
 
-type BoxFuture<T> = Pin<Box<dyn Future<Output = T> + Send>>;
-pub type LspCallback =
-    BoxFuture<Result<Box<dyn FnOnce(&mut Editor, &mut Compositor) + Send>, anyhow::Error>>;
-
-pub type LspCallbacks = FuturesUnordered<LspCallback>;
-pub type LspCallbackWrapper = Box<dyn FnOnce(&mut Editor, &mut Compositor) + Send>;
-
 pub struct Application {
     compositor: Compositor,
     editor: Editor,
@@ -48,7 +42,7 @@ pub struct Application {
     theme_loader: Arc<theme::Loader>,
     syn_loader: Arc<syntax::Loader>,
 
-    callbacks: LspCallbacks,
+    jobs: Jobs,
     lsp_progress: LspProgressMap,
 }
 
@@ -120,7 +114,7 @@ impl Application {
             theme_loader,
             syn_loader,
 
-            callbacks: FuturesUnordered::new(),
+            jobs: Jobs::new(),
             lsp_progress: LspProgressMap::new(),
         };
 
@@ -130,11 +124,11 @@ impl Application {
     fn render(&mut self) {
         let editor = &mut self.editor;
         let compositor = &mut self.compositor;
-        let callbacks = &mut self.callbacks;
+        let jobs = &mut self.jobs;
 
         let mut cx = crate::compositor::Context {
             editor,
-            callbacks,
+            jobs,
             scroll: None,
         };
 
@@ -148,6 +142,7 @@ impl Application {
 
         loop {
             if self.editor.should_close() {
+                self.jobs.finish();
                 break;
             }
 
@@ -172,27 +167,18 @@ impl Application {
                     }
                     self.render();
                 }
-                Some(callback) = &mut self.callbacks.next() => {
-                    self.handle_language_server_callback(callback)
+                Some(callback) = self.jobs.next_job() => {
+                    self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback);
+                    self.render();
                 }
             }
         }
     }
-    pub fn handle_language_server_callback(
-        &mut self,
-        callback: Result<LspCallbackWrapper, anyhow::Error>,
-    ) {
-        if let Ok(callback) = callback {
-            // TODO: handle Err()
-            callback(&mut self.editor, &mut self.compositor);
-            self.render();
-        }
-    }
 
     pub fn handle_terminal_events(&mut self, event: Option<Result<Event, crossterm::ErrorKind>>) {
         let mut cx = crate::compositor::Context {
             editor: &mut self.editor,
-            callbacks: &mut self.callbacks,
+            jobs: &mut self.jobs,
             scroll: None,
         };
         // Handle key events

helix-term/src/commands.rs

@@ -35,8 +35,8 @@ use crate::{
     ui::{self, Completion, Picker, Popup, Prompt, PromptEvent},
 };
 
-use crate::application::{LspCallbackWrapper, LspCallbacks};
-use futures_util::FutureExt;
+use crate::job::{self, Job, JobFuture, Jobs};
+use futures_util::{FutureExt, TryFutureExt};
 use std::{fmt, future::Future, path::Display, str::FromStr};
 
 use std::{
@@ -54,7 +54,7 @@ pub struct Context<'a> {
 
     pub callback: Option<crate::compositor::Callback>,
     pub on_next_key_callback: Option<Box<dyn FnOnce(&mut Context, KeyEvent)>>,
-    pub callbacks: &'a mut LspCallbacks,
+    pub jobs: &'a mut Jobs,
 }
 
 impl<'a> Context<'a> {
@@ -85,13 +85,13 @@ impl<'a> Context<'a> {
         let callback = Box::pin(async move {
             let json = call.await?;
             let response = serde_json::from_value(json)?;
-            let call: LspCallbackWrapper =
+            let call: job::Callback =
                 Box::new(move |editor: &mut Editor, compositor: &mut Compositor| {
                     callback(editor, compositor, response)
                 });
             Ok(call)
         });
-        self.callbacks.push(callback);
+        self.jobs.callback(callback);
     }
 
     /// Returns 1 if no explicit count was provided
@@ -1106,11 +1106,12 @@ mod cmd {
     }
 
     fn write_impl<P: AsRef<Path>>(
-        view: &View,
-        doc: &mut Document,
+        cx: &mut compositor::Context,
         path: Option<P>,
     ) -> Result<tokio::task::JoinHandle<Result<(), anyhow::Error>>, anyhow::Error> {
         use anyhow::anyhow;
+        let jobs = &mut cx.jobs;
+        let (view, doc) = current!(cx.editor);
 
         if let Some(path) = path {
             if let Err(err) = doc.set_path(path.as_ref()) {
@@ -1120,20 +1121,27 @@ mod cmd {
         if doc.path().is_none() {
             return Err(anyhow!("cannot write a buffer without a filename"));
         }
-        let autofmt = doc
-            .language_config()
-            .map(|config| config.auto_format)
-            .unwrap_or_default();
-        if autofmt {
-            doc.format(view.id); // TODO: merge into save
-        }
-        Ok(tokio::spawn(doc.save()))
+        let fmt = doc.auto_format().map(|fmt| {
+            let shared = fmt.shared();
+            let callback = make_format_callback(
+                doc.id(),
+                doc.version(),
+                Modified::SetUnmodified,
+                shared.clone(),
+            );
+            jobs.callback(callback);
+            shared
+        });
+        Ok(tokio::spawn(doc.format_and_save(fmt)))
     }
 
     fn write(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) {
-        let (view, doc) = current!(cx.editor);
-        if let Err(e) = write_impl(view, doc, args.first()) {
-            cx.editor.set_error(e.to_string());
+        match write_impl(cx, args.first()) {
+            Err(e) => cx.editor.set_error(e.to_string()),
+            Ok(handle) => {
+                cx.jobs
+                    .add(Job::new(handle.unwrap_or_else(|e| Err(e.into()))).wait_before_exiting());
+            }
         };
     }
 
@@ -1142,9 +1150,13 @@ mod cmd {
     }
 
     fn format(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) {
-        let (view, doc) = current!(cx.editor);
+        let (_, doc) = current!(cx.editor);
 
-        doc.format(view.id)
+        if let Some(format) = doc.format() {
+            let callback =
+                make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format);
+            cx.jobs.callback(callback);
+        }
     }
 
     fn set_indent_style(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) {
@@ -1249,8 +1261,7 @@ mod cmd {
     }
 
     fn write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) {
-        let (view, doc) = current!(cx.editor);
-        match write_impl(view, doc, args.first()) {
+        match write_impl(cx, args.first()) {
             Ok(handle) => {
                 if let Err(e) = helix_lsp::block_on(handle) {
                     cx.editor.set_error(e.to_string());
@@ -1266,7 +1277,7 @@ mod cmd {
 
     fn force_write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) {
         let (view, doc) = current!(cx.editor);
-        match write_impl(view, doc, args.first()) {
+        match write_impl(cx, args.first()) {
             Ok(handle) => {
                 if let Err(e) = helix_lsp::block_on(handle) {
                     cx.editor.set_error(e.to_string());
@@ -1863,6 +1874,42 @@ fn append_to_line(cx: &mut Context) {
     doc.set_selection(view.id, selection);
 }
 
+/// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for
+/// example because we just applied the same changes while saving.
+enum Modified {
+    SetUnmodified,
+    LeaveModified,
+}
+
+// Creates an LspCallback that waits for formatting changes to be computed. When they're done,
+// it applies them, but only if the doc hasn't changed.
+//
+// TODO: provide some way to cancel this, probably as part of a more general job cancellation
+// scheme
+async fn make_format_callback(
+    doc_id: DocumentId,
+    doc_version: i32,
+    modified: Modified,
+    format: impl Future<Output = helix_lsp::util::LspFormatting> + Send + 'static,
+) -> anyhow::Result<job::Callback> {
+    let format = format.await;
+    let call: job::Callback = Box::new(move |editor: &mut Editor, compositor: &mut Compositor| {
+        let view_id = view!(editor).id;
+        if let Some(doc) = editor.document_mut(doc_id) {
+            if doc.version() == doc_version {
+                doc.apply(&Transaction::from(format), view_id);
+                doc.append_changes_to_history(view_id);
+                if let Modified::SetUnmodified = modified {
+                    doc.reset_modified();
+                }
+            } else {
+                log::info!("discarded formatting changes because the document changed");
+            }
+        }
+    });
+    Ok(call)
+}
+
 enum Open {
     Below,
     Above,

helix-term/src/compositor.rs

@@ -26,12 +26,12 @@ pub enum EventResult {
 
 use helix_view::Editor;
 
-use crate::application::LspCallbacks;
+use crate::job::Jobs;
 
 pub struct Context<'a> {
     pub editor: &'a mut Editor,
     pub scroll: Option<usize>,
-    pub callbacks: &'a mut LspCallbacks,
+    pub jobs: &'a mut Jobs,
 }
 
 pub trait Component: Any + AnyComponent {