From 65048760b2fdff02f44f916b07584ac8608061c6 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 1 Apr 2022 14:01:56 +0200 Subject: [PATCH 1/4] Allow explicit reload of buffers via `Project::reload_buffers` --- crates/language/src/buffer.rs | 65 ++++++++++-------- crates/language/src/tests.rs | 8 ++- crates/project/src/project.rs | 94 ++++++++++++++++++++++++++ crates/rpc/proto/zed.proto | 85 +++++++++++++----------- crates/rpc/src/proto.rs | 4 ++ crates/rpc/src/rpc.rs | 2 +- crates/server/src/rpc.rs | 120 +++++++++++++++++++++++++++++++++- 7 files changed, 310 insertions(+), 68 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 0c1ce7c2285588a50d7bef75357e5ab26f37b990..f2c9d209b17aeddefc88de0ab7700a2b02130cf2 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -498,6 +498,30 @@ impl Buffer { cx.notify(); } + pub fn reload(&mut self, cx: &mut ModelContext) -> Task>> { + cx.spawn(|this, mut cx| async move { + if let Some((new_mtime, new_text)) = this.read_with(&cx, |this, cx| { + let file = this.file.as_ref()?.as_local()?; + Some((file.mtime(), file.load(cx))) + }) { + let new_text = new_text.await?; + let diff = this + .read_with(&cx, |this, cx| this.diff(new_text.into(), cx)) + .await; + this.update(&mut cx, |this, cx| { + if let Some(transaction) = this.apply_diff(diff, cx).cloned() { + this.did_reload(this.version(), new_mtime, cx); + Ok(Some(transaction)) + } else { + Ok(None) + } + }) + } else { + Ok(None) + } + }) + } + pub fn did_reload( &mut self, version: clock::Global, @@ -543,29 +567,8 @@ impl Buffer { file_changed = true; if !self.is_dirty() { - task = cx.spawn(|this, mut cx| { - async move { - let new_text = this.read_with(&cx, |this, cx| { - this.file - .as_ref() - .and_then(|file| file.as_local().map(|f| f.load(cx))) - }); - if let Some(new_text) = new_text { - let new_text = new_text.await?; - let diff = this - .read_with(&cx, |this, cx| this.diff(new_text.into(), cx)) - .await; - this.update(&mut cx, |this, cx| { - if this.apply_diff(diff, cx) { - this.did_reload(this.version(), new_mtime, cx); - } - }); - } - Ok(()) - } - .log_err() - .map(drop) - }); + let reload = self.reload(cx).log_err().map(drop); + task = cx.foreground().spawn(reload); } } } @@ -902,8 +905,13 @@ impl Buffer { }) } - pub(crate) fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> bool { + pub(crate) fn apply_diff( + &mut self, + diff: Diff, + cx: &mut ModelContext, + ) -> Option<&Transaction> { if self.version == diff.base_version { + self.finalize_last_transaction(); self.start_transaction(); let mut offset = diff.start_offset; for (tag, len) in diff.changes { @@ -924,10 +932,13 @@ impl Buffer { } } } - self.end_transaction(cx); - true + if self.end_transaction(cx).is_some() { + self.finalize_last_transaction() + } else { + None + } } else { - false + None } } diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index 3eb87cefb62928573aa9087a39be8533f727bcc9..e980865cb432435ca8a2e1932e69a6a3a740fec3 100644 --- a/crates/language/src/tests.rs +++ b/crates/language/src/tests.rs @@ -136,12 +136,16 @@ async fn test_apply_diff(cx: &mut gpui::TestAppContext) { let text = "a\nccc\ndddd\nffffff\n"; let diff = buffer.read_with(cx, |b, cx| b.diff(text.into(), cx)).await; - buffer.update(cx, |b, cx| b.apply_diff(diff, cx)); + buffer.update(cx, |buffer, cx| { + buffer.apply_diff(diff, cx).unwrap(); + }); cx.read(|cx| assert_eq!(buffer.read(cx).text(), text)); let text = "a\n1\n\nccc\ndd2dd\nffffff\n"; let diff = buffer.read_with(cx, |b, cx| b.diff(text.into(), cx)).await; - buffer.update(cx, |b, cx| b.apply_diff(diff, cx)); + buffer.update(cx, |buffer, cx| { + buffer.apply_diff(diff, cx).unwrap(); + }); cx.read(|cx| assert_eq!(buffer.read(cx).text(), text)); } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 4ec856f19964166d42efd489520e242328ce7685..2316f4d80e24ad774336e87afad7aaf58f31669a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -270,6 +270,7 @@ impl Project { client.add_model_message_handler(Self::handle_update_worktree); 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_reload_buffers); client.add_model_request_handler(Self::handle_format_buffers); client.add_model_request_handler(Self::handle_get_code_actions); client.add_model_request_handler(Self::handle_get_completions); @@ -1973,6 +1974,70 @@ impl Project { Ok(()) } + pub fn reload_buffers( + &self, + buffers: HashSet>, + push_to_history: bool, + cx: &mut ModelContext, + ) -> Task> { + let mut local_buffers = Vec::new(); + let mut remote_buffers = None; + for buffer_handle in buffers { + let buffer = buffer_handle.read(cx); + if buffer.is_dirty() { + if let Some(file) = File::from_dyn(buffer.file()) { + if file.is_local() { + local_buffers.push(buffer_handle); + } else { + remote_buffers.get_or_insert(Vec::new()).push(buffer_handle); + } + } + } + } + + let remote_buffers = self.remote_id().zip(remote_buffers); + let client = self.client.clone(); + + cx.spawn(|this, mut cx| async move { + let mut project_transaction = ProjectTransaction::default(); + + if let Some((project_id, remote_buffers)) = remote_buffers { + let response = client + .request(proto::ReloadBuffers { + project_id, + buffer_ids: remote_buffers + .iter() + .map(|buffer| buffer.read_with(&cx, |buffer, _| buffer.remote_id())) + .collect(), + }) + .await? + .transaction + .ok_or_else(|| anyhow!("missing transaction"))?; + project_transaction = this + .update(&mut cx, |this, cx| { + this.deserialize_project_transaction(response, push_to_history, cx) + }) + .await?; + } + + for buffer in local_buffers { + let transaction = buffer + .update(&mut cx, |buffer, cx| buffer.reload(cx)) + .await?; + buffer.update(&mut cx, |buffer, cx| { + if let Some(transaction) = transaction { + if !push_to_history { + buffer.forget_transaction(transaction.id); + } + project_transaction.0.insert(cx.handle(), transaction); + } + }); + } + + Ok(project_transaction) + }) + } + pub fn format( &self, buffers: HashSet>, @@ -3667,6 +3732,35 @@ impl Project { }) } + async fn handle_reload_buffers( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result { + let sender_id = envelope.original_sender_id()?; + let reload = this.update(&mut cx, |this, cx| { + let mut buffers = HashSet::default(); + for buffer_id in &envelope.payload.buffer_ids { + buffers.insert( + this.opened_buffers + .get(buffer_id) + .map(|buffer| buffer.upgrade(cx).unwrap()) + .ok_or_else(|| anyhow!("unknown buffer id {}", buffer_id))?, + ); + } + Ok::<_, anyhow::Error>(this.reload_buffers(buffers, false, cx)) + })?; + + let project_transaction = reload.await?; + let project_transaction = this.update(&mut cx, |this, cx| { + this.serialize_project_transaction_for_peer(project_transaction, sender_id, cx) + }); + Ok(proto::ReloadBuffersResponse { + transaction: Some(project_transaction), + }) + } + async fn handle_format_buffers( this: ModelHandle, envelope: TypedEnvelope, diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 9d25e66190b14bc7d4624885ec7da3dc57acb61b..e86a9a556edfabd9d37e8443c387a8eb012b956c 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -48,43 +48,45 @@ message Envelope { SaveBuffer save_buffer = 40; BufferSaved buffer_saved = 41; BufferReloaded buffer_reloaded = 42; - FormatBuffers format_buffers = 43; - FormatBuffersResponse format_buffers_response = 44; - GetCompletions get_completions = 45; - GetCompletionsResponse get_completions_response = 46; - ApplyCompletionAdditionalEdits apply_completion_additional_edits = 47; - ApplyCompletionAdditionalEditsResponse apply_completion_additional_edits_response = 48; - GetCodeActions get_code_actions = 49; - GetCodeActionsResponse get_code_actions_response = 50; - ApplyCodeAction apply_code_action = 51; - ApplyCodeActionResponse apply_code_action_response = 52; - PrepareRename prepare_rename = 53; - PrepareRenameResponse prepare_rename_response = 54; - PerformRename perform_rename = 55; - PerformRenameResponse perform_rename_response = 56; - SearchProject search_project = 57; - SearchProjectResponse search_project_response = 58; - - GetChannels get_channels = 59; - GetChannelsResponse get_channels_response = 60; - JoinChannel join_channel = 61; - JoinChannelResponse join_channel_response = 62; - LeaveChannel leave_channel = 63; - SendChannelMessage send_channel_message = 64; - SendChannelMessageResponse send_channel_message_response = 65; - ChannelMessageSent channel_message_sent = 66; - GetChannelMessages get_channel_messages = 67; - GetChannelMessagesResponse get_channel_messages_response = 68; - - UpdateContacts update_contacts = 69; - - GetUsers get_users = 70; - GetUsersResponse get_users_response = 71; - - Follow follow = 72; - FollowResponse follow_response = 73; - UpdateFollowers update_followers = 74; - Unfollow unfollow = 75; + ReloadBuffers reload_buffers = 43; + ReloadBuffersResponse reload_buffers_response = 44; + FormatBuffers format_buffers = 45; + FormatBuffersResponse format_buffers_response = 46; + GetCompletions get_completions = 47; + GetCompletionsResponse get_completions_response = 48; + ApplyCompletionAdditionalEdits apply_completion_additional_edits = 49; + ApplyCompletionAdditionalEditsResponse apply_completion_additional_edits_response = 50; + GetCodeActions get_code_actions = 51; + GetCodeActionsResponse get_code_actions_response = 52; + ApplyCodeAction apply_code_action = 53; + ApplyCodeActionResponse apply_code_action_response = 54; + PrepareRename prepare_rename = 55; + PrepareRenameResponse prepare_rename_response = 56; + PerformRename perform_rename = 57; + PerformRenameResponse perform_rename_response = 58; + SearchProject search_project = 59; + SearchProjectResponse search_project_response = 60; + + GetChannels get_channels = 61; + GetChannelsResponse get_channels_response = 62; + JoinChannel join_channel = 63; + JoinChannelResponse join_channel_response = 64; + LeaveChannel leave_channel = 65; + SendChannelMessage send_channel_message = 66; + SendChannelMessageResponse send_channel_message_response = 67; + ChannelMessageSent channel_message_sent = 68; + GetChannelMessages get_channel_messages = 69; + GetChannelMessagesResponse get_channel_messages_response = 70; + + UpdateContacts update_contacts = 71; + + GetUsers get_users = 72; + GetUsersResponse get_users_response = 73; + + Follow follow = 74; + FollowResponse follow_response = 75; + UpdateFollowers update_followers = 76; + Unfollow unfollow = 77; } } @@ -299,6 +301,15 @@ message BufferReloaded { Timestamp mtime = 4; } +message ReloadBuffers { + uint64 project_id = 1; + repeated uint64 buffer_ids = 2; +} + +message ReloadBuffersResponse { + ProjectTransaction transaction = 1; +} + message FormatBuffers { uint64 project_id = 1; repeated uint64 buffer_ids = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 59d6773451fd2feebc28b17120e0b50a58de1127..a9f6b80f8e8d7895d705657d023f2b95a7c073a9 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -190,6 +190,8 @@ messages!( (Ping, Foreground), (RegisterProject, Foreground), (RegisterWorktree, Foreground), + (ReloadBuffers, Foreground), + (ReloadBuffersResponse, Foreground), (RemoveProjectCollaborator, Foreground), (SaveBuffer, Foreground), (SearchProject, Background), @@ -237,6 +239,7 @@ request_messages!( (PrepareRename, PrepareRenameResponse), (RegisterProject, RegisterProjectResponse), (RegisterWorktree, Ack), + (ReloadBuffers, ReloadBuffersResponse), (SaveBuffer, BufferSaved), (SearchProject, SearchProjectResponse), (SendChannelMessage, SendChannelMessageResponse), @@ -268,6 +271,7 @@ entity_messages!( OpenBufferForSymbol, PerformRename, PrepareRename, + ReloadBuffers, RemoveProjectCollaborator, SaveBuffer, SearchProject, diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index cfe780d5118c764a829cf047d288bc1e7a0b590e..9ee18faae2543b93e194f0e3c2a4fbe9e6604a84 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -5,4 +5,4 @@ pub mod proto; pub use conn::Connection; pub use peer::*; -pub const PROTOCOL_VERSION: u32 = 12; +pub const PROTOCOL_VERSION: u32 = 13; diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 374aaf6a7ec6820022c1802757cdabf3b51aa946..4d23f61ef74914a28c924b957bc113c8e94002a1 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -102,6 +102,7 @@ impl Server { .add_request_handler(Server::forward_project_request::) .add_request_handler(Server::forward_project_request::) .add_request_handler(Server::forward_project_request::) + .add_request_handler(Server::forward_project_request::) .add_request_handler(Server::forward_project_request::) .add_request_handler(Server::update_buffer) .add_message_handler(Server::update_buffer_file) @@ -1089,7 +1090,7 @@ mod tests { use gpui::{executor, geometry::vector::vec2f, ModelHandle, TestAppContext, ViewHandle}; use language::{ tree_sitter_rust, Diagnostic, DiagnosticEntry, Language, LanguageConfig, LanguageRegistry, - LanguageServerConfig, OffsetRangeExt, Point, ToLspPosition, + LanguageServerConfig, OffsetRangeExt, Point, Rope, ToLspPosition, }; use lsp; use parking_lot::Mutex; @@ -2460,6 +2461,123 @@ mod tests { .await; } + #[gpui::test(iterations = 10)] + async fn test_reloading_buffer_manually(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { + cx_a.foreground().forbid_parking(); + let lang_registry = Arc::new(LanguageRegistry::test()); + let fs = FakeFs::new(cx_a.background()); + + // Connect to a server as 2 clients. + let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + + // Share a project as client A + fs.insert_tree( + "/a", + json!({ + ".zed.toml": r#"collaborators = ["user_b"]"#, + "a.rs": "let one = 1;", + }), + ) + .await; + let project_a = cx_a.update(|cx| { + Project::local( + client_a.clone(), + client_a.user_store.clone(), + lang_registry.clone(), + fs.clone(), + cx, + ) + }); + let (worktree_a, _) = project_a + .update(cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", true, cx) + }) + .await + .unwrap(); + worktree_a + .read_with(cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) + .await; + let project_id = project_a.update(cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(cx_a, |tree, _| tree.id()); + project_a.update(cx_a, |p, cx| p.share(cx)).await.unwrap(); + let buffer_a = project_a + .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx)) + .await + .unwrap(); + + // Join the worktree as client B. + let project_b = Project::remote( + project_id, + client_b.clone(), + client_b.user_store.clone(), + lang_registry.clone(), + fs.clone(), + &mut cx_b.to_async(), + ) + .await + .unwrap(); + + let buffer_b = cx_b + .background() + .spawn(project_b.update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx))) + .await + .unwrap(); + buffer_b.update(cx_b, |buffer, cx| { + buffer.edit([4..7], "six", cx); + buffer.edit([10..11], "6", cx); + assert_eq!(buffer.text(), "let six = 6;"); + assert!(buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + buffer_a + .condition(cx_a, |buffer, _| buffer.text() == "let six = 6;") + .await; + + fs.save(Path::new("/a/a.rs"), &Rope::from("let seven = 7;")) + .await + .unwrap(); + buffer_a + .condition(cx_a, |buffer, _| buffer.has_conflict()) + .await; + buffer_b + .condition(cx_b, |buffer, _| buffer.has_conflict()) + .await; + + project_b + .update(cx_b, |project, cx| { + project.reload_buffers(HashSet::from_iter([buffer_b.clone()]), true, cx) + }) + .await + .unwrap(); + buffer_a.read_with(cx_a, |buffer, _| { + assert_eq!(buffer.text(), "let seven = 7;"); + assert!(!buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + buffer_b.read_with(cx_b, |buffer, _| { + assert_eq!(buffer.text(), "let seven = 7;"); + assert!(!buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + + buffer_a.update(cx_a, |buffer, cx| { + // Undoing on the host is a no-op when the reload was initiated by the guest. + buffer.undo(cx); + assert_eq!(buffer.text(), "let seven = 7;"); + assert!(!buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + buffer_b.update(cx_b, |buffer, cx| { + // Undoing on the guest rolls back the buffer to before it was reloaded but the conflict gets cleared. + buffer.undo(cx); + assert_eq!(buffer.text(), "let six = 6;"); + assert!(buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + } + #[gpui::test(iterations = 10)] async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { cx_a.foreground().forbid_parking(); From 703f1c3be086844405be3950a513e17b6eb4bed3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 1 Apr 2022 14:02:49 +0200 Subject: [PATCH 2/4] Introduce `workspace::Item::reload` to manually trigger a reload --- crates/diagnostics/src/diagnostics.rs | 8 ++++++++ crates/editor/src/items.rs | 25 +++++++++++++++++++++++++ crates/search/src/project_search.rs | 9 +++++++++ crates/workspace/src/workspace.rs | 15 +++++++++++++++ 4 files changed, 57 insertions(+) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 56de434cf49e73c82ac19eefd7be36bdd3f5c71e..da50e99f1e5175d04900c4d0a4839479e1dd82c7 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -478,6 +478,14 @@ impl workspace::Item for ProjectDiagnosticsEditor { self.editor.save(project, cx) } + fn reload( + &mut self, + project: ModelHandle, + cx: &mut ViewContext, + ) -> Task> { + self.editor.reload(project, cx) + } + fn can_save_as(&self, _: &AppContext) -> bool { false } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 79b25f8f60fa2ebe9e9832aac99b68c91b775f6b..67d5aee7731d9e23934849a8ee638de034bc82f6 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -371,6 +371,31 @@ impl Item for Editor { }) } + fn reload( + &mut self, + project: ModelHandle, + cx: &mut ViewContext, + ) -> Task> { + let buffer = self.buffer().clone(); + let buffers = self.buffer.read(cx).all_buffers(); + let reload_buffers = + project.update(cx, |project, cx| project.reload_buffers(buffers, true, cx)); + cx.spawn(|this, mut cx| async move { + let transaction = reload_buffers.log_err().await; + this.update(&mut cx, |editor, cx| { + editor.request_autoscroll(Autoscroll::Fit, cx) + }); + buffer.update(&mut cx, |buffer, _| { + if let Some(transaction) = transaction { + if !buffer.is_singleton() { + buffer.push_transaction(&transaction.0); + } + } + }); + Ok(()) + }) + } + fn should_activate_item_on_event(event: &Event) -> bool { matches!(event, Event::Activate) } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 65bb07ae46f4d6a1c43b409b6dc45bd40de2f139..745f23154fd98a77b3f20e79f1f4d513f235ebc5 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -280,6 +280,15 @@ impl Item for ProjectSearchView { unreachable!("save_as should not have been called") } + fn reload( + &mut self, + project: ModelHandle, + cx: &mut ViewContext, + ) -> Task> { + self.results_editor + .update(cx, |editor, cx| editor.reload(project, cx)) + } + fn clone_on_split(&self, cx: &mut ViewContext) -> Option where Self: Sized, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 9929cd9a51cd675e80953ad371ed1f94032e7459..073808e71a807cb1fb6dea48205e4f571738ecf7 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -237,6 +237,11 @@ pub trait Item: View { abs_path: PathBuf, cx: &mut ViewContext, ) -> Task>; + fn reload( + &mut self, + project: ModelHandle, + cx: &mut ViewContext, + ) -> Task>; fn should_activate_item_on_event(_: &Self::Event) -> bool { false } @@ -380,6 +385,8 @@ pub trait ItemHandle: 'static + fmt::Debug { abs_path: PathBuf, cx: &mut MutableAppContext, ) -> Task>; + fn reload(&self, project: ModelHandle, cx: &mut MutableAppContext) + -> Task>; fn act_as_type(&self, type_id: TypeId, cx: &AppContext) -> Option; fn to_followable_item_handle(&self, cx: &AppContext) -> Option>; } @@ -531,6 +538,14 @@ impl ItemHandle for ViewHandle { self.update(cx, |item, cx| item.save_as(project, abs_path, cx)) } + fn reload( + &self, + project: ModelHandle, + cx: &mut MutableAppContext, + ) -> Task> { + self.update(cx, |item, cx| item.reload(project, cx)) + } + fn is_dirty(&self, cx: &AppContext) -> bool { self.read(cx).is_dirty(cx) } From e93ab4db14e3f1f1bd2e991ce8a83c1e805e8f68 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 1 Apr 2022 14:32:41 +0200 Subject: [PATCH 3/4] Prompt before closing buffer with unsaved changes or conflicts --- crates/workspace/src/pane.rs | 331 +++++++++++++++++++++++++----- crates/workspace/src/workspace.rs | 7 +- crates/zed/src/zed.rs | 29 +-- 3 files changed, 305 insertions(+), 62 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index d48a5711a3373ce9aa8752fe2b78a5fcd46f2aec..33df9fd4c79285919330510a383befb94b6e5284 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1,16 +1,17 @@ use super::{ItemHandle, SplitDirection}; use crate::{toolbar::Toolbar, Item, Settings, WeakItemHandle, Workspace}; use collections::{HashMap, VecDeque}; +use futures::StreamExt; use gpui::{ action, elements::*, geometry::{rect::RectF, vector::vec2f}, keymap::Binding, platform::{CursorStyle, NavigationDirection}, - AppContext, Entity, MutableAppContext, Quad, RenderContext, Task, View, ViewContext, - ViewHandle, WeakViewHandle, + AppContext, Entity, ModelHandle, MutableAppContext, PromptLevel, Quad, RenderContext, Task, + View, ViewContext, ViewHandle, WeakViewHandle, }; -use project::{ProjectEntryId, ProjectPath}; +use project::{Project, ProjectEntryId, ProjectPath}; use std::{any::Any, cell::RefCell, cmp, mem, rc::Rc}; use util::ResultExt; @@ -37,13 +38,13 @@ pub fn init(cx: &mut MutableAppContext) { pane.activate_next_item(cx); }); cx.add_action(|pane: &mut Pane, _: &CloseActiveItem, cx| { - pane.close_active_item(cx); + pane.close_active_item(cx).detach(); }); cx.add_action(|pane: &mut Pane, _: &CloseInactiveItems, cx| { - pane.close_inactive_items(cx); + pane.close_inactive_items(cx).detach(); }); cx.add_action(|pane: &mut Pane, action: &CloseItem, cx| { - pane.close_item(action.0, cx); + pane.close_item(action.0, cx).detach(); }); cx.add_action(|pane: &mut Pane, action: &Split, cx| { pane.split(action.0, cx); @@ -97,6 +98,7 @@ pub struct Pane { active_item_index: usize, nav_history: Rc>, toolbar: ViewHandle, + project: ModelHandle, } pub struct ItemNavHistory { @@ -132,12 +134,13 @@ pub struct NavigationEntry { } impl Pane { - pub fn new(cx: &mut ViewContext) -> Self { + pub fn new(project: ModelHandle, cx: &mut ViewContext) -> Self { Self { items: Vec::new(), active_item_index: 0, nav_history: Default::default(), toolbar: cx.add_view(|_| Toolbar::new()), + project, } } @@ -403,65 +406,137 @@ impl Pane { self.activate_item(index, true, cx); } - pub fn close_active_item(&mut self, cx: &mut ViewContext) { - if !self.items.is_empty() { + pub fn close_active_item(&mut self, cx: &mut ViewContext) -> Task<()> { + if self.items.is_empty() { + Task::ready(()) + } else { self.close_item(self.items[self.active_item_index].id(), cx) } } - pub fn close_inactive_items(&mut self, cx: &mut ViewContext) { - if !self.items.is_empty() { + pub fn close_inactive_items(&mut self, cx: &mut ViewContext) -> Task<()> { + if self.items.is_empty() { + Task::ready(()) + } else { let active_item_id = self.items[self.active_item_index].id(); - self.close_items(cx, |id| id != active_item_id); + self.close_items(cx, move |id| id != active_item_id) } } - pub fn close_item(&mut self, view_id_to_close: usize, cx: &mut ViewContext) { - self.close_items(cx, |view_id| view_id == view_id_to_close); + pub fn close_item(&mut self, view_id_to_close: usize, cx: &mut ViewContext) -> Task<()> { + self.close_items(cx, move |view_id| view_id == view_id_to_close) } pub fn close_items( &mut self, cx: &mut ViewContext, - should_close: impl Fn(usize) -> bool, - ) { - let mut item_ix = 0; - let mut new_active_item_index = self.active_item_index; - self.items.retain(|item| { - if should_close(item.id()) { - if item_ix == self.active_item_index { - item.deactivated(cx); - } + should_close: impl 'static + Fn(usize) -> bool, + ) -> Task<()> { + const CONFLICT_MESSAGE: &'static str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; + const DIRTY_MESSAGE: &'static str = + "This file contains unsaved edits. Do you want to save it?"; + + let project = self.project.clone(); + cx.spawn(|this, mut cx| async move { + while let Some(item_to_close_ix) = this.read_with(&cx, |this, _| { + this.items.iter().position(|item| should_close(item.id())) + }) { + let item = + this.read_with(&cx, |this, _| this.items[item_to_close_ix].boxed_clone()); + if cx.read(|cx| item.can_save(cx)) { + if cx.read(|cx| item.has_conflict(cx)) { + let mut answer = this.update(&mut cx, |this, cx| { + this.activate_item(item_to_close_ix, true, cx); + cx.prompt( + PromptLevel::Warning, + CONFLICT_MESSAGE, + &["Overwrite", "Discard", "Cancel"], + ) + }); - if item_ix < self.active_item_index { - new_active_item_index -= 1; - } + match answer.next().await { + Some(0) => { + if cx + .update(|cx| item.save(project.clone(), cx)) + .await + .log_err() + .is_none() + { + break; + } + } + Some(1) => { + if cx + .update(|cx| item.reload(project.clone(), cx)) + .await + .log_err() + .is_none() + { + break; + } + } + _ => break, + } + } else if cx.read(|cx| item.is_dirty(cx)) { + let mut answer = this.update(&mut cx, |this, cx| { + this.activate_item(item_to_close_ix, true, cx); + cx.prompt( + PromptLevel::Warning, + DIRTY_MESSAGE, + &["Save", "Don't Save", "Cancel"], + ) + }); - let mut nav_history = self.nav_history.borrow_mut(); - if let Some(path) = item.project_path(cx) { - nav_history.paths_by_item.insert(item.id(), path); - } else { - nav_history.paths_by_item.remove(&item.id()); + match answer.next().await { + Some(0) => { + if cx + .update(|cx| item.save(project.clone(), cx)) + .await + .log_err() + .is_none() + { + break; + } + } + Some(1) => {} + _ => break, + } + } } - item_ix += 1; - false - } else { - item_ix += 1; - true + this.update(&mut cx, |this, cx| { + if let Some(item_ix) = this.items.iter().position(|i| i.id() == item.id()) { + this.items.remove(item_ix); + if item_ix == this.active_item_index { + item.deactivated(cx); + } + if item_ix < this.active_item_index { + this.active_item_index -= 1; + } + this.active_item_index = + cmp::min(this.active_item_index, this.items.len().saturating_sub(1)); + + let mut nav_history = this.nav_history.borrow_mut(); + if let Some(path) = item.project_path(cx) { + nav_history.paths_by_item.insert(item.id(), path); + } else { + nav_history.paths_by_item.remove(&item.id()); + } + } + }); } - }); - - if self.items.is_empty() { - cx.emit(Event::Remove); - } else { - self.active_item_index = cmp::min(new_active_item_index, self.items.len() - 1); - self.focus_active_item(cx); - self.activate(cx); - } - self.update_toolbar(cx); - cx.notify(); + this.update(&mut cx, |this, cx| { + if this.items.is_empty() { + cx.emit(Event::Remove); + } else { + this.focus_active_item(cx); + this.activate(cx); + } + this.update_toolbar(cx); + cx.notify(); + }) + }) } pub fn focus_active_item(&mut self, cx: &mut ViewContext) { @@ -743,3 +818,165 @@ impl NavHistory { } } } + +#[cfg(test)] +mod tests { + use crate::WorkspaceParams; + + use super::*; + use gpui::TestAppContext; + + #[gpui::test] + async fn test_close_items(cx: &mut TestAppContext) { + cx.foreground().forbid_parking(); + + let params = cx.update(WorkspaceParams::test); + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(¶ms, cx)); + let item1 = cx.add_view(window_id, |_| TestItem::new(false, true)); + let item2 = cx.add_view(window_id, |_| TestItem::new(true, true)); + let item3 = cx.add_view(window_id, |_| TestItem::new(false, true)); + let item4 = cx.add_view(window_id, |_| TestItem::new(true, false)); + let pane = workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item1.clone()), cx); + workspace.add_item(Box::new(item3.clone()), cx); + workspace.add_item(Box::new(item4.clone()), cx); + workspace.add_item(Box::new(item2.clone()), cx); + assert_eq!(workspace.active_item(cx).unwrap().id(), item2.id()); + + workspace.active_pane().clone() + }); + + let close_items = pane.update(cx, |pane, cx| { + let item1_id = item1.id(); + let item3_id = item3.id(); + let item4_id = item4.id(); + pane.close_items(cx, move |id| { + id == item1_id || id == item3_id || id == item4_id + }) + }); + + cx.foreground().run_until_parked(); + pane.read_with(cx, |pane, _| { + assert_eq!(pane.items.len(), 4); + assert_eq!(pane.active_item().unwrap().id(), item1.id()); + }); + + cx.simulate_prompt_answer(window_id, 0); + cx.foreground().run_until_parked(); + pane.read_with(cx, |pane, cx| { + assert_eq!(item1.read(cx).save_count, 1); + assert_eq!(item1.read(cx).reload_count, 0); + assert_eq!(pane.items.len(), 3); + assert_eq!(pane.active_item().unwrap().id(), item3.id()); + }); + + cx.simulate_prompt_answer(window_id, 1); + cx.foreground().run_until_parked(); + pane.read_with(cx, |pane, cx| { + assert_eq!(item3.read(cx).save_count, 0); + assert_eq!(item3.read(cx).reload_count, 1); + assert_eq!(pane.items.len(), 2); + assert_eq!(pane.active_item().unwrap().id(), item4.id()); + }); + + cx.simulate_prompt_answer(window_id, 0); + close_items.await; + pane.read_with(cx, |pane, cx| { + assert_eq!(item4.read(cx).save_count, 1); + assert_eq!(item4.read(cx).reload_count, 0); + assert_eq!(pane.items.len(), 1); + assert_eq!(pane.active_item().unwrap().id(), item2.id()); + }); + } + + struct TestItem { + is_dirty: bool, + has_conflict: bool, + save_count: usize, + reload_count: usize, + } + + impl TestItem { + fn new(is_dirty: bool, has_conflict: bool) -> Self { + Self { + save_count: 0, + reload_count: 0, + is_dirty, + has_conflict, + } + } + } + + impl Entity for TestItem { + type Event = (); + } + + impl View for TestItem { + fn ui_name() -> &'static str { + "TestItem" + } + + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + Empty::new().boxed() + } + } + + impl Item for TestItem { + fn tab_content(&self, _: &theme::Tab, _: &AppContext) -> ElementBox { + Empty::new().boxed() + } + + fn project_path(&self, _: &AppContext) -> Option { + None + } + + fn project_entry_id(&self, _: &AppContext) -> Option { + None + } + + fn set_nav_history(&mut self, _: ItemNavHistory, _: &mut ViewContext) {} + + fn is_dirty(&self, _: &AppContext) -> bool { + self.is_dirty + } + + fn has_conflict(&self, _: &AppContext) -> bool { + self.has_conflict + } + + fn can_save(&self, _: &AppContext) -> bool { + true + } + + fn save( + &mut self, + _: ModelHandle, + _: &mut ViewContext, + ) -> Task> { + self.save_count += 1; + Task::ready(Ok(())) + } + + fn can_save_as(&self, _: &AppContext) -> bool { + false + } + + fn save_as( + &mut self, + _: ModelHandle, + _: std::path::PathBuf, + _: &mut ViewContext, + ) -> Task> { + unreachable!() + } + + fn reload( + &mut self, + _: ModelHandle, + _: &mut ViewContext, + ) -> Task> { + self.reload_count += 1; + Task::ready(Ok(())) + } + } +} diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 073808e71a807cb1fb6dea48205e4f571738ecf7..c447f3a5fd567c1f5f89a11f2e93edce632648b9 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -497,7 +497,8 @@ impl ItemHandle for ViewHandle { } if T::should_close_item_on_event(event) { - pane.update(cx, |pane, cx| pane.close_item(item.id(), cx)); + pane.update(cx, |pane, cx| pane.close_item(item.id(), cx)) + .detach(); return; } @@ -737,7 +738,7 @@ impl Workspace { }) .detach(); - let pane = cx.add_view(|cx| Pane::new(cx)); + let pane = cx.add_view(|cx| Pane::new(params.project.clone(), cx)); let pane_id = pane.id(); cx.observe(&pane, move |me, _, cx| { let active_entry = me.active_project_path(cx); @@ -1069,7 +1070,7 @@ impl Workspace { } fn add_pane(&mut self, cx: &mut ViewContext) -> ViewHandle { - let pane = cx.add_view(|cx| Pane::new(cx)); + let pane = cx.add_view(|cx| Pane::new(self.project.clone(), cx)); let pane_id = pane.id(); cx.observe(&pane, move |me, _, cx| { let active_entry = me.active_project_path(cx); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index c94f8a0e8164a5f1b7a9cbcd9229f1e65ed59593..24bdf4c0ef8e641216f995d7b23f5359bbfc5be6 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -867,12 +867,15 @@ mod tests { // Go forward to an item that has been closed, ensuring it gets re-opened at the same // location. - workspace.update(cx, |workspace, cx| { - workspace - .active_pane() - .update(cx, |pane, cx| pane.close_item(editor3.id(), cx)); - drop(editor3); - }); + workspace + .update(cx, |workspace, cx| { + let editor3_id = editor3.id(); + drop(editor3); + workspace + .active_pane() + .update(cx, |pane, cx| pane.close_item(editor3_id, cx)) + }) + .await; workspace .update(cx, |w, cx| Pane::go_forward(w, None, cx)) .await; @@ -884,15 +887,17 @@ mod tests { // Go back to an item that has been closed and removed from disk, ensuring it gets skipped. workspace .update(cx, |workspace, cx| { + let editor2_id = editor2.id(); + drop(editor2); workspace .active_pane() - .update(cx, |pane, cx| pane.close_item(editor2.id(), cx)); - drop(editor2); - app_state - .fs - .as_fake() - .remove_file(Path::new("/root/a/file2"), Default::default()) + .update(cx, |pane, cx| pane.close_item(editor2_id, cx)) }) + .await; + app_state + .fs + .as_fake() + .remove_file(Path::new("/root/a/file2"), Default::default()) .await .unwrap(); workspace From be677a8a4b76b9a974177508bbec69d78c9a3c77 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 1 Apr 2022 15:27:06 +0200 Subject: [PATCH 4/4] Don't assume the `CloseActiveItem` action is synchronous in test --- crates/zed/src/zed.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 24bdf4c0ef8e641216f995d7b23f5359bbfc5be6..00f57ccccb7bd6ebf34d89c91bcbc1c7fbf7100d 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -683,6 +683,8 @@ mod tests { #[gpui::test] async fn test_pane_actions(cx: &mut TestAppContext) { + cx.foreground().forbid_parking(); + cx.update(|cx| pane::init(cx)); let app_state = cx.update(test_app_state); app_state @@ -740,7 +742,9 @@ mod tests { assert_eq!(pane2_item.project_path(cx.as_ref()), Some(file1.clone())); cx.dispatch_action(window_id, vec![pane_2.id()], &workspace::CloseActiveItem); - let workspace = workspace.read(cx); + }); + cx.foreground().run_until_parked(); + workspace.read_with(cx, |workspace, _| { assert_eq!(workspace.panes().len(), 1); assert_eq!(workspace.active_pane(), &pane_1); });