Allow providing an external format in `format_on_save` setting

Antonio Scandurra created

Change summary

crates/editor/src/items.rs      |   7 
crates/language/src/buffer.rs   |  10 
crates/project/src/project.rs   | 247 +++++++++++++++++++++++++---------
crates/settings/src/settings.rs |  25 ++
4 files changed, 200 insertions(+), 89 deletions(-)

Detailed changes

crates/editor/src/items.rs 🔗

@@ -352,13 +352,8 @@ impl Item for Editor {
         project: ModelHandle<Project>,
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<()>> {
-        let settings = cx.global::<Settings>();
         let buffer = self.buffer().clone();
-        let mut buffers = buffer.read(cx).all_buffers();
-        buffers.retain(|buffer| {
-            let language_name = buffer.read(cx).language().map(|l| l.name());
-            settings.format_on_save(language_name.as_deref())
-        });
+        let buffers = buffer.read(cx).all_buffers();
         let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse();
         let format = project.update(cx, |project, cx| project.format(buffers, true, cx));
         cx.spawn(|this, mut cx| async move {

crates/language/src/buffer.rs 🔗

@@ -273,7 +273,7 @@ pub struct Chunk<'a> {
     pub is_unnecessary: bool,
 }
 
-pub(crate) struct Diff {
+pub struct Diff {
     base_version: clock::Global,
     new_text: Arc<str>,
     changes: Vec<(ChangeTag, usize)>,
@@ -958,7 +958,7 @@ impl Buffer {
         }
     }
 
-    pub(crate) fn diff(&self, mut new_text: String, cx: &AppContext) -> Task<Diff> {
+    pub fn diff(&self, mut new_text: String, cx: &AppContext) -> Task<Diff> {
         let old_text = self.as_rope().clone();
         let base_version = self.version();
         cx.background().spawn(async move {
@@ -979,11 +979,7 @@ impl Buffer {
         })
     }
 
-    pub(crate) fn apply_diff(
-        &mut self,
-        diff: Diff,
-        cx: &mut ModelContext<Self>,
-    ) -> Option<&Transaction> {
+    pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext<Self>) -> Option<&Transaction> {
         if self.version == diff.base_version {
             self.finalize_last_transaction();
             self.start_transaction();

crates/project/src/project.rs 🔗

@@ -12,7 +12,7 @@ use anyhow::{anyhow, Context, Result};
 use client::{proto, Client, PeerId, TypedEnvelope, User, UserStore};
 use clock::ReplicaId;
 use collections::{hash_map, BTreeMap, HashMap, HashSet};
-use futures::{future::Shared, Future, FutureExt, StreamExt, TryFutureExt};
+use futures::{future::Shared, AsyncWriteExt, Future, FutureExt, StreamExt, TryFutureExt};
 use fuzzy::{PathMatch, PathMatchCandidate, PathMatchCandidateSet};
 use gpui::{
     AnyModelHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle,
@@ -51,10 +51,12 @@ use std::{
     ffi::OsString,
     hash::Hash,
     mem,
+    num::NonZeroU32,
     ops::Range,
     os::unix::{ffi::OsStrExt, prelude::OsStringExt},
     path::{Component, Path, PathBuf},
     rc::Rc,
+    str,
     sync::{
         atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst},
         Arc,
@@ -3025,78 +3027,50 @@ impl Project {
             }
 
             for (buffer, buffer_abs_path, language_server) in local_buffers {
-                let text_document = lsp::TextDocumentIdentifier::new(
-                    lsp::Url::from_file_path(&buffer_abs_path).unwrap(),
-                );
-                let capabilities = &language_server.capabilities();
-                let tab_size = cx.update(|cx| {
-                    let language_name = buffer.read(cx).language().map(|language| language.name());
-                    cx.global::<Settings>().tab_size(language_name.as_deref())
+                let (format_on_save, tab_size) = buffer.read_with(&cx, |buffer, cx| {
+                    let settings = cx.global::<Settings>();
+                    let language_name = buffer.language().map(|language| language.name());
+                    (
+                        settings.format_on_save(language_name.as_deref()),
+                        settings.tab_size(language_name.as_deref()),
+                    )
                 });
-                let lsp_edits = if capabilities
-                    .document_formatting_provider
-                    .as_ref()
-                    .map_or(false, |provider| *provider != lsp::OneOf::Left(false))
-                {
-                    language_server
-                        .request::<lsp::request::Formatting>(lsp::DocumentFormattingParams {
-                            text_document,
-                            options: lsp::FormattingOptions {
-                                tab_size: tab_size.into(),
-                                insert_spaces: true,
-                                insert_final_newline: Some(true),
-                                ..Default::default()
-                            },
-                            work_done_progress_params: Default::default(),
-                        })
-                        .await?
-                } else if capabilities
-                    .document_range_formatting_provider
-                    .as_ref()
-                    .map_or(false, |provider| *provider != lsp::OneOf::Left(false))
-                {
-                    let buffer_start = lsp::Position::new(0, 0);
-                    let buffer_end =
-                        buffer.read_with(&cx, |buffer, _| point_to_lsp(buffer.max_point_utf16()));
-                    language_server
-                        .request::<lsp::request::RangeFormatting>(
-                            lsp::DocumentRangeFormattingParams {
-                                text_document,
-                                range: lsp::Range::new(buffer_start, buffer_end),
-                                options: lsp::FormattingOptions {
-                                    tab_size: tab_size.into(),
-                                    insert_spaces: true,
-                                    insert_final_newline: Some(true),
-                                    ..Default::default()
-                                },
-                                work_done_progress_params: Default::default(),
-                            },
+
+                let transaction = match format_on_save {
+                    settings::FormatOnSave::Off => continue,
+                    settings::FormatOnSave::LanguageServer => Self::format_via_lsp(
+                        &this,
+                        &buffer,
+                        &buffer_abs_path,
+                        &language_server,
+                        tab_size,
+                        &mut cx,
+                    )
+                    .await
+                    .context("failed to format via language server")?,
+                    settings::FormatOnSave::External { command, arguments } => {
+                        Self::format_via_external_command(
+                            &buffer,
+                            &buffer_abs_path,
+                            &command,
+                            &arguments,
+                            &mut cx,
                         )
-                        .await?
-                } else {
-                    continue;
+                        .await
+                        .context(format!(
+                            "failed to format via external command {:?}",
+                            command
+                        ))?
+                    }
                 };
 
-                if let Some(lsp_edits) = lsp_edits {
-                    let edits = this
-                        .update(&mut cx, |this, cx| {
-                            this.edits_from_lsp(&buffer, lsp_edits, None, cx)
-                        })
-                        .await?;
-                    buffer.update(&mut cx, |buffer, cx| {
-                        buffer.finalize_last_transaction();
-                        buffer.start_transaction();
-                        for (range, text) in edits {
-                            buffer.edit([(range, text)], cx);
-                        }
-                        if buffer.end_transaction(cx).is_some() {
-                            let transaction = buffer.finalize_last_transaction().unwrap().clone();
-                            if !push_to_history {
-                                buffer.forget_transaction(transaction.id);
-                            }
-                            project_transaction.0.insert(cx.handle(), transaction);
-                        }
-                    });
+                if let Some(transaction) = transaction {
+                    if !push_to_history {
+                        buffer.update(&mut cx, |buffer, _| {
+                            buffer.forget_transaction(transaction.id)
+                        });
+                    }
+                    project_transaction.0.insert(buffer, transaction);
                 }
             }
 
@@ -3104,6 +3078,141 @@ impl Project {
         })
     }
 
+    async fn format_via_lsp(
+        this: &ModelHandle<Self>,
+        buffer: &ModelHandle<Buffer>,
+        abs_path: &Path,
+        language_server: &Arc<LanguageServer>,
+        tab_size: NonZeroU32,
+        cx: &mut AsyncAppContext,
+    ) -> Result<Option<Transaction>> {
+        let text_document =
+            lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path(abs_path).unwrap());
+        let capabilities = &language_server.capabilities();
+        let lsp_edits = if capabilities
+            .document_formatting_provider
+            .as_ref()
+            .map_or(false, |provider| *provider != lsp::OneOf::Left(false))
+        {
+            language_server
+                .request::<lsp::request::Formatting>(lsp::DocumentFormattingParams {
+                    text_document,
+                    options: lsp::FormattingOptions {
+                        tab_size: tab_size.into(),
+                        insert_spaces: true,
+                        insert_final_newline: Some(true),
+                        ..Default::default()
+                    },
+                    work_done_progress_params: Default::default(),
+                })
+                .await?
+        } else if capabilities
+            .document_range_formatting_provider
+            .as_ref()
+            .map_or(false, |provider| *provider != lsp::OneOf::Left(false))
+        {
+            let buffer_start = lsp::Position::new(0, 0);
+            let buffer_end =
+                buffer.read_with(cx, |buffer, _| point_to_lsp(buffer.max_point_utf16()));
+            language_server
+                .request::<lsp::request::RangeFormatting>(lsp::DocumentRangeFormattingParams {
+                    text_document,
+                    range: lsp::Range::new(buffer_start, buffer_end),
+                    options: lsp::FormattingOptions {
+                        tab_size: tab_size.into(),
+                        insert_spaces: true,
+                        insert_final_newline: Some(true),
+                        ..Default::default()
+                    },
+                    work_done_progress_params: Default::default(),
+                })
+                .await?
+        } else {
+            None
+        };
+
+        if let Some(lsp_edits) = lsp_edits {
+            let edits = this
+                .update(cx, |this, cx| {
+                    this.edits_from_lsp(&buffer, lsp_edits, None, cx)
+                })
+                .await?;
+            buffer.update(cx, |buffer, cx| {
+                buffer.finalize_last_transaction();
+                buffer.start_transaction();
+                for (range, text) in edits {
+                    buffer.edit([(range, text)], cx);
+                }
+                if buffer.end_transaction(cx).is_some() {
+                    let transaction = buffer.finalize_last_transaction().unwrap().clone();
+                    Ok(Some(transaction))
+                } else {
+                    Ok(None)
+                }
+            })
+        } else {
+            Ok(None)
+        }
+    }
+
+    async fn format_via_external_command(
+        buffer: &ModelHandle<Buffer>,
+        buffer_abs_path: &Path,
+        command: &str,
+        arguments: &[String],
+        cx: &mut AsyncAppContext,
+    ) -> Result<Option<Transaction>> {
+        let working_dir_path = buffer.read_with(cx, |buffer, cx| {
+            let file = File::from_dyn(buffer.file())?;
+            let worktree = file.worktree.read(cx).as_local()?;
+            let mut worktree_path = worktree.abs_path().to_path_buf();
+            if worktree.root_entry()?.is_file() {
+                worktree_path.pop();
+            }
+            Some(worktree_path)
+        });
+
+        if let Some(working_dir_path) = working_dir_path {
+            let mut child =
+                smol::process::Command::new(command)
+                    .args(arguments.iter().map(|arg| {
+                        arg.replace("{buffer_path}", &buffer_abs_path.to_string_lossy())
+                    }))
+                    .current_dir(&working_dir_path)
+                    .stdin(smol::process::Stdio::piped())
+                    .stdout(smol::process::Stdio::piped())
+                    .stderr(smol::process::Stdio::piped())
+                    .spawn()?;
+            let stdin = child
+                .stdin
+                .as_mut()
+                .ok_or_else(|| anyhow!("failed to acquire stdin"))?;
+            let text = buffer.read_with(cx, |buffer, _| buffer.as_rope().clone());
+            for chunk in text.chunks() {
+                stdin.write_all(chunk.as_bytes()).await?;
+            }
+            stdin.flush().await?;
+
+            let output = child.output().await?;
+            if !output.status.success() {
+                return Err(anyhow!(
+                    "command failed with exit code {:?}:\nstdout: {}\nstderr: {}",
+                    output.status.code(),
+                    String::from_utf8_lossy(&output.stdout),
+                    String::from_utf8_lossy(&output.stderr),
+                ));
+            }
+
+            let stdout = String::from_utf8(output.stdout)?;
+            let diff = buffer
+                .read_with(cx, |buffer, cx| buffer.diff(stdout, cx))
+                .await;
+            Ok(buffer.update(cx, |buffer, cx| buffer.apply_diff(diff, cx).cloned()))
+        } else {
+            Ok(None)
+        }
+    }
+
     pub fn definition<T: ToPointUtf16>(
         &self,
         buffer: &ModelHandle<Buffer>,

crates/settings/src/settings.rs 🔗

@@ -38,7 +38,7 @@ pub struct LanguageSettings {
     pub hard_tabs: Option<bool>,
     pub soft_wrap: Option<SoftWrap>,
     pub preferred_line_length: Option<u32>,
-    pub format_on_save: Option<bool>,
+    pub format_on_save: Option<FormatOnSave>,
     pub enable_language_server: Option<bool>,
 }
 
@@ -50,6 +50,17 @@ pub enum SoftWrap {
     PreferredLineLength,
 }
 
+#[derive(Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)]
+#[serde(rename_all = "snake_case")]
+pub enum FormatOnSave {
+    Off,
+    LanguageServer,
+    External {
+        command: String,
+        arguments: Vec<String>,
+    },
+}
+
 #[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
 pub enum Autosave {
@@ -72,7 +83,7 @@ pub struct SettingsFileContent {
     #[serde(default)]
     pub vim_mode: Option<bool>,
     #[serde(default)]
-    pub format_on_save: Option<bool>,
+    pub format_on_save: Option<FormatOnSave>,
     #[serde(default)]
     pub autosave: Option<Autosave>,
     #[serde(default)]
@@ -136,9 +147,9 @@ impl Settings {
             .unwrap_or(80)
     }
 
-    pub fn format_on_save(&self, language: Option<&str>) -> bool {
-        self.language_setting(language, |settings| settings.format_on_save)
-            .unwrap_or(true)
+    pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave {
+        self.language_setting(language, |settings| settings.format_on_save.clone())
+            .unwrap_or(FormatOnSave::LanguageServer)
     }
 
     pub fn enable_language_server(&self, language: Option<&str>) -> bool {
@@ -215,7 +226,7 @@ impl Settings {
         merge(&mut self.autosave, data.autosave);
         merge_option(
             &mut self.language_settings.format_on_save,
-            data.format_on_save,
+            data.format_on_save.clone(),
         );
         merge_option(
             &mut self.language_settings.enable_language_server,
@@ -339,7 +350,7 @@ fn merge<T: Copy>(target: &mut T, value: Option<T>) {
     }
 }
 
-fn merge_option<T: Copy>(target: &mut Option<T>, value: Option<T>) {
+fn merge_option<T>(target: &mut Option<T>, value: Option<T>) {
     if value.is_some() {
         *target = value;
     }