From cbd266052dee2455ef1252a1a2f1eeafdf664e2d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 10:44:32 +0200 Subject: [PATCH 1/8] Allow returning futures in fake language server request handlers --- crates/editor/src/editor.rs | 80 +++++---- crates/lsp/src/lsp.rs | 55 +++--- crates/project/src/project.rs | 6 +- crates/server/src/rpc.rs | 306 +++++++++++++++++++--------------- 4 files changed, 250 insertions(+), 197 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f9327ee2519d0521fd6fb05b812411b81c9abc3a..b269ce3cd5d43d26a03b4177fcffda4f47c9b6f3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -8850,31 +8850,34 @@ mod tests { position: Point, completions: Vec<(Range, &'static str)>, ) { - fake.handle_request::(move |params, _| { - assert_eq!( - params.text_document_position.text_document.uri, - lsp::Url::from_file_path(path).unwrap() - ); - assert_eq!( - params.text_document_position.position, - lsp::Position::new(position.row, position.column) - ); - Some(lsp::CompletionResponse::Array( - completions - .iter() - .map(|(range, new_text)| lsp::CompletionItem { - label: new_text.to_string(), - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: lsp::Range::new( - lsp::Position::new(range.start.row, range.start.column), - lsp::Position::new(range.start.row, range.start.column), - ), - new_text: new_text.to_string(), - })), - ..Default::default() - }) - .collect(), - )) + fake.handle_request::(move |params, _| { + let completions = completions.clone(); + async move { + assert_eq!( + params.text_document_position.text_document.uri, + lsp::Url::from_file_path(path).unwrap() + ); + assert_eq!( + params.text_document_position.position, + lsp::Position::new(position.row, position.column) + ); + Some(lsp::CompletionResponse::Array( + completions + .iter() + .map(|(range, new_text)| lsp::CompletionItem { + label: new_text.to_string(), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range::new( + lsp::Position::new(range.start.row, range.start.column), + lsp::Position::new(range.start.row, range.start.column), + ), + new_text: new_text.to_string(), + })), + ..Default::default() + }) + .collect(), + )) + } }) .next() .await; @@ -8884,18 +8887,21 @@ mod tests { fake: &mut FakeLanguageServer, edit: Option<(Range, &'static str)>, ) { - fake.handle_request::(move |_, _| { - lsp::CompletionItem { - additional_text_edits: edit.clone().map(|(range, new_text)| { - vec![lsp::TextEdit::new( - lsp::Range::new( - lsp::Position::new(range.start.row, range.start.column), - lsp::Position::new(range.end.row, range.end.column), - ), - new_text.to_string(), - )] - }), - ..Default::default() + fake.handle_request::(move |_, _| { + let edit = edit.clone(); + async move { + lsp::CompletionItem { + additional_text_edits: edit.map(|(range, new_text)| { + vec![lsp::TextEdit::new( + lsp::Range::new( + lsp::Position::new(range.start.row, range.start.column), + lsp::Position::new(range.end.row, range.end.column), + ), + new_text.to_string(), + )] + }), + ..Default::default() + } } }) .next() diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index a8ae9e4cd7071632fb30fb9086c574c4940f53be..6e729fc62bfe2f0bb35307ed460846f08325d85c 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -1,6 +1,8 @@ use anyhow::{anyhow, Context, Result}; use collections::HashMap; -use futures::{channel::oneshot, io::BufWriter, AsyncRead, AsyncWrite}; +use futures::{ + channel::oneshot, future::BoxFuture, io::BufWriter, AsyncRead, AsyncWrite, FutureExt, +}; use gpui::{executor, Task}; use parking_lot::{Mutex, RwLock}; use postage::{barrier, prelude::Stream}; @@ -556,7 +558,10 @@ type FakeLanguageServerHandlers = Arc< Mutex< HashMap< &'static str, - Box Vec>, + Box< + dyn Send + + FnMut(usize, &[u8], gpui::AsyncAppContext) -> BoxFuture<'static, Vec>, + >, >, >, >; @@ -585,11 +590,16 @@ impl LanguageServer { let (stdout_writer, stdout_reader) = async_pipe::pipe(); let mut fake = FakeLanguageServer::new(stdin_reader, stdout_writer, cx); - fake.handle_request::({ + fake.handle_request::({ let capabilities = capabilities.clone(); - move |_, _| InitializeResult { - capabilities: capabilities.clone(), - ..Default::default() + move |_, _| { + let capabilities = capabilities.clone(); + async move { + InitializeResult { + capabilities, + ..Default::default() + } + } } }); @@ -628,7 +638,8 @@ impl FakeLanguageServer { let response; if let Some(handler) = handlers.lock().get_mut(request.method) { response = - handler(request.id, request.params.get().as_bytes(), cx.clone()); + handler(request.id, request.params.get().as_bytes(), cx.clone()) + .await; log::debug!("handled lsp request. method:{}", request.method); } else { response = serde_json::to_vec(&AnyResponse { @@ -704,28 +715,34 @@ impl FakeLanguageServer { } } - pub fn handle_request( + pub fn handle_request( &mut self, mut handler: F, ) -> futures::channel::mpsc::UnboundedReceiver<()> where T: 'static + request::Request, - F: 'static + Send + FnMut(T::Params, gpui::AsyncAppContext) -> T::Result, + F: 'static + Send + FnMut(T::Params, gpui::AsyncAppContext) -> Fut, + Fut: 'static + Send + Future, { let (responded_tx, responded_rx) = futures::channel::mpsc::unbounded(); self.handlers.lock().insert( T::METHOD, Box::new(move |id, params, cx| { let result = handler(serde_json::from_slice::(params).unwrap(), cx); - let result = serde_json::to_string(&result).unwrap(); - let result = serde_json::from_str::<&RawValue>(&result).unwrap(); - let response = AnyResponse { - id, - error: None, - result: Some(result), - }; - responded_tx.unbounded_send(()).ok(); - serde_json::to_vec(&response).unwrap() + let responded_tx = responded_tx.clone(); + async move { + let result = result.await; + let result = serde_json::to_string(&result).unwrap(); + let result = serde_json::from_str::<&RawValue>(&result).unwrap(); + let response = AnyResponse { + id, + error: None, + result: Some(result), + }; + responded_tx.unbounded_send(()).ok(); + serde_json::to_vec(&response).unwrap() + } + .boxed() }), ); responded_rx @@ -844,7 +861,7 @@ mod tests { "file://b/c" ); - fake.handle_request::(|_, _| ()); + fake.handle_request::(|_, _| async move {}); drop(server); fake.receive_notification::().await; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0801043a09aec1845f8fc2a0b4eca438a1215b5b..404d867069ec383c24dac33f550ac63306468122 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5789,7 +5789,7 @@ mod tests { .unwrap(); let mut fake_server = fake_servers.next().await.unwrap(); - fake_server.handle_request::(move |params, _| { + fake_server.handle_request::(|params, _| async move { let params = params.text_document_position_params; assert_eq!( params.text_document.uri.to_file_path().unwrap(), @@ -6724,7 +6724,7 @@ mod tests { project.prepare_rename(buffer.clone(), 7, cx) }); fake_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!(params.text_document.uri.as_str(), "file:///dir/one.rs"); assert_eq!(params.position, lsp::Position::new(0, 7)); Some(lsp::PrepareRenameResponse::Range(lsp::Range::new( @@ -6743,7 +6743,7 @@ mod tests { project.perform_rename(buffer.clone(), 7, "THREE".to_string(), true, cx) }); fake_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!( params.text_document_position.text_document.uri.as_str(), "file:///dir/one.rs" diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 458712676391fc245878324157426bb9a3da5e98..374aaf6a7ec6820022c1802757cdabf3b51aa946 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -2360,7 +2360,7 @@ mod tests { // Return some completions from the host's language server. cx_a.foreground().start_waiting(); fake_language_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!( params.text_document_position.text_document.uri, lsp::Url::from_file_path("/a/main.rs").unwrap(), @@ -2424,8 +2424,8 @@ mod tests { // Return a resolved completion from the host's language server. // The resolved completion has an additional text edit. - fake_language_server.handle_request::( - |params, _| { + fake_language_server.handle_request::( + |params, _| async move { assert_eq!(params.label, "first_method(…)"); lsp::CompletionItem { label: "first_method(…)".into(), @@ -2535,7 +2535,7 @@ mod tests { .unwrap(); let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|_, _| { + fake_language_server.handle_request::(|_, _| async move { Some(vec![ lsp::TextEdit { range: lsp::Range::new(lsp::Position::new(0, 4), lsp::Position::new(0, 4)), @@ -2644,12 +2644,14 @@ mod tests { // Request the definition of a symbol as the guest. let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|_, _| { - Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( - lsp::Url::from_file_path("/root-2/b.rs").unwrap(), - lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), - ))) - }); + fake_language_server.handle_request::( + |_, _| async move { + Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( + lsp::Url::from_file_path("/root-2/b.rs").unwrap(), + lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), + ))) + }, + ); let definitions_1 = project_b .update(cx_b, |p, cx| p.definition(&buffer_b, 23, cx)) @@ -2671,12 +2673,14 @@ mod tests { // Try getting more definitions for the same buffer, ensuring the buffer gets reused from // the previous call to `definition`. - fake_language_server.handle_request::(|_, _| { - Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( - lsp::Url::from_file_path("/root-2/b.rs").unwrap(), - lsp::Range::new(lsp::Position::new(1, 6), lsp::Position::new(1, 11)), - ))) - }); + fake_language_server.handle_request::( + |_, _| async move { + Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( + lsp::Url::from_file_path("/root-2/b.rs").unwrap(), + lsp::Range::new(lsp::Position::new(1, 6), lsp::Position::new(1, 11)), + ))) + }, + ); let definitions_2 = project_b .update(cx_b, |p, cx| p.definition(&buffer_b, 33, cx)) @@ -2783,26 +2787,37 @@ mod tests { // Request references to a symbol as the guest. let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|params, _| { - assert_eq!( - params.text_document_position.text_document.uri.as_str(), - "file:///root-1/one.rs" - ); - Some(vec![ - lsp::Location { - uri: lsp::Url::from_file_path("/root-1/two.rs").unwrap(), - range: lsp::Range::new(lsp::Position::new(0, 24), lsp::Position::new(0, 27)), - }, - lsp::Location { - uri: lsp::Url::from_file_path("/root-1/two.rs").unwrap(), - range: lsp::Range::new(lsp::Position::new(0, 35), lsp::Position::new(0, 38)), - }, - lsp::Location { - uri: lsp::Url::from_file_path("/root-2/three.rs").unwrap(), - range: lsp::Range::new(lsp::Position::new(0, 37), lsp::Position::new(0, 40)), - }, - ]) - }); + fake_language_server.handle_request::( + |params, _| async move { + assert_eq!( + params.text_document_position.text_document.uri.as_str(), + "file:///root-1/one.rs" + ); + Some(vec![ + lsp::Location { + uri: lsp::Url::from_file_path("/root-1/two.rs").unwrap(), + range: lsp::Range::new( + lsp::Position::new(0, 24), + lsp::Position::new(0, 27), + ), + }, + lsp::Location { + uri: lsp::Url::from_file_path("/root-1/two.rs").unwrap(), + range: lsp::Range::new( + lsp::Position::new(0, 35), + lsp::Position::new(0, 38), + ), + }, + lsp::Location { + uri: lsp::Url::from_file_path("/root-2/three.rs").unwrap(), + range: lsp::Range::new( + lsp::Position::new(0, 37), + lsp::Position::new(0, 40), + ), + }, + ]) + }, + ); let references = project_b .update(cx_b, |p, cx| p.references(&buffer_b, 7, cx)) @@ -3012,8 +3027,8 @@ mod tests { // Request document highlights as the guest. let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::( - |params, _| { + fake_language_server.handle_request::( + |params, _| async move { assert_eq!( params .text_document_position_params @@ -3158,20 +3173,22 @@ mod tests { .unwrap(); let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|_, _| { - #[allow(deprecated)] - Some(vec![lsp::SymbolInformation { - name: "TWO".into(), - location: lsp::Location { - uri: lsp::Url::from_file_path("/code/crate-2/two.rs").unwrap(), - range: lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), - }, - kind: lsp::SymbolKind::CONSTANT, - tags: None, - container_name: None, - deprecated: None, - }]) - }); + fake_language_server.handle_request::( + |_, _| async move { + #[allow(deprecated)] + Some(vec![lsp::SymbolInformation { + name: "TWO".into(), + location: lsp::Location { + uri: lsp::Url::from_file_path("/code/crate-2/two.rs").unwrap(), + range: lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), + }, + kind: lsp::SymbolKind::CONSTANT, + tags: None, + container_name: None, + deprecated: None, + }]) + }, + ); // Request the definition of a symbol as the guest. let symbols = project_b @@ -3289,12 +3306,14 @@ mod tests { .unwrap(); let mut fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|_, _| { - Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( - lsp::Url::from_file_path("/root/b.rs").unwrap(), - lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), - ))) - }); + fake_language_server.handle_request::( + |_, _| async move { + Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location::new( + lsp::Url::from_file_path("/root/b.rs").unwrap(), + lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 9)), + ))) + }, + ); let definitions; let buffer_b2; @@ -3402,7 +3421,7 @@ mod tests { let mut fake_language_server = fake_language_servers.next().await.unwrap(); fake_language_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!( params.text_document.uri, lsp::Url::from_file_path("/a/main.rs").unwrap(), @@ -3421,7 +3440,7 @@ mod tests { }); fake_language_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!( params.text_document.uri, lsp::Url::from_file_path("/a/main.rs").unwrap(), @@ -3492,41 +3511,43 @@ mod tests { Editor::confirm_code_action(workspace, &ConfirmCodeAction(Some(0)), cx) }) .unwrap(); - fake_language_server.handle_request::(|_, _| { - lsp::CodeAction { - title: "Inline into all callers".to_string(), - edit: Some(lsp::WorkspaceEdit { - changes: Some( - [ - ( - lsp::Url::from_file_path("/a/main.rs").unwrap(), - vec![lsp::TextEdit::new( - lsp::Range::new( - lsp::Position::new(1, 22), - lsp::Position::new(1, 34), - ), - "4".to_string(), - )], - ), - ( - lsp::Url::from_file_path("/a/other.rs").unwrap(), - vec![lsp::TextEdit::new( - lsp::Range::new( - lsp::Position::new(0, 0), - lsp::Position::new(0, 27), - ), - "".to_string(), - )], - ), - ] - .into_iter() - .collect(), - ), + fake_language_server.handle_request::( + |_, _| async move { + lsp::CodeAction { + title: "Inline into all callers".to_string(), + edit: Some(lsp::WorkspaceEdit { + changes: Some( + [ + ( + lsp::Url::from_file_path("/a/main.rs").unwrap(), + vec![lsp::TextEdit::new( + lsp::Range::new( + lsp::Position::new(1, 22), + lsp::Position::new(1, 34), + ), + "4".to_string(), + )], + ), + ( + lsp::Url::from_file_path("/a/other.rs").unwrap(), + vec![lsp::TextEdit::new( + lsp::Range::new( + lsp::Position::new(0, 0), + lsp::Position::new(0, 27), + ), + "".to_string(), + )], + ), + ] + .into_iter() + .collect(), + ), + ..Default::default() + }), ..Default::default() - }), - ..Default::default() - } - }); + } + }, + ); // After the action is confirmed, an editor containing both modified files is opened. confirm_action.await.unwrap(); @@ -3642,7 +3663,7 @@ mod tests { }); fake_language_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!(params.text_document.uri.as_str(), "file:///dir/one.rs"); assert_eq!(params.position, lsp::Position::new(0, 7)); Some(lsp::PrepareRenameResponse::Range(lsp::Range::new( @@ -3672,7 +3693,7 @@ mod tests { Editor::confirm_rename(workspace, &ConfirmRename, cx).unwrap() }); fake_language_server - .handle_request::(|params, _| { + .handle_request::(|params, _| async move { assert_eq!( params.text_document_position.text_document.uri.as_str(), "file:///dir/one.rs" @@ -5320,30 +5341,34 @@ mod tests { let files = files.clone(); let project = project.downgrade(); move |fake_server| { - fake_server.handle_request::(|_, _| { - Some(lsp::CompletionResponse::Array(vec![lsp::CompletionItem { - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: lsp::Range::new( - lsp::Position::new(0, 0), - lsp::Position::new(0, 0), - ), - new_text: "the-new-text".to_string(), - })), - ..Default::default() - }])) - }); - - fake_server.handle_request::(|_, _| { - Some(vec![lsp::CodeActionOrCommand::CodeAction( - lsp::CodeAction { - title: "the-code-action".to_string(), + fake_server.handle_request::( + |_, _| async move { + Some(lsp::CompletionResponse::Array(vec![lsp::CompletionItem { + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range::new( + lsp::Position::new(0, 0), + lsp::Position::new(0, 0), + ), + new_text: "the-new-text".to_string(), + })), ..Default::default() - }, - )]) - }); + }])) + }, + ); + + fake_server.handle_request::( + |_, _| async move { + Some(vec![lsp::CodeActionOrCommand::CodeAction( + lsp::CodeAction { + title: "the-code-action".to_string(), + ..Default::default() + }, + )]) + }, + ); - fake_server.handle_request::( - |params, _| { + fake_server.handle_request::( + |params, _| async move { Some(lsp::PrepareRenameResponse::Range(lsp::Range::new( params.position, params.position, @@ -5351,34 +5376,38 @@ mod tests { }, ); - fake_server.handle_request::({ + fake_server.handle_request::({ let files = files.clone(); let rng = rng.clone(); move |_, _| { - let files = files.lock(); - let mut rng = rng.lock(); - let count = rng.gen_range::(1..3); - let files = (0..count) - .map(|_| files.choose(&mut *rng).unwrap()) - .collect::>(); - log::info!("LSP: Returning definitions in files {:?}", &files); - Some(lsp::GotoDefinitionResponse::Array( - files - .into_iter() - .map(|file| lsp::Location { - uri: lsp::Url::from_file_path(file).unwrap(), - range: Default::default(), - }) - .collect(), - )) + let files = files.clone(); + let rng = rng.clone(); + async move { + let files = files.lock(); + let mut rng = rng.lock(); + let count = rng.gen_range::(1..3); + let files = (0..count) + .map(|_| files.choose(&mut *rng).unwrap()) + .collect::>(); + log::info!("LSP: Returning definitions in files {:?}", &files); + Some(lsp::GotoDefinitionResponse::Array( + files + .into_iter() + .map(|file| lsp::Location { + uri: lsp::Url::from_file_path(file).unwrap(), + range: Default::default(), + }) + .collect(), + )) + } } }); - fake_server.handle_request::({ + fake_server.handle_request::({ let rng = rng.clone(); let project = project.clone(); move |params, mut cx| { - if let Some(project) = project.upgrade(&cx) { + let highlights = if let Some(project) = project.upgrade(&cx) { project.update(&mut cx, |project, cx| { let path = params .text_document_position_params @@ -5415,7 +5444,8 @@ mod tests { }) } else { None - } + }; + async move { highlights } } }); } From 26aa13842972db719b01fb7b29d2357eeeb1a9f2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 10:57:17 +0200 Subject: [PATCH 2/8] Fire fake timers waking up at the same time as the current clock --- crates/gpui/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 2089b954fb83376237d692993f731e9ab8f2fb31..2c3a8df8a1468a127589bef5ea8f12ec427b8e28 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -337,7 +337,7 @@ impl Deterministic { if let Some((_, wakeup_time, _)) = state.pending_timers.first() { let wakeup_time = *wakeup_time; - if wakeup_time < new_now { + if wakeup_time <= new_now { let timer_count = state .pending_timers .iter() From 2c78c830eb6a7834f0c98e57732589cc92bd6cc3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 10:58:40 +0200 Subject: [PATCH 3/8] Skip formatting during save if it takes too long --- Cargo.lock | 1 + crates/editor/Cargo.toml | 1 + crates/editor/src/editor.rs | 90 ++++++++++++++++++++++++++++++++++++- crates/editor/src/items.rs | 19 ++++++-- 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 38ffa70955b57bdef1c42144726845686062ffb3..9c3692396461c298a0c73160fb177ee8f8c45850 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1617,6 +1617,7 @@ dependencies = [ "collections", "ctor", "env_logger", + "futures", "fuzzy", "gpui", "itertools", diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index 02069fb6108dc4f0a54254594b2d53e1c2032311..77e169b91b2e03648402687722b29ea045c7822b 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -35,6 +35,7 @@ util = { path = "../util" } workspace = { path = "../workspace" } aho-corasick = "0.7" anyhow = "1.0" +futures = "0.3" itertools = "0.10" lazy_static = "1.4" log = "0.4" diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b269ce3cd5d43d26a03b4177fcffda4f47c9b6f3..7c74d36ecb038e5a6715c8eade5b756a45d48037 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6291,7 +6291,7 @@ mod tests { use text::Point; use unindent::Unindent; use util::test::{marked_text_by, sample_text}; - use workspace::FollowableItem; + use workspace::{FollowableItem, ItemHandle}; #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { @@ -8669,6 +8669,94 @@ mod tests { }); } + #[gpui::test] + async fn test_format_during_save(cx: &mut gpui::TestAppContext) { + cx.foreground().forbid_parking(); + cx.update(populate_settings); + + let (mut language_server_config, mut fake_servers) = LanguageServerConfig::fake(); + language_server_config.set_fake_capabilities(lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }); + let language = Arc::new(Language::new( + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".to_string()], + language_server: Some(language_server_config), + ..Default::default() + }, + Some(tree_sitter_rust::language()), + )); + + let fs = FakeFs::new(cx.background().clone()); + fs.insert_file("/file.rs", Default::default()).await; + + let project = Project::test(fs, cx); + project.update(cx, |project, _| project.languages().add(language)); + + let worktree_id = project + .update(cx, |project, cx| { + project.find_or_create_local_worktree("/file.rs", true, cx) + }) + .await + .unwrap() + .0 + .read_with(cx, |tree, _| tree.id()); + let buffer = project + .update(cx, |project, cx| project.open_buffer((worktree_id, ""), cx)) + .await + .unwrap(); + let mut 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)); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let save = cx.update(|cx| editor.save(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() + ); + Some(vec![lsp::TextEdit::new( + lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(1, 0)), + ", ".to_string(), + )]) + }) + .next() + .await; + save.await.unwrap(); + assert_eq!( + editor.read_with(cx, |editor, cx| editor.text(cx)), + "one, two\nthree\n" + ); + assert!(!cx.read(|cx| editor.is_dirty(cx))); + + editor.update(cx, |editor, cx| editor.set_text("one\ntwo\nthree\n", cx)); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + // Ensure we can still save even 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 save = cx.update(|cx| editor.save(project.clone(), cx)); + cx.foreground().advance_clock(items::FORMAT_TIMEOUT); + save.await.unwrap(); + assert_eq!( + editor.read_with(cx, |editor, cx| editor.text(cx)), + "one\ntwo\nthree\n" + ); + assert!(!cx.read(|cx| editor.is_dirty(cx))); + } + #[gpui::test] async fn test_completion(cx: &mut gpui::TestAppContext) { cx.update(populate_settings); diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index f10956c125e3d3cc9bc0eab86432a22942a884af..79b25f8f60fa2ebe9e9832aac99b68c91b775f6b 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1,5 +1,6 @@ use crate::{Anchor, Autoscroll, Editor, Event, ExcerptId, NavigationData, ToOffset, ToPoint as _}; use anyhow::{anyhow, Result}; +use futures::FutureExt; use gpui::{ elements::*, geometry::vector::vec2f, AppContext, Entity, ModelHandle, MutableAppContext, RenderContext, Subscription, Task, View, ViewContext, ViewHandle, @@ -7,13 +8,15 @@ use gpui::{ use language::{Bias, Buffer, Diagnostic, File as _, SelectionGoal}; use project::{File, Project, ProjectEntryId, ProjectPath}; use rpc::proto::{self, update_view}; -use std::{fmt::Write, path::PathBuf}; +use std::{fmt::Write, path::PathBuf, time::Duration}; use text::{Point, Selection}; -use util::ResultExt; +use util::TryFutureExt; use workspace::{ FollowableItem, Item, ItemHandle, ItemNavHistory, ProjectItem, Settings, StatusItemView, }; +pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); + impl FollowableItem for Editor { fn from_state_proto( pane: ViewHandle, @@ -317,9 +320,17 @@ impl Item for Editor { ) -> Task> { let buffer = self.buffer().clone(); let buffers = buffer.read(cx).all_buffers(); - let transaction = project.update(cx, |project, cx| project.format(buffers, true, cx)); + 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 { - let transaction = transaction.await.log_err(); + let transaction = futures::select_biased! { + _ = timeout => { + log::warn!("timed out waiting for formatting"); + None + } + transaction = format.log_err().fuse() => transaction, + }; + this.update(&mut cx, |editor, cx| { editor.request_autoscroll(Autoscroll::Fit, cx) }); From 03752f913d194a4154c23682b125a34811f96c81 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 11:05:55 +0200 Subject: [PATCH 4/8] Fix warnings --- crates/lsp/src/lsp.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 6e729fc62bfe2f0bb35307ed460846f08325d85c..fad49d2424017eea8f66308d2e061b02c9a41ffe 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -1,8 +1,6 @@ use anyhow::{anyhow, Context, Result}; use collections::HashMap; -use futures::{ - channel::oneshot, future::BoxFuture, io::BufWriter, AsyncRead, AsyncWrite, FutureExt, -}; +use futures::{channel::oneshot, io::BufWriter, AsyncRead, AsyncWrite}; use gpui::{executor, Task}; use parking_lot::{Mutex, RwLock}; use postage::{barrier, prelude::Stream}; @@ -560,7 +558,11 @@ type FakeLanguageServerHandlers = Arc< &'static str, Box< dyn Send - + FnMut(usize, &[u8], gpui::AsyncAppContext) -> BoxFuture<'static, Vec>, + + FnMut( + usize, + &[u8], + gpui::AsyncAppContext, + ) -> futures::future::BoxFuture<'static, Vec>, >, >, >, @@ -724,6 +726,8 @@ impl FakeLanguageServer { F: 'static + Send + FnMut(T::Params, gpui::AsyncAppContext) -> Fut, Fut: 'static + Send + Future, { + use futures::FutureExt as _; + let (responded_tx, responded_rx) = futures::channel::mpsc::unbounded(); self.handlers.lock().insert( T::METHOD, From a2c4205c5c3640e6f3ebdb889460b3064bf64bde Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 11:17:19 +0200 Subject: [PATCH 5/8] Make indent and outdent explicit actions and unify `tab`bing logic --- crates/editor/src/editor.rs | 49 +++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f9327ee2519d0521fd6fb05b812411b81c9abc3a..7cb50167b698d305326264491f2f6f60973e4cf8 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -68,7 +68,8 @@ action!(Backspace); action!(Delete); action!(Input, String); action!(Newline); -action!(Tab); +action!(Tab, Direction); +action!(Indent); action!(Outdent); action!(DeleteLine); action!(DeleteToPreviousWordStart); @@ -171,13 +172,13 @@ pub fn init(cx: &mut MutableAppContext) { Some("Editor && showing_code_actions"), ), Binding::new("enter", ConfirmRename, Some("Editor && renaming")), - Binding::new("tab", Tab, Some("Editor")), + Binding::new("tab", Tab(Direction::Next), Some("Editor")), + Binding::new("shift-tab", Tab(Direction::Prev), Some("Editor")), Binding::new( "tab", ConfirmCompletion(None), Some("Editor && showing_completions"), ), - Binding::new("shift-tab", Outdent, Some("Editor")), Binding::new("ctrl-shift-K", DeleteLine, Some("Editor")), Binding::new("alt-backspace", DeleteToPreviousWordStart, Some("Editor")), Binding::new("alt-h", DeleteToPreviousWordStart, Some("Editor")), @@ -304,7 +305,8 @@ pub fn init(cx: &mut MutableAppContext) { cx.add_action(Editor::newline); cx.add_action(Editor::backspace); cx.add_action(Editor::delete); - cx.add_action(Editor::tab); + cx.add_action(Editor::handle_tab); + cx.add_action(Editor::indent); cx.add_action(Editor::outdent); cx.add_action(Editor::delete_line); cx.add_action(Editor::delete_to_previous_word_start); @@ -2861,18 +2863,34 @@ impl Editor { }); } - pub fn tab(&mut self, _: &Tab, cx: &mut ViewContext) { - if self.move_to_next_snippet_tabstop(cx) { - return; + pub fn handle_tab(&mut self, &Tab(direction): &Tab, cx: &mut ViewContext) { + match direction { + Direction::Prev => { + if !self.snippet_stack.is_empty() { + self.move_to_prev_snippet_tabstop(cx); + return; + } + + self.outdent(&Outdent, cx); + } + Direction::Next => { + if self.move_to_next_snippet_tabstop(cx) { + return; + } + + self.tab(false, cx); + } } + } + pub fn tab(&mut self, force_indentation: bool, cx: &mut ViewContext) { let tab_size = cx.global::().tab_size; let mut selections = self.local_selections::(cx); self.transact(cx, |this, cx| { let mut last_indent = None; this.buffer.update(cx, |buffer, cx| { for selection in &mut selections { - if selection.is_empty() { + if selection.is_empty() && !force_indentation { let char_column = buffer .read(cx) .text_for_range(Point::new(selection.start.row, 0)..selection.start) @@ -2938,12 +2956,11 @@ impl Editor { }); } - pub fn outdent(&mut self, _: &Outdent, cx: &mut ViewContext) { - if !self.snippet_stack.is_empty() { - self.move_to_prev_snippet_tabstop(cx); - return; - } + pub fn indent(&mut self, _: &Indent, cx: &mut ViewContext) { + self.tab(true, cx); + } + pub fn outdent(&mut self, _: &Outdent, cx: &mut ViewContext) { let tab_size = cx.global::().tab_size; let selections = self.local_selections::(cx); let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); @@ -7458,7 +7475,7 @@ mod tests { ); // indent from mid-tabstop to full tabstop - view.tab(&Tab, cx); + view.tab(false, cx); assert_eq!(view.text(cx), " one two\nthree\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7483,7 +7500,7 @@ mod tests { view.select_display_ranges(&[DisplayPoint::new(1, 1)..DisplayPoint::new(2, 0)], cx); // indent and outdent affect only the preceding line - view.tab(&Tab, cx); + view.tab(false, cx); assert_eq!(view.text(cx), "one two\n three\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7498,7 +7515,7 @@ mod tests { // Ensure that indenting/outdenting works when the cursor is at column 0. view.select_display_ranges(&[DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0)], cx); - view.tab(&Tab, cx); + view.tab(false, cx); assert_eq!(view.text(cx), "one two\n three\n four"); assert_eq!( view.selected_display_ranges(cx), From ac88003c19db7b90636990982f7748e62236fecd Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 11:34:46 +0200 Subject: [PATCH 6/8] Bind `Outdent` and `Indent` respectively to `cmd-[` and `cmd-]` --- crates/editor/src/editor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 7cb50167b698d305326264491f2f6f60973e4cf8..6c0b43f39357398e96877efdc2860c1ceb1b4c55 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -179,6 +179,8 @@ pub fn init(cx: &mut MutableAppContext) { ConfirmCompletion(None), Some("Editor && showing_completions"), ), + Binding::new("cmd-[", Outdent, Some("Editor")), + Binding::new("cmd-]", Indent, Some("Editor")), Binding::new("ctrl-shift-K", DeleteLine, Some("Editor")), Binding::new("alt-backspace", DeleteToPreviousWordStart, Some("Editor")), Binding::new("alt-h", DeleteToPreviousWordStart, Some("Editor")), From f69bd0e327d785d7bab8e723660c4c739e465798 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 11:52:13 +0200 Subject: [PATCH 7/8] Snap icon sprites to pixel grid This should resolve some rendering artifacts potentially caused by floating point errors when sampling the texture. It should also lead to crisper images when icons are rendered midway through a pixel. --- crates/gpui/src/platform/mac/renderer.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/platform/mac/renderer.rs b/crates/gpui/src/platform/mac/renderer.rs index 07d425af3ebd4235a178ca479a21052d0d71b043..873586b61e35b6e1c5eebd0728213feb0bc1ebc0 100644 --- a/crates/gpui/src/platform/mac/renderer.rs +++ b/crates/gpui/src/platform/mac/renderer.rs @@ -561,9 +561,10 @@ impl Renderer { } for icon in icons { - let origin = icon.bounds.origin() * scale_factor; - let target_size = icon.bounds.size() * scale_factor; - let source_size = (target_size * 2.).ceil().to_i32(); + // Snap sprite to pixel grid. + let origin = (icon.bounds.origin() * scale_factor).floor(); + let target_size = (icon.bounds.size() * scale_factor).ceil(); + let source_size = (target_size * 2.).to_i32(); let sprite = self.sprite_cache From 2a1fed1387d1c675a78cff5cd86a1293cfa1c420 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 28 Mar 2022 16:36:12 +0200 Subject: [PATCH 8/8] Insert tabs instead of indenting only when all selections are empty Co-Authored-By: Nathan Sobo --- crates/editor/src/editor.rs | 136 +++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 65 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6c0b43f39357398e96877efdc2860c1ceb1b4c55..ca94c6923f5d952db183a990e66adbd3cf232495 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -307,7 +307,7 @@ pub fn init(cx: &mut MutableAppContext) { cx.add_action(Editor::newline); cx.add_action(Editor::backspace); cx.add_action(Editor::delete); - cx.add_action(Editor::handle_tab); + cx.add_action(Editor::tab); cx.add_action(Editor::indent); cx.add_action(Editor::outdent); cx.add_action(Editor::delete_line); @@ -2865,7 +2865,7 @@ impl Editor { }); } - pub fn handle_tab(&mut self, &Tab(direction): &Tab, cx: &mut ViewContext) { + pub fn tab(&mut self, &Tab(direction): &Tab, cx: &mut ViewContext) { match direction { Direction::Prev => { if !self.snippet_stack.is_empty() { @@ -2880,76 +2880,86 @@ impl Editor { return; } - self.tab(false, cx); + let tab_size = cx.global::().tab_size; + let mut selections = self.local_selections::(cx); + if selections.iter().all(|s| s.is_empty()) { + self.transact(cx, |this, cx| { + this.buffer.update(cx, |buffer, cx| { + for selection in &mut selections { + let char_column = buffer + .read(cx) + .text_for_range( + Point::new(selection.start.row, 0)..selection.start, + ) + .flat_map(str::chars) + .count(); + let chars_to_next_tab_stop = tab_size - (char_column % tab_size); + buffer.edit( + [selection.start..selection.start], + " ".repeat(chars_to_next_tab_stop), + cx, + ); + selection.start.column += chars_to_next_tab_stop as u32; + selection.end = selection.start; + } + }); + this.update_selections(selections, Some(Autoscroll::Fit), cx); + }); + } else { + self.indent(&Indent, cx); + } } } } - pub fn tab(&mut self, force_indentation: bool, cx: &mut ViewContext) { + pub fn indent(&mut self, _: &Indent, cx: &mut ViewContext) { let tab_size = cx.global::().tab_size; let mut selections = self.local_selections::(cx); self.transact(cx, |this, cx| { let mut last_indent = None; this.buffer.update(cx, |buffer, cx| { for selection in &mut selections { - if selection.is_empty() && !force_indentation { - let char_column = buffer - .read(cx) - .text_for_range(Point::new(selection.start.row, 0)..selection.start) - .flat_map(str::chars) - .count(); - let chars_to_next_tab_stop = tab_size - (char_column % tab_size); + let mut start_row = selection.start.row; + let mut end_row = selection.end.row + 1; + + // If a selection ends at the beginning of a line, don't indent + // that last line. + if selection.end.column == 0 { + end_row -= 1; + } + + // Avoid re-indenting a row that has already been indented by a + // previous selection, but still update this selection's column + // to reflect that indentation. + if let Some((last_indent_row, last_indent_len)) = last_indent { + if last_indent_row == selection.start.row { + selection.start.column += last_indent_len; + start_row += 1; + } + if last_indent_row == selection.end.row { + selection.end.column += last_indent_len; + } + } + + for row in start_row..end_row { + let indent_column = buffer.read(cx).indent_column_for_line(row) as usize; + let columns_to_next_tab_stop = tab_size - (indent_column % tab_size); + let row_start = Point::new(row, 0); buffer.edit( - [selection.start..selection.start], - " ".repeat(chars_to_next_tab_stop), + [row_start..row_start], + " ".repeat(columns_to_next_tab_stop), cx, ); - selection.start.column += chars_to_next_tab_stop as u32; - selection.end = selection.start; - } else { - let mut start_row = selection.start.row; - let mut end_row = selection.end.row + 1; - // If a selection ends at the beginning of a line, don't indent - // that last line. - if selection.end.column == 0 { - end_row -= 1; + // Update this selection's endpoints to reflect the indentation. + if row == selection.start.row { + selection.start.column += columns_to_next_tab_stop as u32; } - - // Avoid re-indenting a row that has already been indented by a - // previous selection, but still update this selection's column - // to reflect that indentation. - if let Some((last_indent_row, last_indent_len)) = last_indent { - if last_indent_row == selection.start.row { - selection.start.column += last_indent_len; - start_row += 1; - } - if last_indent_row == selection.end.row { - selection.end.column += last_indent_len; - } + if row == selection.end.row { + selection.end.column += columns_to_next_tab_stop as u32; } - for row in start_row..end_row { - let indent_column = - buffer.read(cx).indent_column_for_line(row) as usize; - let columns_to_next_tab_stop = tab_size - (indent_column % tab_size); - let row_start = Point::new(row, 0); - buffer.edit( - [row_start..row_start], - " ".repeat(columns_to_next_tab_stop), - cx, - ); - - // Update this selection's endpoints to reflect the indentation. - if row == selection.start.row { - selection.start.column += columns_to_next_tab_stop as u32; - } - if row == selection.end.row { - selection.end.column += columns_to_next_tab_stop as u32; - } - - last_indent = Some((row, columns_to_next_tab_stop as u32)); - } + last_indent = Some((row, columns_to_next_tab_stop as u32)); } } }); @@ -2958,10 +2968,6 @@ impl Editor { }); } - pub fn indent(&mut self, _: &Indent, cx: &mut ViewContext) { - self.tab(true, cx); - } - pub fn outdent(&mut self, _: &Outdent, cx: &mut ViewContext) { let tab_size = cx.global::().tab_size; let selections = self.local_selections::(cx); @@ -7477,7 +7483,7 @@ mod tests { ); // indent from mid-tabstop to full tabstop - view.tab(false, cx); + view.tab(&Tab(Direction::Next), cx); assert_eq!(view.text(cx), " one two\nthree\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7488,7 +7494,7 @@ mod tests { ); // outdent from 1 tabstop to 0 tabstops - view.outdent(&Outdent, cx); + view.tab(&Tab(Direction::Prev), cx); assert_eq!(view.text(cx), "one two\nthree\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7502,13 +7508,13 @@ mod tests { view.select_display_ranges(&[DisplayPoint::new(1, 1)..DisplayPoint::new(2, 0)], cx); // indent and outdent affect only the preceding line - view.tab(false, cx); + view.tab(&Tab(Direction::Next), cx); assert_eq!(view.text(cx), "one two\n three\n four"); assert_eq!( view.selected_display_ranges(cx), &[DisplayPoint::new(1, 5)..DisplayPoint::new(2, 0)] ); - view.outdent(&Outdent, cx); + view.tab(&Tab(Direction::Prev), cx); assert_eq!(view.text(cx), "one two\nthree\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7517,7 +7523,7 @@ mod tests { // Ensure that indenting/outdenting works when the cursor is at column 0. view.select_display_ranges(&[DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0)], cx); - view.tab(false, cx); + view.tab(&Tab(Direction::Next), cx); assert_eq!(view.text(cx), "one two\n three\n four"); assert_eq!( view.selected_display_ranges(cx), @@ -7525,7 +7531,7 @@ mod tests { ); view.select_display_ranges(&[DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0)], cx); - view.outdent(&Outdent, cx); + view.tab(&Tab(Direction::Prev), cx); assert_eq!(view.text(cx), "one two\nthree\n four"); assert_eq!( view.selected_display_ranges(cx),