From f6d7b3d2e8fb68077dbb575314b4bb8ee81116e1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 19 May 2023 13:18:50 +0300 Subject: [PATCH 01/10] Send and handle OnTypeFormatting LSP request --- crates/editor/src/editor.rs | 24 +++++++ crates/project/src/lsp_command.rs | 103 +++++++++++++++++++++++++++++- crates/project/src/project.rs | 36 ++++++++++- 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 41fd03bf7f..c6709b51fd 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2122,6 +2122,13 @@ impl Editor { let had_active_copilot_suggestion = this.has_active_copilot_suggestion(cx); this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); + if text.len() == 1 { + let input_char = text.chars().next().expect("single char input"); + if let Some(on_type_format_task) = this.trigger_on_type_format(input_char, cx) { + on_type_format_task.detach_and_log_err(cx); + } + } + if had_active_copilot_suggestion { this.refresh_copilot_suggestions(true, cx); if !this.has_active_copilot_suggestion(cx) { @@ -2500,6 +2507,23 @@ impl Editor { } } + fn trigger_on_type_format( + &self, + input: char, + cx: &mut ViewContext, + ) -> Option>> { + let project = self.project.as_ref()?; + let position = self.selections.newest_anchor().head(); + let (buffer, buffer_position) = self + .buffer + .read(cx) + .text_anchor_for_position(position.clone(), cx)?; + + Some(project.update(cx, |project, cx| { + project.on_type_format(buffer.clone(), buffer_position, input, cx) + })) + } + fn show_completions(&mut self, _: &ShowCompletions, cx: &mut ViewContext) { if self.pending_rename.is_some() { return; diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index ddae9b59ae..523011c76a 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -2,7 +2,7 @@ use crate::{ DocumentHighlight, Hover, HoverBlock, HoverBlockKind, Location, LocationLink, Project, ProjectTransaction, }; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; use client::proto::{self, PeerId}; use fs::LineEnding; @@ -109,6 +109,12 @@ pub(crate) struct GetCodeActions { pub range: Range, } +pub(crate) struct OnTypeFormatting { + pub position: PointUtf16, + pub new_char: char, + // TODO kb formatting options? +} + #[async_trait(?Send)] impl LspCommand for PrepareRename { type Response = Option>; @@ -1596,3 +1602,98 @@ impl LspCommand for GetCodeActions { message.buffer_id } } + +#[async_trait(?Send)] +impl LspCommand for OnTypeFormatting { + type Response = Vec<(Range, String)>; + type LspRequest = lsp::request::OnTypeFormatting; + type ProtoRequest = proto::PerformRename; + + fn check_capabilities(&self, server_capabilities: &lsp::ServerCapabilities) -> bool { + let Some(on_type_formatting_options) = &server_capabilities.document_on_type_formatting_provider else { return false }; + on_type_formatting_options + .first_trigger_character + .contains(self.new_char) + || on_type_formatting_options + .more_trigger_character + .iter() + .flatten() + .any(|chars| chars.contains(self.new_char)) + } + + fn to_lsp( + &self, + path: &Path, + _: &Buffer, + _: &Arc, + _: &AppContext, + ) -> lsp::DocumentOnTypeFormattingParams { + lsp::DocumentOnTypeFormattingParams { + text_document_position: lsp::TextDocumentPositionParams::new( + lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path(path).unwrap()), + point_to_lsp(self.position), + ), + ch: self.new_char.to_string(), + // TODO kb pass current editor ones + options: lsp::FormattingOptions::default(), + } + } + + async fn response_from_lsp( + self, + message: Option>, + project: ModelHandle, + buffer: ModelHandle, + server_id: LanguageServerId, + mut cx: AsyncAppContext, + ) -> Result, String)>> { + cx.update(|cx| { + project.update(cx, |project, cx| { + project.edits_from_lsp(&buffer, message.into_iter().flatten(), server_id, None, cx) + }) + }) + .await + .context("LSP edits conversion") + } + + fn to_proto(&self, project_id: u64, buffer: &Buffer) -> proto::PerformRename { + todo!("TODO kb") + } + + async fn from_proto( + message: proto::PerformRename, + _: ModelHandle, + buffer: ModelHandle, + mut cx: AsyncAppContext, + ) -> Result { + todo!("TODO kb") + } + + fn response_to_proto( + response: Vec<(Range, String)>, + project: &mut Project, + peer_id: PeerId, + _: &clock::Global, + cx: &mut AppContext, + ) -> proto::PerformRenameResponse { + // let transaction = project.serialize_project_transaction_for_peer(response, peer_id, cx); + // proto::PerformRenameResponse { + // transaction: Some(transaction), + // } + todo!("TODO kb") + } + + async fn response_from_proto( + self, + message: proto::PerformRenameResponse, + project: ModelHandle, + _: ModelHandle, + mut cx: AsyncAppContext, + ) -> Result, String)>> { + todo!("TODO kb") + } + + fn buffer_id_from_proto(message: &proto::PerformRename) -> u64 { + message.buffer_id + } +} diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index dd53c30d14..d8d29cbadd 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4209,6 +4209,40 @@ impl Project { ) } + pub fn on_type_format( + &self, + buffer: ModelHandle, + position: T, + input: char, + cx: &mut ModelContext, + ) -> Task> { + let position = position.to_point_utf16(buffer.read(cx)); + let edits_task = self.request_lsp( + buffer.clone(), + OnTypeFormatting { + position, + new_char: input, + }, + cx, + ); + + cx.spawn(|_project, mut cx| async move { + let edits = edits_task + .await + .context("requesting OnTypeFormatting edits for char '{new_char}'")?; + + if !edits.is_empty() { + cx.update(|cx| { + buffer.update(cx, |buffer, cx| { + buffer.edit(edits, None, cx); + }); + }); + } + + Ok(()) + }) + } + #[allow(clippy::type_complexity)] pub fn search( &self, @@ -6349,7 +6383,7 @@ impl Project { } #[allow(clippy::type_complexity)] - fn edits_from_lsp( + pub fn edits_from_lsp( &mut self, buffer: &ModelHandle, lsp_edits: impl 'static + Send + IntoIterator, From 3327e8a6dd964eb32df0537679af127b7f0106e8 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 May 2023 17:11:23 +0300 Subject: [PATCH 02/10] Support remote sessions --- crates/project/src/lsp_command.rs | 96 +++++++++++++++++++++++-------- crates/project/src/project.rs | 2 +- crates/rpc/proto/zed.proto | 22 +++++++ crates/rpc/src/proto.rs | 4 ++ 4 files changed, 99 insertions(+), 25 deletions(-) diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 523011c76a..01aba89aa7 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -111,7 +111,7 @@ pub(crate) struct GetCodeActions { pub(crate) struct OnTypeFormatting { pub position: PointUtf16, - pub new_char: char, + pub trigger: String, // TODO kb formatting options? } @@ -1607,18 +1607,18 @@ impl LspCommand for GetCodeActions { impl LspCommand for OnTypeFormatting { type Response = Vec<(Range, String)>; type LspRequest = lsp::request::OnTypeFormatting; - type ProtoRequest = proto::PerformRename; + type ProtoRequest = proto::OnTypeFormatting; fn check_capabilities(&self, server_capabilities: &lsp::ServerCapabilities) -> bool { let Some(on_type_formatting_options) = &server_capabilities.document_on_type_formatting_provider else { return false }; on_type_formatting_options .first_trigger_character - .contains(self.new_char) + .contains(&self.trigger) || on_type_formatting_options .more_trigger_character .iter() .flatten() - .any(|chars| chars.contains(self.new_char)) + .any(|chars| chars.contains(&self.trigger)) } fn to_lsp( @@ -1633,7 +1633,7 @@ impl LspCommand for OnTypeFormatting { lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path(path).unwrap()), point_to_lsp(self.position), ), - ch: self.new_char.to_string(), + ch: self.trigger.clone(), // TODO kb pass current editor ones options: lsp::FormattingOptions::default(), } @@ -1656,44 +1656,92 @@ impl LspCommand for OnTypeFormatting { .context("LSP edits conversion") } - fn to_proto(&self, project_id: u64, buffer: &Buffer) -> proto::PerformRename { - todo!("TODO kb") + fn to_proto(&self, project_id: u64, buffer: &Buffer) -> proto::OnTypeFormatting { + proto::OnTypeFormatting { + project_id, + buffer_id: buffer.remote_id(), + position: Some(language::proto::serialize_anchor( + &buffer.anchor_before(self.position), + )), + trigger: self.trigger.clone(), + version: serialize_version(&buffer.version()), + } } async fn from_proto( - message: proto::PerformRename, + message: proto::OnTypeFormatting, _: ModelHandle, buffer: ModelHandle, mut cx: AsyncAppContext, ) -> Result { - todo!("TODO kb") + let position = message + .position + .and_then(deserialize_anchor) + .ok_or_else(|| anyhow!("invalid position"))?; + buffer + .update(&mut cx, |buffer, _| { + buffer.wait_for_version(deserialize_version(&message.version)) + }) + .await?; + + Ok(Self { + position: buffer.read_with(&cx, |buffer, _| position.to_point_utf16(buffer)), + trigger: message.trigger.clone(), + }) } fn response_to_proto( response: Vec<(Range, String)>, - project: &mut Project, - peer_id: PeerId, - _: &clock::Global, - cx: &mut AppContext, - ) -> proto::PerformRenameResponse { - // let transaction = project.serialize_project_transaction_for_peer(response, peer_id, cx); - // proto::PerformRenameResponse { - // transaction: Some(transaction), - // } - todo!("TODO kb") + _: &mut Project, + _: PeerId, + buffer_version: &clock::Global, + _: &mut AppContext, + ) -> proto::OnTypeFormattingResponse { + proto::OnTypeFormattingResponse { + entries: response + .into_iter() + .map( + |(response_range, new_text)| proto::OnTypeFormattingResponseEntry { + start: Some(language::proto::serialize_anchor(&response_range.start)), + end: Some(language::proto::serialize_anchor(&response_range.end)), + new_text, + }, + ) + .collect(), + version: serialize_version(&buffer_version), + } } async fn response_from_proto( self, - message: proto::PerformRenameResponse, - project: ModelHandle, - _: ModelHandle, + message: proto::OnTypeFormattingResponse, + _: ModelHandle, + buffer: ModelHandle, mut cx: AsyncAppContext, ) -> Result, String)>> { - todo!("TODO kb") + buffer + .update(&mut cx, |buffer, _| { + buffer.wait_for_version(deserialize_version(&message.version)) + }) + .await?; + message + .entries + .into_iter() + .map(|entry| { + let start = entry + .start + .and_then(language::proto::deserialize_anchor) + .ok_or_else(|| anyhow!("invalid start"))?; + let end = entry + .end + .and_then(language::proto::deserialize_anchor) + .ok_or_else(|| anyhow!("invalid end"))?; + Ok((start..end, entry.new_text)) + }) + .collect() } - fn buffer_id_from_proto(message: &proto::PerformRename) -> u64 { + fn buffer_id_from_proto(message: &proto::OnTypeFormatting) -> u64 { message.buffer_id } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d8d29cbadd..1047ca1d52 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4221,7 +4221,7 @@ impl Project { buffer.clone(), OnTypeFormatting { position, - new_char: input, + trigger: input.to_string(), }, cx, ); diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index eca5fda306..fe0d18f422 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -129,6 +129,9 @@ message Envelope { GetPrivateUserInfo get_private_user_info = 105; GetPrivateUserInfoResponse get_private_user_info_response = 106; UpdateDiffBase update_diff_base = 107; + + OnTypeFormatting on_type_formatting = 111; + OnTypeFormattingResponse on_type_formatting_response = 112; } } @@ -670,6 +673,25 @@ message PerformRename { repeated VectorClockEntry version = 5; } +message OnTypeFormatting { + uint64 project_id = 1; + uint64 buffer_id = 2; + Anchor position = 3; + string trigger = 4; + repeated VectorClockEntry version = 5; +} + +message OnTypeFormattingResponse { + repeated OnTypeFormattingResponseEntry entries = 1; + repeated VectorClockEntry version = 2; +} + +message OnTypeFormattingResponseEntry { + Anchor start = 1; + Anchor end = 2; + string new_text = 3; +} + message PerformRenameResponse { ProjectTransaction transaction = 2; } diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index cef4e6867c..07925a0486 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -195,6 +195,8 @@ messages!( (OpenBufferResponse, Background), (PerformRename, Background), (PerformRenameResponse, Background), + (OnTypeFormatting, Background), + (OnTypeFormattingResponse, Background), (Ping, Foreground), (PrepareRename, Background), (PrepareRenameResponse, Background), @@ -279,6 +281,7 @@ request_messages!( (Ping, Ack), (PerformRename, PerformRenameResponse), (PrepareRename, PrepareRenameResponse), + (OnTypeFormatting, OnTypeFormattingResponse), (ReloadBuffers, ReloadBuffersResponse), (RequestContact, Ack), (RemoveContact, Ack), @@ -323,6 +326,7 @@ entity_messages!( OpenBufferByPath, OpenBufferForSymbol, PerformRename, + OnTypeFormatting, PrepareRename, ReloadBuffers, RemoveProjectCollaborator, From b9dabb165e2ea1ee058dd8aebe30d3e6381b9bc6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 May 2023 17:38:05 +0300 Subject: [PATCH 03/10] Use formatting options --- crates/editor/src/editor.rs | 1 + crates/project/src/lsp_command.rs | 35 ++++++++++++++++++++++++++++--- crates/project/src/project.rs | 19 +++++++---------- crates/rpc/proto/zed.proto | 11 +++++++--- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c6709b51fd..20116da34f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2122,6 +2122,7 @@ impl Editor { let had_active_copilot_suggestion = this.has_active_copilot_suggestion(cx); this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); + // When buffer contents is updated and caret is moved, try triggering on type formatting. if text.len() == 1 { let input_char = text.chars().next().expect("single char input"); if let Some(on_type_format_task) = this.trigger_on_type_format(input_char, cx) { diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 01aba89aa7..5eb97f5c95 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -16,6 +16,15 @@ use language::{ use lsp::{DocumentHighlightKind, LanguageServer, LanguageServerId, ServerCapabilities}; use std::{cmp::Reverse, ops::Range, path::Path, sync::Arc}; +pub fn lsp_formatting_options(tab_size: u32) -> lsp::FormattingOptions { + lsp::FormattingOptions { + tab_size, + insert_spaces: true, + insert_final_newline: Some(true), + ..lsp::FormattingOptions::default() + } +} + #[async_trait(?Send)] pub(crate) trait LspCommand: 'static + Sized { type Response: 'static + Default + Send; @@ -112,7 +121,19 @@ pub(crate) struct GetCodeActions { pub(crate) struct OnTypeFormatting { pub position: PointUtf16, pub trigger: String, - // TODO kb formatting options? + pub options: FormattingOptions, +} + +pub(crate) struct FormattingOptions { + tab_size: u32, +} + +impl From for FormattingOptions { + fn from(value: lsp::FormattingOptions) -> Self { + Self { + tab_size: value.tab_size, + } + } } #[async_trait(?Send)] @@ -1634,8 +1655,7 @@ impl LspCommand for OnTypeFormatting { point_to_lsp(self.position), ), ch: self.trigger.clone(), - // TODO kb pass current editor ones - options: lsp::FormattingOptions::default(), + options: lsp_formatting_options(self.options.tab_size), } } @@ -1660,6 +1680,9 @@ impl LspCommand for OnTypeFormatting { proto::OnTypeFormatting { project_id, buffer_id: buffer.remote_id(), + options: Some(proto::FormattingOptions { + tab_size: self.options.tab_size, + }), position: Some(language::proto::serialize_anchor( &buffer.anchor_before(self.position), )), @@ -1687,6 +1710,12 @@ impl LspCommand for OnTypeFormatting { Ok(Self { position: buffer.read_with(&cx, |buffer, _| position.to_point_utf16(buffer)), trigger: message.trigger.clone(), + options: message + .options + .map(|options| options.tab_size) + .map(lsp_formatting_options) + .unwrap_or_default() + .into(), }) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 1047ca1d52..652f1107d1 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3476,12 +3476,7 @@ impl Project { 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() - }, + options: lsp_command::lsp_formatting_options(tab_size.get()), work_done_progress_params: Default::default(), }) .await? @@ -3497,12 +3492,7 @@ impl Project { .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() - }, + options: lsp_command::lsp_formatting_options(tab_size.get()), work_done_progress_params: Default::default(), }) .await? @@ -4216,12 +4206,17 @@ impl Project { input: char, cx: &mut ModelContext, ) -> Task> { + let tab_size = buffer.read_with(cx, |buffer, cx| { + let language_name = buffer.language().map(|language| language.name()); + language_settings(language_name.as_deref(), cx).tab_size + }); let position = position.to_point_utf16(buffer.read(cx)); let edits_task = self.request_lsp( buffer.clone(), OnTypeFormatting { position, trigger: input.to_string(), + options: lsp_command::lsp_formatting_options(tab_size.get()).into(), }, cx, ); diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index fe0d18f422..2ccc0590e5 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -676,9 +676,14 @@ message PerformRename { message OnTypeFormatting { uint64 project_id = 1; uint64 buffer_id = 2; - Anchor position = 3; - string trigger = 4; - repeated VectorClockEntry version = 5; + FormattingOptions options = 3; + Anchor position = 4; + string trigger = 5; + repeated VectorClockEntry version = 6; +} + +message FormattingOptions { + uint32 tab_size = 1; } message OnTypeFormattingResponse { From d1f4b60fa1b639ffc64c7636605aebf37ca67885 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 May 2023 17:50:50 +0300 Subject: [PATCH 04/10] Allow to disable the new feature --- assets/settings/default.json | 3 +++ crates/editor/src/editor.rs | 2 +- crates/editor/src/editor_settings.rs | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 246e28cc8e..23599c8dfb 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -39,6 +39,9 @@ // Whether to pop the completions menu while typing in an editor without // explicitly requesting it. "show_completions_on_input": true, + // Whether to use additional LSP queries to format (and amend) the code after + // every "trigger" symbol input, defined by LSP server capabilities. + "use_on_type_format": true, // Controls whether copilot provides suggestion immediately // or waits for a `copilot::Toggle` "show_copilot_suggestions": true, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 20116da34f..a33988dc5d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2123,7 +2123,7 @@ impl Editor { this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); // When buffer contents is updated and caret is moved, try triggering on type formatting. - if text.len() == 1 { + if settings::get::(cx).use_on_type_format && text.len() == 1 { let input_char = text.chars().next().expect("single char input"); if let Some(on_type_format_task) = this.trigger_on_type_format(input_char, cx) { on_type_format_task.detach_and_log_err(cx); diff --git a/crates/editor/src/editor_settings.rs b/crates/editor/src/editor_settings.rs index 7f01834b16..387d4d2c34 100644 --- a/crates/editor/src/editor_settings.rs +++ b/crates/editor/src/editor_settings.rs @@ -7,6 +7,7 @@ pub struct EditorSettings { pub cursor_blink: bool, pub hover_popover_enabled: bool, pub show_completions_on_input: bool, + pub use_on_type_format: bool, pub scrollbar: Scrollbar, } @@ -30,6 +31,7 @@ pub struct EditorSettingsContent { pub cursor_blink: Option, pub hover_popover_enabled: Option, pub show_completions_on_input: Option, + pub use_on_type_format: Option, pub scrollbar: Option, } From 58a56bdda26743d60b3f22faa9a8ffffcfe95a49 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 May 2023 22:58:40 +0300 Subject: [PATCH 05/10] Always use server formatting settings --- crates/project/src/lsp_command.rs | 16 +++++++--------- crates/rpc/proto/zed.proto | 11 +++-------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 5eb97f5c95..992973182f 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -8,6 +8,7 @@ use client::proto::{self, PeerId}; use fs::LineEnding; use gpui::{AppContext, AsyncAppContext, ModelHandle}; use language::{ + language_settings::language_settings, point_from_lsp, point_to_lsp, proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version}, range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CharKind, CodeAction, @@ -1680,9 +1681,6 @@ impl LspCommand for OnTypeFormatting { proto::OnTypeFormatting { project_id, buffer_id: buffer.remote_id(), - options: Some(proto::FormattingOptions { - tab_size: self.options.tab_size, - }), position: Some(language::proto::serialize_anchor( &buffer.anchor_before(self.position), )), @@ -1707,15 +1705,15 @@ impl LspCommand for OnTypeFormatting { }) .await?; + let tab_size = buffer.read_with(&cx, |buffer, cx| { + let language_name = buffer.language().map(|language| language.name()); + language_settings(language_name.as_deref(), cx).tab_size + }); + Ok(Self { position: buffer.read_with(&cx, |buffer, _| position.to_point_utf16(buffer)), trigger: message.trigger.clone(), - options: message - .options - .map(|options| options.tab_size) - .map(lsp_formatting_options) - .unwrap_or_default() - .into(), + options: lsp_formatting_options(tab_size.get()).into(), }) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 2ccc0590e5..fe0d18f422 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -676,14 +676,9 @@ message PerformRename { message OnTypeFormatting { uint64 project_id = 1; uint64 buffer_id = 2; - FormattingOptions options = 3; - Anchor position = 4; - string trigger = 5; - repeated VectorClockEntry version = 6; -} - -message FormattingOptions { - uint32 tab_size = 1; + Anchor position = 3; + string trigger = 4; + repeated VectorClockEntry version = 5; } message OnTypeFormattingResponse { From eca6d2b597d49730b4eaba209c75e7a076b1b2f3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 May 2023 23:16:59 +0300 Subject: [PATCH 06/10] Process remote format typing also --- crates/collab/src/rpc.rs | 1 + crates/project/src/project.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index ac86f8c171..4c117b613d 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -223,6 +223,7 @@ impl Server { .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) .add_request_handler(forward_project_request::) + .add_request_handler(forward_project_request::) .add_message_handler(create_buffer_for_peer) .add_request_handler(update_buffer) .add_message_handler(update_buffer_file) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 652f1107d1..36b3290121 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -429,6 +429,7 @@ impl Project { client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_lsp_command::); + client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_search_project); client.add_model_request_handler(Self::handle_get_project_symbols); client.add_model_request_handler(Self::handle_open_buffer_for_symbol); From f812151840e575b99a4733449599db25a6e5c5e9 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 24 May 2023 00:11:50 +0300 Subject: [PATCH 07/10] Add integration tests --- crates/collab/src/tests/integration_tests.rs | 218 +++++++++++++++++++ 1 file changed, 218 insertions(+) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 439ee0786a..01be8d08df 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7377,6 +7377,224 @@ async fn test_peers_simultaneously_following_each_other( }); } +#[gpui::test(iterations = 10)] +async fn test_on_input_format_from_host_to_guest( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + + // Set up a fake language server. + 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_language_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_on_type_formatting_provider: Some(lsp::DocumentOnTypeFormattingOptions { + first_trigger_character: ":".to_string(), + more_trigger_character: Some(vec![">".to_string()]), + }), + ..Default::default() + }, + ..Default::default() + })) + .await; + client_a.language_registry.add(Arc::new(language)); + + client_a + .fs + .insert_tree( + "/a", + json!({ + "main.rs": "fn main() { a }", + "other.rs": "// Test file", + }), + ) + .await; + let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + let project_b = client_b.build_remote_project(project_id, cx_b).await; + + // Open a file in an editor as the host. + let buffer_a = project_a + .update(cx_a, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + .await + .unwrap(); + let (window_a, _) = cx_a.add_window(|_| EmptyView); + let editor_a = cx_a.add_view(window_a, |cx| { + Editor::for_buffer(buffer_a, Some(project_a.clone()), cx) + }); + + let fake_language_server = fake_language_servers.next().await.unwrap(); + cx_b.foreground().run_until_parked(); + // Type a on type formatting trigger character as the guest. + editor_a.update(cx_a, |editor, cx| { + editor.change_selections(None, cx, |s| s.select_ranges([13..13])); + editor.handle_input(">", cx); + cx.focus(&editor_a); + }); + + // Receive an OnTypeFormatting request as the host's language server. + // Return some formattings from the host's language server. + cx_b.foreground().start_waiting(); + fake_language_server + .handle_request::(|params, _| async move { + assert_eq!( + params.text_document_position.text_document.uri, + lsp::Url::from_file_path("/a/main.rs").unwrap(), + ); + assert_eq!( + params.text_document_position.position, + lsp::Position::new(0, 14), + ); + + Ok(Some(vec![lsp::TextEdit { + new_text: "~<".to_string(), + range: lsp::Range::new(lsp::Position::new(0, 14), lsp::Position::new(0, 14)), + }])) + }) + .next() + .await + .unwrap(); + cx_b.foreground().finish_waiting(); + + // Open the buffer on the guest and see that the formattings worked + let buffer_b = project_b + .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + .await + .unwrap(); + cx_b.foreground().run_until_parked(); + buffer_b.read_with(cx_b, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a>~< }") + }); +} + +#[gpui::test(iterations = 10)] +async fn test_on_input_format_from_guest_to_host( + deterministic: Arc, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + + // Set up a fake language server. + 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_language_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_on_type_formatting_provider: Some(lsp::DocumentOnTypeFormattingOptions { + first_trigger_character: ":".to_string(), + more_trigger_character: Some(vec![">".to_string()]), + }), + ..Default::default() + }, + ..Default::default() + })) + .await; + client_a.language_registry.add(Arc::new(language)); + + client_a + .fs + .insert_tree( + "/a", + json!({ + "main.rs": "fn main() { a }", + "other.rs": "// Test file", + }), + ) + .await; + let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + let project_b = client_b.build_remote_project(project_id, cx_b).await; + + // Open a file in an editor as the guest. + let buffer_b = project_b + .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + .await + .unwrap(); + let (window_b, _) = cx_b.add_window(|_| EmptyView); + let editor_b = cx_b.add_view(window_b, |cx| { + Editor::for_buffer(buffer_b, Some(project_b.clone()), cx) + }); + + let fake_language_server = fake_language_servers.next().await.unwrap(); + cx_a.foreground().run_until_parked(); + // Type a on type formatting trigger character as the guest. + editor_b.update(cx_b, |editor, cx| { + editor.change_selections(None, cx, |s| s.select_ranges([13..13])); + editor.handle_input(":", cx); + cx.focus(&editor_b); + }); + + // Receive an OnTypeFormatting request as the host's language server. + // Return some formattings from the host's language server. + cx_a.foreground().start_waiting(); + fake_language_server + .handle_request::(|params, _| async move { + assert_eq!( + params.text_document_position.text_document.uri, + lsp::Url::from_file_path("/a/main.rs").unwrap(), + ); + assert_eq!( + params.text_document_position.position, + lsp::Position::new(0, 14), + ); + + Ok(Some(vec![lsp::TextEdit { + new_text: "~:".to_string(), + range: lsp::Range::new(lsp::Position::new(0, 14), lsp::Position::new(0, 14)), + }])) + }) + .next() + .await + .unwrap(); + cx_a.foreground().finish_waiting(); + + // Open the buffer on the host and see that the formattings worked + let buffer_a = project_a + .update(cx_a, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) + .await + .unwrap(); + cx_a.foreground().run_until_parked(); + buffer_a.read_with(cx_a, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a:~: }") + }); +} + #[derive(Debug, Eq, PartialEq)] struct RoomParticipants { remote: Vec, From aa58d0fd776b74e2b2d04afe3699a261f92ce59f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 May 2023 12:35:07 +0300 Subject: [PATCH 08/10] Do not send edits over the wire --- crates/editor/src/editor.rs | 37 ++++-- crates/project/src/lsp_command.rs | 86 +++++++------- crates/project/src/project.rs | 179 ++++++++++++++++++++++++++---- crates/rpc/proto/zed.proto | 9 +- 4 files changed, 225 insertions(+), 86 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a33988dc5d..c0755cf1fe 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2123,9 +2123,10 @@ impl Editor { this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); // When buffer contents is updated and caret is moved, try triggering on type formatting. - if settings::get::(cx).use_on_type_format && text.len() == 1 { - let input_char = text.chars().next().expect("single char input"); - if let Some(on_type_format_task) = this.trigger_on_type_format(input_char, cx) { + if settings::get::(cx).use_on_type_format { + if let Some(on_type_format_task) = + this.trigger_on_type_formatting(text.to_string(), cx) + { on_type_format_task.detach_and_log_err(cx); } } @@ -2508,20 +2509,42 @@ impl Editor { } } - fn trigger_on_type_format( + fn trigger_on_type_formatting( &self, - input: char, + input: String, cx: &mut ViewContext, ) -> Option>> { + if input.len() != 1 { + return None; + } + + let transaction_title = format!("OnTypeFormatting after {input}"); + let workspace = self.workspace(cx)?; let project = self.project.as_ref()?; let position = self.selections.newest_anchor().head(); let (buffer, buffer_position) = self .buffer .read(cx) .text_anchor_for_position(position.clone(), cx)?; + let on_type_formatting = project.update(cx, |project, cx| { + project.on_type_format(buffer, buffer_position, input, cx) + }); - Some(project.update(cx, |project, cx| { - project.on_type_format(buffer.clone(), buffer_position, input, cx) + Some(cx.spawn(|editor, mut cx| async move { + let project_transaction = on_type_formatting.await?; + Self::open_project_transaction( + &editor, + workspace.downgrade(), + project_transaction, + transaction_title, + cx.clone(), + ) + .await?; + + editor.update(&mut cx, |editor, cx| { + editor.refresh_document_highlights(cx); + })?; + Ok(()) })) } diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 992973182f..72dc06e14f 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -2,7 +2,7 @@ use crate::{ DocumentHighlight, Hover, HoverBlock, HoverBlockKind, Location, LocationLink, Project, ProjectTransaction, }; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, Result}; use async_trait::async_trait; use client::proto::{self, PeerId}; use fs::LineEnding; @@ -123,6 +123,7 @@ pub(crate) struct OnTypeFormatting { pub position: PointUtf16, pub trigger: String, pub options: FormattingOptions, + pub push_to_history: bool, } pub(crate) struct FormattingOptions { @@ -1627,7 +1628,7 @@ impl LspCommand for GetCodeActions { #[async_trait(?Send)] impl LspCommand for OnTypeFormatting { - type Response = Vec<(Range, String)>; + type Response = ProjectTransaction; type LspRequest = lsp::request::OnTypeFormatting; type ProtoRequest = proto::OnTypeFormatting; @@ -1667,14 +1668,23 @@ impl LspCommand for OnTypeFormatting { buffer: ModelHandle, server_id: LanguageServerId, mut cx: AsyncAppContext, - ) -> Result, String)>> { - cx.update(|cx| { - project.update(cx, |project, cx| { - project.edits_from_lsp(&buffer, message.into_iter().flatten(), server_id, None, cx) - }) - }) - .await - .context("LSP edits conversion") + ) -> Result { + if let Some(edits) = message { + let (lsp_adapter, lsp_server) = + language_server_for_buffer(&project, &buffer, server_id, &mut cx)?; + Project::deserialize_edits( + project, + buffer, + edits, + self.push_to_history, + lsp_adapter, + lsp_server, + &mut cx, + ) + .await + } else { + Ok(ProjectTransaction::default()) + } } fn to_proto(&self, project_id: u64, buffer: &Buffer) -> proto::OnTypeFormatting { @@ -1714,58 +1724,38 @@ impl LspCommand for OnTypeFormatting { position: buffer.read_with(&cx, |buffer, _| position.to_point_utf16(buffer)), trigger: message.trigger.clone(), options: lsp_formatting_options(tab_size.get()).into(), + push_to_history: false, }) } fn response_to_proto( - response: Vec<(Range, String)>, - _: &mut Project, - _: PeerId, - buffer_version: &clock::Global, - _: &mut AppContext, + response: ProjectTransaction, + project: &mut Project, + peer_id: PeerId, + _: &clock::Global, + cx: &mut AppContext, ) -> proto::OnTypeFormattingResponse { + let transaction = project.serialize_project_transaction_for_peer(response, peer_id, cx); proto::OnTypeFormattingResponse { - entries: response - .into_iter() - .map( - |(response_range, new_text)| proto::OnTypeFormattingResponseEntry { - start: Some(language::proto::serialize_anchor(&response_range.start)), - end: Some(language::proto::serialize_anchor(&response_range.end)), - new_text, - }, - ) - .collect(), - version: serialize_version(&buffer_version), + transaction: Some(transaction), } } async fn response_from_proto( self, message: proto::OnTypeFormattingResponse, - _: ModelHandle, - buffer: ModelHandle, + project: ModelHandle, + _: ModelHandle, mut cx: AsyncAppContext, - ) -> Result, String)>> { - buffer - .update(&mut cx, |buffer, _| { - buffer.wait_for_version(deserialize_version(&message.version)) + ) -> Result { + let message = message + .transaction + .ok_or_else(|| anyhow!("missing transaction"))?; + project + .update(&mut cx, |project, cx| { + project.deserialize_project_transaction(message, self.push_to_history, cx) }) - .await?; - message - .entries - .into_iter() - .map(|entry| { - let start = entry - .start - .and_then(language::proto::deserialize_anchor) - .ok_or_else(|| anyhow!("invalid start"))?; - let end = entry - .end - .and_then(language::proto::deserialize_anchor) - .ok_or_else(|| anyhow!("invalid end"))?; - Ok((start..end, entry.new_text)) - }) - .collect() + .await } fn buffer_id_from_proto(message: &proto::OnTypeFormatting) -> u64 { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 36b3290121..5df4a94dc0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -417,6 +417,7 @@ impl Project { client.add_model_request_handler(Self::handle_delete_project_entry); client.add_model_request_handler(Self::handle_apply_additional_edits_for_completion); client.add_model_request_handler(Self::handle_apply_code_action); + client.add_model_request_handler(Self::handle_on_type_formatting); client.add_model_request_handler(Self::handle_reload_buffers); client.add_model_request_handler(Self::handle_synchronize_buffers); client.add_model_request_handler(Self::handle_format_buffers); @@ -429,7 +430,6 @@ impl Project { client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_lsp_command::); - client.add_model_request_handler(Self::handle_lsp_command::); client.add_model_request_handler(Self::handle_search_project); client.add_model_request_handler(Self::handle_get_project_symbols); client.add_model_request_handler(Self::handle_open_buffer_for_symbol); @@ -4035,6 +4035,118 @@ impl Project { } } + fn apply_on_type_formatting( + &self, + buffer: ModelHandle, + position: Anchor, + trigger: String, + push_to_history: bool, + cx: &mut ModelContext, + ) -> Task> { + if self.is_local() { + cx.spawn(|this, mut cx| async move { + // Do not allow multiple concurrent formatting requests for the + // same buffer. + this.update(&mut cx, |this, cx| { + this.buffers_being_formatted + .insert(buffer.read(cx).remote_id()) + }); + + let _cleanup = defer({ + let this = this.clone(); + let mut cx = cx.clone(); + let closure_buffer = buffer.clone(); + move || { + this.update(&mut cx, |this, cx| { + this.buffers_being_formatted + .remove(&closure_buffer.read(cx).remote_id()); + }); + } + }); + + buffer + .update(&mut cx, |buffer, _| { + buffer.wait_for_edits(Some(position.timestamp)) + }) + .await?; + this.update(&mut cx, |this, cx| { + let position = position.to_point_utf16(buffer.read(cx)); + this.on_type_format(buffer, position, trigger, cx) + }) + .await + }) + } else if let Some(project_id) = self.remote_id() { + let client = self.client.clone(); + let request = proto::OnTypeFormatting { + project_id, + buffer_id: buffer.read(cx).remote_id(), + position: Some(serialize_anchor(&position)), + trigger, + version: serialize_version(&buffer.read(cx).version()), + }; + cx.spawn(|this, mut cx| async move { + let response = client + .request(request) + .await? + .transaction + .ok_or_else(|| anyhow!("missing transaction"))?; + this.update(&mut cx, |this, cx| { + this.deserialize_project_transaction(response, push_to_history, cx) + }) + .await + }) + } else { + Task::ready(Err(anyhow!("project does not have a remote id"))) + } + } + + async fn deserialize_edits( + this: ModelHandle, + buffer_to_edit: ModelHandle, + edits: Vec, + push_to_history: bool, + _: Arc, + language_server: Arc, + cx: &mut AsyncAppContext, + ) -> Result { + let edits = this + .update(cx, |this, cx| { + this.edits_from_lsp( + &buffer_to_edit, + edits, + language_server.server_id(), + None, + cx, + ) + }) + .await?; + + let transaction = buffer_to_edit.update(cx, |buffer, cx| { + buffer.finalize_last_transaction(); + buffer.start_transaction(); + for (range, text) in edits { + buffer.edit([(range, text)], None, 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); + } + Some(transaction) + } else { + None + } + }); + + let mut project_transaction = ProjectTransaction::default(); + if let Some(transaction) = transaction { + project_transaction.0.insert(buffer_to_edit, transaction); + } + + Ok(project_transaction) + } + async fn deserialize_workspace_edit( this: ModelHandle, edit: lsp::WorkspaceEdit, @@ -4204,39 +4316,24 @@ impl Project { &self, buffer: ModelHandle, position: T, - input: char, + trigger: String, cx: &mut ModelContext, - ) -> Task> { + ) -> Task> { let tab_size = buffer.read_with(cx, |buffer, cx| { let language_name = buffer.language().map(|language| language.name()); language_settings(language_name.as_deref(), cx).tab_size }); let position = position.to_point_utf16(buffer.read(cx)); - let edits_task = self.request_lsp( + self.request_lsp( buffer.clone(), OnTypeFormatting { position, - trigger: input.to_string(), + trigger, options: lsp_command::lsp_formatting_options(tab_size.get()).into(), + push_to_history: true, }, cx, - ); - - cx.spawn(|_project, mut cx| async move { - let edits = edits_task - .await - .context("requesting OnTypeFormatting edits for char '{new_char}'")?; - - if !edits.is_empty() { - cx.update(|cx| { - buffer.update(cx, |buffer, cx| { - buffer.edit(edits, None, cx); - }); - }); - } - - Ok(()) - }) + ) } #[allow(clippy::type_complexity)] @@ -5809,6 +5906,42 @@ impl Project { }) } + async fn handle_on_type_formatting( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result { + let sender_id = envelope.original_sender_id()?; + let on_type_formatting = this.update(&mut cx, |this, cx| { + let buffer = this + .opened_buffers + .get(&envelope.payload.buffer_id) + .and_then(|buffer| buffer.upgrade(cx)) + .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; + let position = envelope + .payload + .position + .and_then(deserialize_anchor) + .ok_or_else(|| anyhow!("invalid position"))?; + Ok::<_, anyhow::Error>(this.apply_on_type_formatting( + buffer, + position, + envelope.payload.trigger.clone(), + false, + cx, + )) + })?; + + let project_transaction = on_type_formatting.await?; + let project_transaction = this.update(&mut cx, |this, cx| { + this.serialize_project_transaction_for_peer(project_transaction, sender_id, cx) + }); + Ok(proto::OnTypeFormattingResponse { + transaction: Some(project_transaction), + }) + } + async fn handle_lsp_command( this: ModelHandle, envelope: TypedEnvelope, @@ -6379,7 +6512,7 @@ impl Project { } #[allow(clippy::type_complexity)] - pub fn edits_from_lsp( + fn edits_from_lsp( &mut self, buffer: &ModelHandle, lsp_edits: impl 'static + Send + IntoIterator, diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index fe0d18f422..9e0d334944 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -682,14 +682,7 @@ message OnTypeFormatting { } message OnTypeFormattingResponse { - repeated OnTypeFormattingResponseEntry entries = 1; - repeated VectorClockEntry version = 2; -} - -message OnTypeFormattingResponseEntry { - Anchor start = 1; - Anchor end = 2; - string new_text = 3; + ProjectTransaction transaction = 1; } message PerformRenameResponse { From e2ff829f98e8cbcb062319bfc8e337f0ed0484c4 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 May 2023 15:49:07 +0300 Subject: [PATCH 09/10] Use Transaction instead of ProjectTransaction Co-Authored-By: Antonio Scandurra --- crates/collab/src/tests/integration_tests.rs | 30 ++++++------- crates/editor/src/editor.rs | 15 +------ crates/project/src/lsp_command.rs | 36 +++++++--------- crates/project/src/project.rs | 44 +++++++------------- crates/rpc/proto/zed.proto | 2 +- crates/rpc/src/rpc.rs | 2 +- 6 files changed, 51 insertions(+), 78 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 01be8d08df..ecbce24cf1 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7444,18 +7444,11 @@ async fn test_on_input_format_from_host_to_guest( let fake_language_server = fake_language_servers.next().await.unwrap(); cx_b.foreground().run_until_parked(); - // Type a on type formatting trigger character as the guest. - editor_a.update(cx_a, |editor, cx| { - editor.change_selections(None, cx, |s| s.select_ranges([13..13])); - editor.handle_input(">", cx); - cx.focus(&editor_a); - }); // Receive an OnTypeFormatting request as the host's language server. // Return some formattings from the host's language server. - cx_b.foreground().start_waiting(); - fake_language_server - .handle_request::(|params, _| async move { + fake_language_server.handle_request::( + |params, _| async move { assert_eq!( params.text_document_position.text_document.uri, lsp::Url::from_file_path("/a/main.rs").unwrap(), @@ -7469,18 +7462,27 @@ async fn test_on_input_format_from_host_to_guest( new_text: "~<".to_string(), range: lsp::Range::new(lsp::Position::new(0, 14), lsp::Position::new(0, 14)), }])) - }) - .next() - .await - .unwrap(); - cx_b.foreground().finish_waiting(); + }, + ); + // .next() + // .await + // .unwrap(); // Open the buffer on the guest and see that the formattings worked let buffer_b = project_b .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)) .await .unwrap(); + + // Type a on type formatting trigger character as the guest. + editor_a.update(cx_a, |editor, cx| { + cx.focus(&editor_a); + editor.change_selections(None, cx, |s| s.select_ranges([13..13])); + editor.handle_input(">", cx); + }); + cx_b.foreground().run_until_parked(); + buffer_b.read_with(cx_b, |buffer, _| { assert_eq!(buffer.text(), "fn main() { a>~< }") }); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c0755cf1fe..d944b97124 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2518,8 +2518,6 @@ impl Editor { return None; } - let transaction_title = format!("OnTypeFormatting after {input}"); - let workspace = self.workspace(cx)?; let project = self.project.as_ref()?; let position = self.selections.newest_anchor().head(); let (buffer, buffer_position) = self @@ -2527,20 +2525,11 @@ impl Editor { .read(cx) .text_anchor_for_position(position.clone(), cx)?; let on_type_formatting = project.update(cx, |project, cx| { - project.on_type_format(buffer, buffer_position, input, cx) + project.on_type_format(buffer, buffer_position, input, true, cx) }); Some(cx.spawn(|editor, mut cx| async move { - let project_transaction = on_type_formatting.await?; - Self::open_project_transaction( - &editor, - workspace.downgrade(), - project_transaction, - transaction_title, - cx.clone(), - ) - .await?; - + on_type_formatting.await?; editor.update(&mut cx, |editor, cx| { editor.refresh_document_highlights(cx); })?; diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 72dc06e14f..5ee6443896 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -12,7 +12,7 @@ use language::{ point_from_lsp, point_to_lsp, proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version}, range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CharKind, CodeAction, - Completion, OffsetRangeExt, PointUtf16, ToOffset, ToPointUtf16, Unclipped, + Completion, OffsetRangeExt, PointUtf16, ToOffset, ToPointUtf16, Transaction, Unclipped, }; use lsp::{DocumentHighlightKind, LanguageServer, LanguageServerId, ServerCapabilities}; use std::{cmp::Reverse, ops::Range, path::Path, sync::Arc}; @@ -1628,7 +1628,7 @@ impl LspCommand for GetCodeActions { #[async_trait(?Send)] impl LspCommand for OnTypeFormatting { - type Response = ProjectTransaction; + type Response = Option; type LspRequest = lsp::request::OnTypeFormatting; type ProtoRequest = proto::OnTypeFormatting; @@ -1668,7 +1668,7 @@ impl LspCommand for OnTypeFormatting { buffer: ModelHandle, server_id: LanguageServerId, mut cx: AsyncAppContext, - ) -> Result { + ) -> Result> { if let Some(edits) = message { let (lsp_adapter, lsp_server) = language_server_for_buffer(&project, &buffer, server_id, &mut cx)?; @@ -1683,7 +1683,7 @@ impl LspCommand for OnTypeFormatting { ) .await } else { - Ok(ProjectTransaction::default()) + Ok(None) } } @@ -1729,33 +1729,27 @@ impl LspCommand for OnTypeFormatting { } fn response_to_proto( - response: ProjectTransaction, - project: &mut Project, - peer_id: PeerId, + response: Option, + _: &mut Project, + _: PeerId, _: &clock::Global, - cx: &mut AppContext, + _: &mut AppContext, ) -> proto::OnTypeFormattingResponse { - let transaction = project.serialize_project_transaction_for_peer(response, peer_id, cx); proto::OnTypeFormattingResponse { - transaction: Some(transaction), + transaction: response + .map(|transaction| language::proto::serialize_transaction(&transaction)), } } async fn response_from_proto( self, message: proto::OnTypeFormattingResponse, - project: ModelHandle, + _: ModelHandle, _: ModelHandle, - mut cx: AsyncAppContext, - ) -> Result { - let message = message - .transaction - .ok_or_else(|| anyhow!("missing transaction"))?; - project - .update(&mut cx, |project, cx| { - project.deserialize_project_transaction(message, self.push_to_history, cx) - }) - .await + _: AsyncAppContext, + ) -> Result> { + let Some(transaction) = message.transaction else { return Ok(None) }; + Ok(Some(language::proto::deserialize_transaction(transaction)?)) } fn buffer_id_from_proto(message: &proto::OnTypeFormatting) -> u64 { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 5df4a94dc0..391a698a1b 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4040,9 +4040,8 @@ impl Project { buffer: ModelHandle, position: Anchor, trigger: String, - push_to_history: bool, cx: &mut ModelContext, - ) -> Task> { + ) -> Task>> { if self.is_local() { cx.spawn(|this, mut cx| async move { // Do not allow multiple concurrent formatting requests for the @@ -4071,7 +4070,7 @@ impl Project { .await?; this.update(&mut cx, |this, cx| { let position = position.to_point_utf16(buffer.read(cx)); - this.on_type_format(buffer, position, trigger, cx) + this.on_type_format(buffer, position, trigger, false, cx) }) .await }) @@ -4084,16 +4083,13 @@ impl Project { trigger, version: serialize_version(&buffer.read(cx).version()), }; - cx.spawn(|this, mut cx| async move { - let response = client + cx.spawn(|_, _| async move { + client .request(request) .await? .transaction - .ok_or_else(|| anyhow!("missing transaction"))?; - this.update(&mut cx, |this, cx| { - this.deserialize_project_transaction(response, push_to_history, cx) - }) - .await + .map(language::proto::deserialize_transaction) + .transpose() }) } else { Task::ready(Err(anyhow!("project does not have a remote id"))) @@ -4108,7 +4104,7 @@ impl Project { _: Arc, language_server: Arc, cx: &mut AsyncAppContext, - ) -> Result { + ) -> Result> { let edits = this .update(cx, |this, cx| { this.edits_from_lsp( @@ -4139,12 +4135,7 @@ impl Project { } }); - let mut project_transaction = ProjectTransaction::default(); - if let Some(transaction) = transaction { - project_transaction.0.insert(buffer_to_edit, transaction); - } - - Ok(project_transaction) + Ok(transaction) } async fn deserialize_workspace_edit( @@ -4317,8 +4308,9 @@ impl Project { buffer: ModelHandle, position: T, trigger: String, + push_to_history: bool, cx: &mut ModelContext, - ) -> Task> { + ) -> Task>> { let tab_size = buffer.read_with(cx, |buffer, cx| { let language_name = buffer.language().map(|language| language.name()); language_settings(language_name.as_deref(), cx).tab_size @@ -4330,7 +4322,7 @@ impl Project { position, trigger, options: lsp_command::lsp_formatting_options(tab_size.get()).into(), - push_to_history: true, + push_to_history, }, cx, ) @@ -5912,7 +5904,6 @@ impl Project { _: Arc, mut cx: AsyncAppContext, ) -> Result { - let sender_id = envelope.original_sender_id()?; let on_type_formatting = this.update(&mut cx, |this, cx| { let buffer = this .opened_buffers @@ -5928,18 +5919,15 @@ impl Project { buffer, position, envelope.payload.trigger.clone(), - false, cx, )) })?; - let project_transaction = on_type_formatting.await?; - let project_transaction = this.update(&mut cx, |this, cx| { - this.serialize_project_transaction_for_peer(project_transaction, sender_id, cx) - }); - Ok(proto::OnTypeFormattingResponse { - transaction: Some(project_transaction), - }) + let transaction = on_type_formatting + .await? + .as_ref() + .map(language::proto::serialize_transaction); + Ok(proto::OnTypeFormattingResponse { transaction }) } async fn handle_lsp_command( diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 9e0d334944..848cc1c2fa 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -682,7 +682,7 @@ message OnTypeFormatting { } message OnTypeFormattingResponse { - ProjectTransaction transaction = 1; + Transaction transaction = 1; } message PerformRenameResponse { diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index 64fbf19462..b929de9596 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 = 55; +pub const PROTOCOL_VERSION: u32 = 56; From 739d5ca3735cd2ad69517b8d661186ac1f349356 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 May 2023 18:07:38 +0300 Subject: [PATCH 10/10] Have proper undo for both client and host --- crates/collab/src/tests/integration_tests.rs | 45 ++++++++++++++++++-- crates/editor/src/editor.rs | 32 +++++++++++--- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index ecbce24cf1..d771f969d8 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7464,9 +7464,6 @@ async fn test_on_input_format_from_host_to_guest( }])) }, ); - // .next() - // .await - // .unwrap(); // Open the buffer on the guest and see that the formattings worked let buffer_b = project_b @@ -7486,6 +7483,27 @@ async fn test_on_input_format_from_host_to_guest( buffer_b.read_with(cx_b, |buffer, _| { assert_eq!(buffer.text(), "fn main() { a>~< }") }); + + // Undo should remove LSP edits first + editor_a.update(cx_a, |editor, cx| { + assert_eq!(editor.text(cx), "fn main() { a>~< }"); + editor.undo(&Undo, cx); + assert_eq!(editor.text(cx), "fn main() { a> }"); + }); + cx_b.foreground().run_until_parked(); + buffer_b.read_with(cx_b, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a> }") + }); + + editor_a.update(cx_a, |editor, cx| { + assert_eq!(editor.text(cx), "fn main() { a> }"); + editor.undo(&Undo, cx); + assert_eq!(editor.text(cx), "fn main() { a }"); + }); + cx_b.foreground().run_until_parked(); + buffer_b.read_with(cx_b, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a }") + }); } #[gpui::test(iterations = 10)] @@ -7595,6 +7613,27 @@ async fn test_on_input_format_from_guest_to_host( buffer_a.read_with(cx_a, |buffer, _| { assert_eq!(buffer.text(), "fn main() { a:~: }") }); + + // Undo should remove LSP edits first + editor_b.update(cx_b, |editor, cx| { + assert_eq!(editor.text(cx), "fn main() { a:~: }"); + editor.undo(&Undo, cx); + assert_eq!(editor.text(cx), "fn main() { a: }"); + }); + cx_a.foreground().run_until_parked(); + buffer_a.read_with(cx_a, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a: }") + }); + + editor_b.update(cx_b, |editor, cx| { + assert_eq!(editor.text(cx), "fn main() { a: }"); + editor.undo(&Undo, cx); + assert_eq!(editor.text(cx), "fn main() { a }"); + }); + cx_a.foreground().run_until_parked(); + buffer_a.read_with(cx_a, |buffer, _| { + assert_eq!(buffer.text(), "fn main() { a }") + }); } #[derive(Debug, Eq, PartialEq)] diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d944b97124..5a504a610c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2524,15 +2524,33 @@ impl Editor { .buffer .read(cx) .text_anchor_for_position(position.clone(), cx)?; - let on_type_formatting = project.update(cx, |project, cx| { - project.on_type_format(buffer, buffer_position, input, true, cx) - }); + // OnTypeFormatting retuns a list of edits, no need to pass them between Zed instances, + // hence we do LSP request & edit on host side only — add formats to host's history. + let push_to_lsp_host_history = true; + // If this is not the host, append its history with new edits. + let push_to_client_history = project.read(cx).is_remote(); + + let on_type_formatting = project.update(cx, |project, cx| { + project.on_type_format( + buffer.clone(), + buffer_position, + input, + push_to_lsp_host_history, + cx, + ) + }); Some(cx.spawn(|editor, mut cx| async move { - on_type_formatting.await?; - editor.update(&mut cx, |editor, cx| { - editor.refresh_document_highlights(cx); - })?; + if let Some(transaction) = on_type_formatting.await? { + if push_to_client_history { + buffer.update(&mut cx, |buffer, _| { + buffer.push_transaction(transaction, Instant::now()); + }); + } + editor.update(&mut cx, |editor, cx| { + editor.refresh_document_highlights(cx); + })?; + } Ok(()) })) }