Merge pull request #1302 from zed-industries/external-formatting

Introduce support for formatting via an external command

crates/client/src/client.rs

@@ -549,7 +549,7 @@ impl Client {
                 client.respond_with_error(
                     receipt,
                     proto::Error {
-                        message: error.to_string(),
+                        message: format!("{:?}", error),
                     },
                 )?;
                 Err(error)

crates/collab/src/integration_tests.rs

@@ -35,7 +35,7 @@ use project::{
 use rand::prelude::*;
 use rpc::PeerId;
 use serde_json::json;
-use settings::Settings;
+use settings::{FormatOnSave, Settings};
 use sqlx::types::time::OffsetDateTime;
 use std::{
     cell::RefCell,
@@ -1912,7 +1912,6 @@ async fn test_reloading_buffer_manually(cx_a: &mut TestAppContext, cx_b: &mut Te
 
 #[gpui::test(iterations = 10)]
 async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
-    cx_a.foreground().forbid_parking();
     let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await;
     let client_a = server.create_client(cx_a, "user_a").await;
     let client_b = server.create_client(cx_b, "user_b").await;
@@ -1932,11 +1931,15 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon
     let mut fake_language_servers = language.set_fake_lsp_adapter(Default::default());
     client_a.language_registry.add(Arc::new(language));
 
+    // Here we insert a fake tree with a directory that exists on disk. This is needed
+    // because later we'll invoke a command, which requires passing a working directory
+    // that points to a valid location on disk.
+    let directory = env::current_dir().unwrap();
     client_a
         .fs
-        .insert_tree("/a", json!({ "a.rs": "let one = two" }))
+        .insert_tree(&directory, json!({ "a.rs": "let one = \"two\"" }))
         .await;
-    let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await;
+    let (project_a, worktree_id) = client_a.build_local_project(&directory, cx_a).await;
     let project_b = client_b.build_remote_project(&project_a, cx_a, cx_b).await;
 
     let buffer_b = cx_b
@@ -1967,7 +1970,28 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon
         .unwrap();
     assert_eq!(
         buffer_b.read_with(cx_b, |buffer, _| buffer.text()),
-        "let honey = two"
+        "let honey = \"two\""
+    );
+
+    // Ensure buffer can be formatted using an external command. Notice how the
+    // host's configuration is honored as opposed to using the guest's settings.
+    cx_a.update(|cx| {
+        cx.update_global(|settings: &mut Settings, _| {
+            settings.language_settings.format_on_save = Some(FormatOnSave::External {
+                command: "awk".to_string(),
+                arguments: vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()],
+            });
+        });
+    });
+    project_b
+        .update(cx_b, |project, cx| {
+            project.format(HashSet::from_iter([buffer_b.clone()]), true, cx)
+        })
+        .await
+        .unwrap();
+    assert_eq!(
+        buffer_b.read_with(cx_b, |buffer, _| buffer.text()),
+        format!("let honey = \"{}/a.rs\"\n", directory.to_str().unwrap())
     );
 }
 

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/fs.rs

@@ -334,28 +334,6 @@ impl FakeFs {
         })
     }
 
-    pub async fn insert_dir(&self, path: impl AsRef<Path>) {
-        let mut state = self.state.lock().await;
-        let path = path.as_ref();
-        state.validate_path(path).unwrap();
-
-        let inode = state.next_inode;
-        state.next_inode += 1;
-        state.entries.insert(
-            path.to_path_buf(),
-            FakeFsEntry {
-                metadata: Metadata {
-                    inode,
-                    mtime: SystemTime::now(),
-                    is_dir: true,
-                    is_symlink: false,
-                },
-                content: None,
-            },
-        );
-        state.emit_event(&[path]).await;
-    }
-
     pub async fn insert_file(&self, path: impl AsRef<Path>, content: String) {
         let mut state = self.state.lock().await;
         let path = path.as_ref();
@@ -392,7 +370,7 @@ impl FakeFs {
 
             match tree {
                 Object(map) => {
-                    self.insert_dir(path).await;
+                    self.create_dir(path).await.unwrap();
                     for (name, contents) in map {
                         let mut path = PathBuf::from(path);
                         path.push(name);
@@ -400,7 +378,7 @@ impl FakeFs {
                     }
                 }
                 Null => {
-                    self.insert_dir(&path).await;
+                    self.create_dir(&path).await.unwrap();
                 }
                 String(contents) => {
                     self.insert_file(&path, contents).await;

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;
     }

crates/zed/src/zed.rs

@@ -554,7 +554,7 @@ mod tests {
         });
 
         let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx));
-        app_state.fs.as_fake().insert_dir("/root").await;
+        app_state.fs.create_dir(Path::new("/root")).await.unwrap();
         cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name")));
         save_task.await.unwrap();
         editor.read_with(cx, |editor, cx| {
@@ -680,14 +680,25 @@ mod tests {
     async fn test_open_paths(cx: &mut TestAppContext) {
         let app_state = init(cx);
 
-        let fs = app_state.fs.as_fake();
-        fs.insert_dir("/dir1").await;
-        fs.insert_dir("/dir2").await;
-        fs.insert_dir("/dir3").await;
-        fs.insert_file("/dir1/a.txt", "".into()).await;
-        fs.insert_file("/dir2/b.txt", "".into()).await;
-        fs.insert_file("/dir3/c.txt", "".into()).await;
-        fs.insert_file("/d.txt", "".into()).await;
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/",
+                json!({
+                    "dir1": {
+                        "a.txt": ""
+                    },
+                    "dir2": {
+                        "b.txt": ""
+                    },
+                    "dir3": {
+                        "c.txt": ""
+                    },
+                    "d.txt": ""
+                }),
+            )
+            .await;
 
         let project = Project::test(app_state.fs.clone(), ["/dir1".as_ref()], cx).await;
         let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx));
@@ -891,7 +902,7 @@ mod tests {
     #[gpui::test]
     async fn test_open_and_save_new_file(cx: &mut TestAppContext) {
         let app_state = init(cx);
-        app_state.fs.as_fake().insert_dir("/root").await;
+        app_state.fs.create_dir(Path::new("/root")).await.unwrap();
 
         let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await;
         project.update(cx, |project, _| project.languages().add(rust_lang()));
@@ -980,7 +991,7 @@ mod tests {
     #[gpui::test]
     async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) {
         let app_state = init(cx);
-        app_state.fs.as_fake().insert_dir("/root").await;
+        app_state.fs.create_dir(Path::new("/root")).await.unwrap();
 
         let project = Project::test(app_state.fs.clone(), [], cx).await;
         project.update(cx, |project, _| project.languages().add(rust_lang()));