diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 538b0fa4b0..0e9ec4076a 100644 --- a/crates/client/src/client.rs +++ b/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) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index a4c8386b13..7767b361c1 100644 --- a/crates/collab/src/integration_tests.rs +++ b/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()) ); } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index f7aa80beaa..0e3aca1447 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -352,13 +352,8 @@ impl Item for Editor { project: ModelHandle, cx: &mut ViewContext, ) -> Task> { - let settings = cx.global::(); 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 { diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index cebf5f504e..d5ed1c1620 100644 --- a/crates/language/src/buffer.rs +++ b/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, changes: Vec<(ChangeTag, usize)>, @@ -958,7 +958,7 @@ impl Buffer { } } - pub(crate) fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { + pub fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { 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, - ) -> Option<&Transaction> { + pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option<&Transaction> { if self.version == diff.base_version { self.finalize_last_transaction(); self.start_transaction(); diff --git a/crates/project/src/fs.rs b/crates/project/src/fs.rs index 17d7264f1d..5c52801611 100644 --- a/crates/project/src/fs.rs +++ b/crates/project/src/fs.rs @@ -334,28 +334,6 @@ impl FakeFs { }) } - pub async fn insert_dir(&self, path: impl AsRef) { - 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, 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; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e4425e3414..0ac3064e56 100644 --- a/crates/project/src/project.rs +++ b/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::().tab_size(language_name.as_deref()) + let (format_on_save, tab_size) = buffer.read_with(&cx, |buffer, cx| { + let settings = cx.global::(); + 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::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::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, + buffer: &ModelHandle, + abs_path: &Path, + language_server: &Arc, + tab_size: NonZeroU32, + cx: &mut AsyncAppContext, + ) -> Result> { + 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::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::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_abs_path: &Path, + command: &str, + arguments: &[String], + cx: &mut AsyncAppContext, + ) -> Result> { + 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( &self, buffer: &ModelHandle, diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 0a59d28cc9..98df5e2f1f 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -38,7 +38,7 @@ pub struct LanguageSettings { pub hard_tabs: Option, pub soft_wrap: Option, pub preferred_line_length: Option, - pub format_on_save: Option, + pub format_on_save: Option, pub enable_language_server: Option, } @@ -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, + }, +} + #[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, #[serde(default)] - pub format_on_save: Option, + pub format_on_save: Option, #[serde(default)] pub autosave: Option, #[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(target: &mut T, value: Option) { } } -fn merge_option(target: &mut Option, value: Option) { +fn merge_option(target: &mut Option, value: Option) { if value.is_some() { *target = value; } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f88aee3d7c..56ccfaf9fe 100644 --- a/crates/zed/src/zed.rs +++ b/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()));