From 636f35f3846b967408c888867257c27cc4c2d86f Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 2 Sep 2022 11:58:21 -0700 Subject: [PATCH] Avoid undesirable pane item deduping with multibuffers --- crates/workspace/src/pane.rs | 144 +++++++++++++++++++++++++++--- crates/workspace/src/workspace.rs | 15 ++++ 2 files changed, 148 insertions(+), 11 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 1535f9e8fb..ceb2385ab8 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -478,14 +478,28 @@ impl Pane { item.added_to_pane(workspace, pane.clone(), cx); // Does the item already exist? - if let Some(existing_item_index) = pane.read(cx).items.iter().position(|existing_item| { - let existing_item_entry_ids = existing_item.project_entry_ids(cx); - let added_item_entry_ids = item.project_entry_ids(cx); - let entries_match = !existing_item_entry_ids.is_empty() - && existing_item_entry_ids == added_item_entry_ids; + let project_entry_id = if item.is_singleton(cx) { + item.project_entry_ids(cx).get(0).copied() + } else { + None + }; - existing_item.id() == item.id() || entries_match - }) { + let existing_item_index = pane.read(cx).items.iter().position(|existing_item| { + if existing_item.id() == item.id() { + true + } else if existing_item.is_singleton(cx) { + existing_item + .project_entry_ids(cx) + .get(0) + .map_or(false, |existing_entry_id| { + Some(existing_entry_id) == project_entry_id.as_ref() + }) + } else { + false + } + }); + + if let Some(existing_item_index) = existing_item_index { // If the item already exists, move it to the desired destination and activate it pane.update(cx, |pane, cx| { if existing_item_index != insertion_index { @@ -1540,13 +1554,11 @@ impl NavHistory { #[cfg(test)] mod tests { + use super::*; + use crate::tests::TestItem; use gpui::TestAppContext; use project::FakeFs; - use crate::tests::TestItem; - - use super::*; - #[gpui::test] async fn test_add_item_with_new_item(cx: &mut TestAppContext) { cx.foreground().forbid_parking(); @@ -1711,6 +1723,116 @@ mod tests { assert_item_labels(&pane, ["A*", "B", "C"], cx); } + #[gpui::test] + async fn test_add_item_with_same_project_entries(cx: &mut TestAppContext) { + cx.foreground().forbid_parking(); + Settings::test_async(cx); + let fs = FakeFs::new(cx.background()); + + let project = Project::test(fs, None, cx).await; + let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + + // singleton view + workspace.update(cx, |workspace, cx| { + let item = TestItem::new() + .with_singleton(true) + .with_label("buffer 1") + .with_project_entry_ids(&[1]); + + Pane::add_item( + workspace, + &pane, + Box::new(cx.add_view(|_| item)), + false, + false, + None, + cx, + ); + }); + assert_item_labels(&pane, ["buffer 1*"], cx); + + // new singleton view with the same project entry + workspace.update(cx, |workspace, cx| { + let item = TestItem::new() + .with_singleton(true) + .with_label("buffer 1") + .with_project_entry_ids(&[1]); + + Pane::add_item( + workspace, + &pane, + Box::new(cx.add_view(|_| item)), + false, + false, + None, + cx, + ); + }); + assert_item_labels(&pane, ["buffer 1*"], cx); + + // new singleton view with different project entry + workspace.update(cx, |workspace, cx| { + let item = TestItem::new() + .with_singleton(true) + .with_label("buffer 2") + .with_project_entry_ids(&[2]); + + Pane::add_item( + workspace, + &pane, + Box::new(cx.add_view(|_| item)), + false, + false, + None, + cx, + ); + }); + assert_item_labels(&pane, ["buffer 1", "buffer 2*"], cx); + + // new multibuffer view with the same project entry + workspace.update(cx, |workspace, cx| { + let item = TestItem::new() + .with_singleton(false) + .with_label("multibuffer 1") + .with_project_entry_ids(&[1]); + + Pane::add_item( + workspace, + &pane, + Box::new(cx.add_view(|_| item)), + false, + false, + None, + cx, + ); + }); + assert_item_labels(&pane, ["buffer 1", "buffer 2", "multibuffer 1*"], cx); + + // another multibuffer view with the same project entry + workspace.update(cx, |workspace, cx| { + let item = TestItem::new() + .with_singleton(false) + .with_label("multibuffer 1b") + .with_project_entry_ids(&[1]); + + Pane::add_item( + workspace, + &pane, + Box::new(cx.add_view(|_| item)), + false, + false, + None, + cx, + ); + }); + assert_item_labels( + &pane, + ["buffer 1", "buffer 2", "multibuffer 1", "multibuffer 1b*"], + cx, + ); + } + fn set_labeled_items( workspace: &ViewHandle, pane: &ViewHandle, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index ac24467343..5bc3c1054e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3451,6 +3451,21 @@ mod tests { self } + pub fn with_singleton(mut self, singleton: bool) -> Self { + self.is_singleton = singleton; + self + } + + pub fn with_project_entry_ids(mut self, project_entry_ids: &[u64]) -> Self { + self.project_entry_ids.extend( + project_entry_ids + .iter() + .copied() + .map(ProjectEntryId::from_proto), + ); + self + } + fn set_state(&mut self, state: String, cx: &mut ViewContext) { self.push_to_nav_history(cx); self.state = state;