From f3395cf4fd5fe900dac3eccebb476655a6321de6 Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 22 Sep 2022 18:21:05 -0400 Subject: [PATCH 1/2] Add editor action to manually invoke buffer format --- assets/keymaps/default.json | 1 + assets/settings/default.json | 13 ++++--- crates/collab/src/integration_tests.rs | 20 ++++++++--- crates/editor/src/editor.rs | 48 ++++++++++++++++++++++++-- crates/editor/src/items.rs | 10 +++--- crates/project/src/project.rs | 36 +++++++++++++++---- crates/rpc/proto/zed.proto | 8 ++++- crates/rpc/src/rpc.rs | 2 +- crates/settings/src/settings.rs | 19 ++++++---- 9 files changed, 124 insertions(+), 33 deletions(-) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 7a25dc19d3bcd0ff491f31d9ae1609b1ba267f3e..1d994f86afd925e0b1f0e46f7c7427af57acf0ce 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -93,6 +93,7 @@ "cmd-shift-down": "editor::SelectToEnd", "cmd-a": "editor::SelectAll", "cmd-l": "editor::SelectLine", + "cmd-shift-i": "editor::Format", "cmd-shift-left": [ "editor::SelectToBeginningOfLine", { diff --git a/assets/settings/default.json b/assets/settings/default.json index d8efdc41ff02851e28a8536c4bb63acc8c393ae0..34b665b41d70e3d8edb6e3692ac45076fda6de01 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -42,21 +42,20 @@ // 3. Position the dock full screen over the entire workspace" // "default_dock_anchor": "expanded" "default_dock_anchor": "right", - // How to auto-format modified buffers when saving them. This - // setting can take three values: + // Whether or not to perform a buffer format before saving + "format_on_save": true, + // How to perform a buffer format. This setting can take two values: // - // 1. Don't format code - // "format_on_save": "off" - // 2. Format code using the current language server: + // 1. Format code using the current language server: // "format_on_save": "language_server" - // 3. Format code using an external command: + // 2. Format code using an external command: // "format_on_save": { // "external": { // "command": "prettier", // "arguments": ["--stdin-filepath", "{buffer_path}"] // } // } - "format_on_save": "language_server", + "formatter": "language_server", // How to soft-wrap long lines of text. This setting can take // three values: // diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 6b512d950ff6f9a0e7a2c782a88bce7c60d918ee..073547472854be6be9dedec921a17a89b467d5e6 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -36,7 +36,7 @@ use project::{ use rand::prelude::*; use rpc::PeerId; use serde_json::json; -use settings::{FormatOnSave, Settings}; +use settings::{Formatter, Settings}; use sqlx::types::time::OffsetDateTime; use std::{ cell::RefCell, @@ -1990,6 +1990,8 @@ 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) { + use project::FormatTrigger; + 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; @@ -2042,7 +2044,12 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon project_b .update(cx_b, |project, cx| { - project.format(HashSet::from_iter([buffer_b.clone()]), true, cx) + project.format( + HashSet::from_iter([buffer_b.clone()]), + true, + FormatTrigger::Save, + cx, + ) }) .await .unwrap(); @@ -2055,7 +2062,7 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon // host's configuration is honored as opposed to using the guest's settings. cx_a.update(|cx| { cx.update_global(|settings: &mut Settings, _| { - settings.editor_defaults.format_on_save = Some(FormatOnSave::External { + settings.editor_defaults.formatter = Some(Formatter::External { command: "awk".to_string(), arguments: vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()], }); @@ -2063,7 +2070,12 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon }); project_b .update(cx_b, |project, cx| { - project.format(HashSet::from_iter([buffer_b.clone()]), true, cx) + project.format( + HashSet::from_iter([buffer_b.clone()]), + true, + FormatTrigger::Save, + cx, + ) }) .await .unwrap(); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a7d46f771046a0067b31a9aeb1a8e6d6c926a98b..b23e1660b6d8005a25b4f7db14abaa7e0ee33f27 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -19,6 +19,7 @@ use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; pub use display_map::DisplayPoint; use display_map::*; pub use element::*; +use futures::FutureExt; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, @@ -49,7 +50,7 @@ pub use multi_buffer::{ }; use multi_buffer::{MultiBufferChunks, ToOffsetUtf16}; use ordered_float::OrderedFloat; -use project::{LocationLink, Project, ProjectPath, ProjectTransaction}; +use project::{FormatTrigger, LocationLink, Project, ProjectPath, ProjectTransaction}; use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection}; use serde::{Deserialize, Serialize}; use settings::Settings; @@ -76,6 +77,8 @@ const MAX_LINE_LEN: usize = 1024; const MIN_NAVIGATION_HISTORY_ROW_DELTA: i64 = 10; const MAX_SELECTION_HISTORY_LEN: usize = 1024; +pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); + #[derive(Clone, Deserialize, PartialEq, Default)] pub struct SelectNext { #[serde(default)] @@ -204,6 +207,7 @@ actions!( OpenExcerpts, RestartLanguageServer, Hover, + Format, ] ); @@ -310,6 +314,7 @@ pub fn init(cx: &mut MutableAppContext) { cx.add_action(Editor::toggle_code_actions); cx.add_action(Editor::open_excerpts); cx.add_action(Editor::jump); + cx.add_async_action(Editor::format); cx.add_action(Editor::restart_language_server); cx.add_action(Editor::show_character_palette); cx.add_async_action(Editor::confirm_completion); @@ -5173,6 +5178,43 @@ impl Editor { self.pending_rename.as_ref() } + fn format(&mut self, _: &Format, cx: &mut ViewContext<'_, Self>) -> Option>> { + let project = match &self.project { + Some(project) => project, + None => return None, + }; + + let buffer = self.buffer().clone(); + 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, FormatTrigger::Manual, cx) + }); + + Some(cx.spawn(|_, mut cx| async move { + let transaction = futures::select_biased! { + _ = timeout => { + log::warn!("timed out waiting for formatting"); + None + } + transaction = format.log_err().fuse() => transaction, + }; + + buffer.update(&mut cx, |buffer, cx| { + if let Some(transaction) = transaction { + if !buffer.is_singleton() { + buffer.push_transaction(&transaction.0); + } + } + + cx.notify(); + }); + + Ok(()) + })) + } + fn restart_language_server(&mut self, _: &RestartLanguageServer, cx: &mut ViewContext) { if let Some(project) = self.project.clone() { self.buffer.update(cx, |multi_buffer, cx| { @@ -10087,7 +10129,7 @@ mod tests { unreachable!() }); let save = cx.update(|cx| editor.save(project.clone(), cx)); - cx.foreground().advance_clock(items::FORMAT_TIMEOUT); + cx.foreground().advance_clock(super::FORMAT_TIMEOUT); cx.foreground().start_waiting(); save.await.unwrap(); assert_eq!( @@ -10203,7 +10245,7 @@ mod tests { }, ); let save = cx.update(|cx| editor.save(project.clone(), cx)); - cx.foreground().advance_clock(items::FORMAT_TIMEOUT); + cx.foreground().advance_clock(super::FORMAT_TIMEOUT); cx.foreground().start_waiting(); save.await.unwrap(); assert_eq!( diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index fb6f12a16f7335dcdc62d0809158faa14d656881..ba600b2a5cad5c739098071bfa9f92f91ea70b16 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1,7 +1,7 @@ use crate::{ display_map::ToDisplayPoint, link_go_to_definition::hide_link_definition, movement::surrounding_word, Anchor, Autoscroll, Editor, Event, ExcerptId, MultiBuffer, - MultiBufferSnapshot, NavigationData, ToPoint as _, + MultiBufferSnapshot, NavigationData, ToPoint as _, FORMAT_TIMEOUT, }; use anyhow::{anyhow, Result}; use futures::FutureExt; @@ -10,7 +10,7 @@ use gpui::{ RenderContext, Subscription, Task, View, ViewContext, ViewHandle, }; use language::{Bias, Buffer, File as _, OffsetRangeExt, SelectionGoal}; -use project::{File, Project, ProjectEntryId, ProjectPath}; +use project::{File, FormatTrigger, Project, ProjectEntryId, ProjectPath}; use rpc::proto::{self, update_view}; use settings::Settings; use smallvec::SmallVec; @@ -20,7 +20,6 @@ use std::{ fmt::Write, ops::Range, path::{Path, PathBuf}, - time::Duration, }; use text::{Point, Selection}; use util::TryFutureExt; @@ -30,7 +29,6 @@ use workspace::{ ToolbarItemLocation, }; -pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); pub const MAX_TAB_TITLE_LEN: usize = 24; impl FollowableItem for Editor { @@ -409,7 +407,9 @@ impl Item for Editor { let buffer = self.buffer().clone(); 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)); + let format = project.update(cx, |project, cx| { + project.format(buffers, true, FormatTrigger::Save, cx) + }); cx.spawn(|_, mut cx| async move { let transaction = futures::select_biased! { _ = timeout => { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 09c5a72315d3552e4df29d0608186966503be3a6..8a3ffbc1c45a5257c590f85d2e04796c79770d70 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -364,6 +364,22 @@ impl ProjectEntryId { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FormatTrigger { + Save, + Manual, +} + +impl FormatTrigger { + fn from_proto(value: i32) -> FormatTrigger { + match value { + 0 => FormatTrigger::Save, + 1 => FormatTrigger::Manual, + _ => FormatTrigger::Save, + } + } +} + impl Project { pub fn init(client: &Arc) { client.add_model_message_handler(Self::handle_request_join_project); @@ -3047,6 +3063,7 @@ impl Project { &self, buffers: HashSet>, push_to_history: bool, + trigger: FormatTrigger, cx: &mut ModelContext, ) -> Task> { let mut local_buffers = Vec::new(); @@ -3076,6 +3093,7 @@ impl Project { let response = client .request(proto::FormatBuffers { project_id, + trigger: trigger as i32, buffer_ids: remote_buffers .iter() .map(|buffer| buffer.read_with(&cx, |buffer, _| buffer.remote_id())) @@ -3092,18 +3110,22 @@ impl Project { } for (buffer, buffer_abs_path, language_server) in local_buffers { - let (format_on_save, tab_size) = buffer.read_with(&cx, |buffer, cx| { + let (format_on_save, formatter, 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.formatter(language_name.as_deref()), settings.tab_size(language_name.as_deref()), ) }); - let transaction = match format_on_save { - settings::FormatOnSave::Off => continue, - settings::FormatOnSave::LanguageServer => Self::format_via_lsp( + if trigger == FormatTrigger::Save && !format_on_save { + continue; + } + + let transaction = match formatter { + settings::Formatter::LanguageServer => Self::format_via_lsp( &this, &buffer, &buffer_abs_path, @@ -3113,7 +3135,8 @@ impl Project { ) .await .context("failed to format via language server")?, - settings::FormatOnSave::External { command, arguments } => { + + settings::Formatter::External { command, arguments } => { Self::format_via_external_command( &buffer, &buffer_abs_path, @@ -5296,7 +5319,8 @@ impl Project { .ok_or_else(|| anyhow!("unknown buffer id {}", buffer_id))?, ); } - Ok::<_, anyhow::Error>(this.format(buffers, false, cx)) + let trigger = FormatTrigger::from_proto(envelope.payload.trigger); + Ok::<_, anyhow::Error>(this.format(buffers, false, trigger, cx)) })?; let project_transaction = format.await?; diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 4cf7e38a139037ccd294ebc4cc377724779023cb..7840829b4461bc7ca497f7bf1bcc7b34e397f0f8 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -420,9 +420,15 @@ message ReloadBuffersResponse { ProjectTransaction transaction = 1; } +enum FormatTrigger { + Save = 0; + Manual = 1; +} + message FormatBuffers { uint64 project_id = 1; - repeated uint64 buffer_ids = 2; + FormatTrigger trigger = 2; + repeated uint64 buffer_ids = 3; } message FormatBuffersResponse { diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index 308e5b0f4349c281616c56884279e4aae622a93e..b9f6e6a7390a759b4317ed53bd7309d47a9e37b3 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 31; +pub const PROTOCOL_VERSION: u32 = 32; diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index d2b076b0126e6b4ebe56f3ea340e33954cd4bd98..8e4c49572a8a4bb01c6efea0d79dc97924bb1684 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -58,7 +58,8 @@ pub struct EditorSettings { pub hard_tabs: Option, pub soft_wrap: Option, pub preferred_line_length: Option, - pub format_on_save: Option, + pub format_on_save: Option, + pub formatter: Option, pub enable_language_server: Option, } @@ -72,8 +73,7 @@ pub enum SoftWrap { #[derive(Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] -pub enum FormatOnSave { - Off, +pub enum Formatter { LanguageServer, External { command: String, @@ -207,6 +207,7 @@ impl Settings { font_cache: &FontCache, themes: &ThemeRegistry, ) -> Self { + #[track_caller] fn required(value: Option) -> Option { assert!(value.is_some(), "missing default setting value"); value @@ -236,6 +237,7 @@ impl Settings { soft_wrap: required(defaults.editor.soft_wrap), preferred_line_length: required(defaults.editor.preferred_line_length), format_on_save: required(defaults.editor.format_on_save), + formatter: required(defaults.editor.formatter), enable_language_server: required(defaults.editor.enable_language_server), }, editor_overrides: Default::default(), @@ -322,8 +324,12 @@ impl Settings { self.language_setting(language, |settings| settings.preferred_line_length) } - pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave { - self.language_setting(language, |settings| settings.format_on_save.clone()) + pub fn format_on_save(&self, language: Option<&str>) -> bool { + self.language_setting(language, |settings| settings.format_on_save) + } + + pub fn formatter(&self, language: Option<&str>) -> Formatter { + self.language_setting(language, |settings| settings.formatter.clone()) } pub fn enable_language_server(&self, language: Option<&str>) -> bool { @@ -358,7 +364,8 @@ impl Settings { hard_tabs: Some(false), soft_wrap: Some(SoftWrap::None), preferred_line_length: Some(80), - format_on_save: Some(FormatOnSave::LanguageServer), + format_on_save: Some(true), + formatter: Some(Formatter::LanguageServer), enable_language_server: Some(true), }, editor_overrides: Default::default(), From 12e439bda950fc00fd6c712ebb8603f2a6144557 Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 23 Sep 2022 12:15:24 -0400 Subject: [PATCH 2/2] Test manual buffer format trigger --- crates/editor/src/editor.rs | 95 +++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b23e1660b6d8005a25b4f7db14abaa7e0ee33f27..c9faa7c662462d744dd6fd1dd1386a2fd10cdaea 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5180,10 +5180,18 @@ impl Editor { fn format(&mut self, _: &Format, cx: &mut ViewContext<'_, Self>) -> Option>> { let project = match &self.project { - Some(project) => project, + Some(project) => project.clone(), None => return None, }; + Some(self.perform_format(project, cx)) + } + + fn perform_format( + &mut self, + project: ModelHandle, + cx: &mut ViewContext<'_, Self>, + ) -> Task> { let buffer = self.buffer().clone(); let buffers = buffer.read(cx).all_buffers(); @@ -5192,7 +5200,7 @@ impl Editor { project.format(buffers, true, FormatTrigger::Manual, cx) }); - Some(cx.spawn(|_, mut cx| async move { + cx.spawn(|_, mut cx| async move { let transaction = futures::select_biased! { _ = timeout => { log::warn!("timed out waiting for formatting"); @@ -5212,7 +5220,7 @@ impl Editor { }); Ok(()) - })) + }) } fn restart_language_server(&mut self, _: &RestartLanguageServer, cx: &mut ViewContext) { @@ -10283,6 +10291,87 @@ mod tests { save.await.unwrap(); } + #[gpui::test] + async fn test_document_format_manual_trigger(cx: &mut gpui::TestAppContext) { + cx.foreground().forbid_parking(); + + let mut language = Language::new( + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".to_string()], + ..Default::default() + }, + Some(tree_sitter_rust::language()), + ); + let mut fake_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + ..Default::default() + })) + .await; + + let fs = FakeFs::new(cx.background()); + fs.insert_file("/file.rs", Default::default()).await; + + let project = Project::test(fs, ["/file.rs".as_ref()], cx).await; + project.update(cx, |project, _| project.languages().add(Arc::new(language))); + let buffer = project + .update(cx, |project, cx| project.open_local_buffer("/file.rs", cx)) + .await + .unwrap(); + + cx.foreground().start_waiting(); + let fake_server = fake_servers.next().await.unwrap(); + + let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx)); + let (_, editor) = cx.add_window(|cx| build_editor(buffer, cx)); + editor.update(cx, |editor, cx| editor.set_text("one\ntwo\nthree\n", cx)); + + let format = editor.update(cx, |editor, cx| editor.perform_format(project.clone(), cx)); + fake_server + .handle_request::(move |params, _| async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/file.rs").unwrap() + ); + assert_eq!(params.options.tab_size, 4); + Ok(Some(vec![lsp::TextEdit::new( + lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(1, 0)), + ", ".to_string(), + )])) + }) + .next() + .await; + cx.foreground().start_waiting(); + format.await.unwrap(); + assert_eq!( + editor.read_with(cx, |editor, cx| editor.text(cx)), + "one, two\nthree\n" + ); + + editor.update(cx, |editor, cx| editor.set_text("one\ntwo\nthree\n", cx)); + // Ensure we don't lock if formatting hangs. + fake_server.handle_request::(move |params, _| async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/file.rs").unwrap() + ); + futures::future::pending::<()>().await; + unreachable!() + }); + let format = editor.update(cx, |editor, cx| editor.perform_format(project, cx)); + cx.foreground().advance_clock(super::FORMAT_TIMEOUT); + cx.foreground().start_waiting(); + format.await.unwrap(); + assert_eq!( + editor.read_with(cx, |editor, cx| editor.text(cx)), + "one\ntwo\nthree\n" + ); + } + #[gpui::test] async fn test_completion(cx: &mut gpui::TestAppContext) { let mut cx = EditorLspTestContext::new_rust(