Change default formatter for any language (#2942)

* Change default formatter for any language

* Fix clippy error

* Close stdin for Stdio formatters

* Better indentation and pattern matching

* Return Result<Option<...>> for fn format instead of Option

* Remove unwrap for stdin

* Handle FormatterErrors instead of Result<Option<...>>

* Use Transaction instead of LspFormatting

* Use Transaction directly in Document::format

* Perform stdin type formatting asynchronously

* Rename formatter.type values to kebab-case

* Debug format for displaying io::ErrorKind (msrv fix)

* Solve conflict?

* Use only stdio type formatters

* Remove FormatterType enum

* Remove old comment

* Check if the formatter exited correctly

* Add formatter configuration to the book

* Avoid allocations when writing to stdin and formatting errors

* Remove unused import

Co-authored-by: Gokul Soumya <gokulps15@gmail.com>

2 files changed, 94 insertions, 12 deletions

diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index 50c70af6..ab64f485 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -1,4 +1,6 @@
 use anyhow::{anyhow, bail, Context, Error};
+use futures_util::future::BoxFuture;
+use futures_util::FutureExt;
 use helix_core::auto_pairs::AutoPairs;
 use helix_core::Range;
 use serde::de::{self, Deserialize, Deserializer};
@@ -20,7 +22,6 @@ use helix_core::{
     ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, Selection, State, Syntax, Transaction,
     DEFAULT_LINE_ENDING,
 };
-use helix_lsp::util::LspFormatting;
 
 use crate::{DocumentId, Editor, ViewId};
 
@@ -397,7 +398,7 @@ impl Document {
 
     /// The same as [`format`], but only returns formatting changes if auto-formatting
     /// is configured.
-    pub fn auto_format(&self) -> Option<impl Future<Output = LspFormatting> + 'static> {
+    pub fn auto_format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
         if self.language_config()?.auto_format {
             self.format()
         } else {
@@ -407,7 +408,56 @@ impl Document {
 
     /// If supported, returns the changes that should be applied to this document in order
     /// to format it nicely.
-    pub fn format(&self) -> Option<impl Future<Output = LspFormatting> + 'static> {
+    // We can't use anyhow::Result here since the output of the future has to be
+    // clonable to be used as shared future. So use a custom error type.
+    pub fn format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
+        if let Some(formatter) = self.language_config().and_then(|c| c.formatter.clone()) {
+            use std::process::Stdio;
+            let text = self.text().clone();
+            let mut process = tokio::process::Command::new(&formatter.command);
+            process
+                .args(&formatter.args)
+                .stdin(Stdio::piped())
+                .stdout(Stdio::piped())
+                .stderr(Stdio::piped());
+
+            let formatting_future = async move {
+                let mut process = process
+                    .spawn()
+                    .map_err(|e| FormatterError::SpawningFailed {
+                        command: formatter.command.clone(),
+                        error: e.kind(),
+                    })?;
+                {
+                    let mut stdin = process.stdin.take().ok_or(FormatterError::BrokenStdin)?;
+                    to_writer(&mut stdin, encoding::UTF_8, &text)
+                        .await
+                        .map_err(|_| FormatterError::BrokenStdin)?;
+                }
+
+                let output = process
+                    .wait_with_output()
+                    .await
+                    .map_err(|_| FormatterError::WaitForOutputFailed)?;
+
+                if !output.stderr.is_empty() {
+                    return Err(FormatterError::Stderr(
+                        String::from_utf8_lossy(&output.stderr).to_string(),
+                    ));
+                }
+
+                if !output.status.success() {
+                    return Err(FormatterError::NonZeroExitStatus);
+                }
+
+                let str = String::from_utf8(output.stdout)
+                    .map_err(|_| FormatterError::InvalidUtf8Output)?;
+
+                Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str)))
+            };
+            return Some(formatting_future.boxed());
+        };
+
         let language_server = self.language_server()?;
         let text = self.text.clone();
         let offset_encoding = language_server.offset_encoding();
@@ -427,13 +477,13 @@ impl Document {
                 log::warn!("LSP formatting failed: {}", e);
                 Default::default()
             });
-            LspFormatting {
-                doc: text,
+            Ok(helix_lsp::util::generate_transaction_from_edits(
+                &text,
                 edits,
                 offset_encoding,
-            }
+            ))
         };
-        Some(fut)
+        Some(fut.boxed())
     }
 
     pub fn save(&mut self, force: bool) -> impl Future<Output = Result<(), anyhow::Error>> {
@@ -442,7 +492,7 @@ impl Document {
 
     pub fn format_and_save(
         &mut self,
-        formatting: Option<impl Future<Output = LspFormatting>>,
+        formatting: Option<impl Future<Output = Result<Transaction, FormatterError>>>,
         force: bool,
     ) -> impl Future<Output = anyhow::Result<()>> {
         self.save_impl(formatting, force)
@@ -454,7 +504,7 @@ impl Document {
     /// at its `path()`.
     ///
     /// If `formatting` is present, it supplies some changes that we apply to the text before saving.
-    fn save_impl<F: Future<Output = LspFormatting>>(
+    fn save_impl<F: Future<Output = Result<Transaction, FormatterError>>>(
         &mut self,
         formatting: Option<F>,
         force: bool,
@@ -488,7 +538,8 @@ impl Document {
             }
 
             if let Some(fmt) = formatting {
-                let success = Transaction::from(fmt.await).changes().apply(&mut text);
+                let transaction = fmt.await?;
+                let success = transaction.changes().apply(&mut text);
                 if !success {
                     // This shouldn't happen, because the transaction changes were generated
                     // from the same text we're saving.
@@ -1034,6 +1085,38 @@ impl Default for Document {
     }
 }
 
+#[derive(Clone, Debug)]
+pub enum FormatterError {
+    SpawningFailed {
+        command: String,
+        error: std::io::ErrorKind,
+    },
+    BrokenStdin,
+    WaitForOutputFailed,
+    Stderr(String),
+    InvalidUtf8Output,
+    DiskReloadError(String),
+    NonZeroExitStatus,
+}
+
+impl std::error::Error for FormatterError {}
+
+impl Display for FormatterError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::SpawningFailed { command, error } => {
+                write!(f, "Failed to spawn formatter {}: {:?}", command, error)
+            }
+            Self::BrokenStdin => write!(f, "Could not write to formatter stdin"),
+            Self::WaitForOutputFailed => write!(f, "Waiting for formatter output failed"),
+            Self::Stderr(output) => write!(f, "Formatter error: {}", output),
+            Self::InvalidUtf8Output => write!(f, "Invalid UTF-8 formatter output"),
+            Self::DiskReloadError(error) => write!(f, "Error reloading file from disk: {}", error),
+            Self::NonZeroExitStatus => write!(f, "Formatter exited with non zero exit status:"),
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 501c3069..1e7f508c 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -613,7 +613,6 @@ impl Editor {
         syn_loader: Arc<syntax::Loader>,
         config: Box<dyn DynAccess<Config>>,
     ) -> Self {
-        let language_servers = helix_lsp::Registry::new();
         let conf = config.load();
         let auto_pairs = (&conf.auto_pairs).into();
 
@@ -629,7 +628,7 @@ impl Editor {
             macro_recording: None,
             macro_replaying: Vec::new(),
             theme: theme_loader.default(),
-            language_servers,
+            language_servers: helix_lsp::Registry::new(),
             diagnostics: BTreeMap::new(),
             debugger: None,
             debugger_events: SelectAll::new(),