From f142568172972391df6ecae3a07f2e883013200e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Mar 2024 15:05:08 +0200 Subject: [PATCH] Allow navigating back to multibuffers (#9334) Fixes https://github.com/orgs/zed-industries/projects/14/views/1?pane=issue&itemId=56263346 Fixes a state where Zed multi buffers were not reachable after going to an excerpt inside it (with alt-enter). I suspect, we will have to come back to multi buffer history and check the way it behaves on inner excerpts clicking, but now this change seems to restore the main thing: multi buffers not being shown in the history at all. Release Notes: - Fixes "go backwards" not considering multibuffers in history --- crates/editor/src/editor.rs | 5 +- crates/editor/src/editor_tests.rs | 298 ++++++++++++++++++++++++++++++ 2 files changed, 299 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index aa1081e509791dbe791e583d536c64254ab33784..3235ca624070385bf0b0ce0214799b6f28a8d773 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -9531,9 +9531,8 @@ impl Editor { } else { workspace.active_pane().clone() }; - pane.update(cx, |pane, _| pane.disable_history()); - for (buffer, ranges) in new_selections_by_buffer.into_iter() { + for (buffer, ranges) in new_selections_by_buffer { let editor = workspace.open_project_item::(pane.clone(), buffer, cx); editor.update(cx, |editor, cx| { editor.change_selections(Some(Autoscroll::newest()), cx, |s| { @@ -9541,8 +9540,6 @@ impl Editor { }); }); } - - pane.update(cx, |pane, _| pane.enable_history()); }) }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index f910ba69630fe7687bc1c8317274f14d2cfaacb1..44d08b23cade757d89ec06f4dce48a373bf40f64 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9309,6 +9309,304 @@ async fn test_multibuffer_reverts(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test] +async fn test_mutlibuffer_in_navigation_history(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let cols = 4; + let rows = 10; + let sample_text_1 = sample_text(rows, cols, 'a'); + assert_eq!( + sample_text_1, + "aaaa\nbbbb\ncccc\ndddd\neeee\nffff\ngggg\nhhhh\niiii\njjjj" + ); + let sample_text_2 = sample_text(rows, cols, 'l'); + assert_eq!( + sample_text_2, + "llll\nmmmm\nnnnn\noooo\npppp\nqqqq\nrrrr\nssss\ntttt\nuuuu" + ); + let sample_text_3 = sample_text(rows, cols, 'v'); + assert_eq!( + sample_text_3, + "vvvv\nwwww\nxxxx\nyyyy\nzzzz\n{{{{\n||||\n}}}}\n~~~~\n\u{7f}\u{7f}\u{7f}\u{7f}" + ); + + let buffer_1 = cx.new_model(|cx| { + Buffer::new( + 0, + BufferId::new(cx.entity_id().as_u64()).unwrap(), + sample_text_1.clone(), + ) + }); + + let buffer_2 = cx.new_model(|cx| { + Buffer::new( + 1, + BufferId::new(cx.entity_id().as_u64() + 1).unwrap(), + sample_text_2.clone(), + ) + }); + + let buffer_3 = cx.new_model(|cx| { + Buffer::new( + 2, + BufferId::new(cx.entity_id().as_u64() + 2).unwrap(), + sample_text_3.clone(), + ) + }); + + let multi_buffer = cx.new_model(|cx| { + let mut multibuffer = MultiBuffer::new(0, ReadWrite); + multibuffer.push_excerpts( + buffer_1.clone(), + [ + ExcerptRange { + context: Point::new(0, 0)..Point::new(3, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(5, 0)..Point::new(7, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(9, 0)..Point::new(10, 4), + primary: None, + }, + ], + cx, + ); + multibuffer.push_excerpts( + buffer_2.clone(), + [ + ExcerptRange { + context: Point::new(0, 0)..Point::new(3, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(5, 0)..Point::new(7, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(9, 0)..Point::new(10, 4), + primary: None, + }, + ], + cx, + ); + multibuffer.push_excerpts( + buffer_3.clone(), + [ + ExcerptRange { + context: Point::new(0, 0)..Point::new(3, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(5, 0)..Point::new(7, 0), + primary: None, + }, + ExcerptRange { + context: Point::new(9, 0)..Point::new(10, 4), + primary: None, + }, + ], + cx, + ); + multibuffer + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/a", + json!({ + "main.rs": sample_text_1, + "other.rs": sample_text_2, + "lib.rs": sample_text_3, + }), + ) + .await; + let project = Project::test(fs, ["/a".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace.deref(), cx); + let multi_buffer_editor = + cx.new_view(|cx| Editor::new(EditorMode::Full, multi_buffer, Some(project.clone()), cx)); + let multibuffer_item_id = workspace + .update(cx, |workspace, cx| { + assert!( + workspace.active_item(cx).is_none(), + "active item should be None before the first item is added" + ); + workspace.add_item_to_active_pane(Box::new(multi_buffer_editor.clone()), cx); + let active_item = workspace + .active_item(cx) + .expect("should have an active item after adding the multi buffer"); + assert!( + !active_item.is_singleton(cx), + "A multi buffer was expected to active after adding" + ); + active_item.item_id() + }) + .unwrap(); + cx.executor().run_until_parked(); + + multi_buffer_editor.update(cx, |editor, cx| { + editor.change_selections(Some(Autoscroll::Next), cx, |s| s.select_ranges(Some(1..2))); + editor.open_excerpts(&OpenExcerpts, cx); + }); + cx.executor().run_until_parked(); + let first_item_id = workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating into the 1st buffer"); + let first_item_id = active_item.item_id(); + assert_ne!( + first_item_id, multibuffer_item_id, + "Should navigate into the 1st buffer and activate it" + ); + assert!( + active_item.is_singleton(cx), + "New active item should be a singleton buffer" + ); + assert_eq!( + active_item + .act_as::(cx) + .expect("should have navigated into an editor for the 1st buffer") + .read(cx) + .text(cx), + sample_text_1 + ); + + workspace + .go_back(workspace.active_pane().downgrade(), cx) + .detach_and_log_err(cx); + + first_item_id + }) + .unwrap(); + cx.executor().run_until_parked(); + workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating back"); + assert_eq!( + active_item.item_id(), + multibuffer_item_id, + "Should navigate back to the multi buffer" + ); + assert!(!active_item.is_singleton(cx)); + }) + .unwrap(); + + multi_buffer_editor.update(cx, |editor, cx| { + editor.change_selections(Some(Autoscroll::Next), cx, |s| { + s.select_ranges(Some(39..40)) + }); + editor.open_excerpts(&OpenExcerpts, cx); + }); + cx.executor().run_until_parked(); + let second_item_id = workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating into the 2nd buffer"); + let second_item_id = active_item.item_id(); + assert_ne!( + second_item_id, multibuffer_item_id, + "Should navigate away from the multibuffer" + ); + assert_ne!( + second_item_id, first_item_id, + "Should navigate into the 2nd buffer and activate it" + ); + assert!( + active_item.is_singleton(cx), + "New active item should be a singleton buffer" + ); + assert_eq!( + active_item + .act_as::(cx) + .expect("should have navigated into an editor") + .read(cx) + .text(cx), + sample_text_2 + ); + + workspace + .go_back(workspace.active_pane().downgrade(), cx) + .detach_and_log_err(cx); + + second_item_id + }) + .unwrap(); + cx.executor().run_until_parked(); + workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating back from the 2nd buffer"); + assert_eq!( + active_item.item_id(), + multibuffer_item_id, + "Should navigate back from the 2nd buffer to the multi buffer" + ); + assert!(!active_item.is_singleton(cx)); + }) + .unwrap(); + + multi_buffer_editor.update(cx, |editor, cx| { + editor.change_selections(Some(Autoscroll::Next), cx, |s| { + s.select_ranges(Some(60..70)) + }); + editor.open_excerpts(&OpenExcerpts, cx); + }); + cx.executor().run_until_parked(); + workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating into the 3rd buffer"); + let third_item_id = active_item.item_id(); + assert_ne!( + third_item_id, multibuffer_item_id, + "Should navigate into the 3rd buffer and activate it" + ); + assert_ne!(third_item_id, first_item_id); + assert_ne!(third_item_id, second_item_id); + assert!( + active_item.is_singleton(cx), + "New active item should be a singleton buffer" + ); + assert_eq!( + active_item + .act_as::(cx) + .expect("should have navigated into an editor") + .read(cx) + .text(cx), + sample_text_3 + ); + + workspace + .go_back(workspace.active_pane().downgrade(), cx) + .detach_and_log_err(cx); + }) + .unwrap(); + cx.executor().run_until_parked(); + workspace + .update(cx, |workspace, cx| { + let active_item = workspace + .active_item(cx) + .expect("should have an active item after navigating back from the 3rd buffer"); + assert_eq!( + active_item.item_id(), + multibuffer_item_id, + "Should navigate back from the 3rd buffer to the multi buffer" + ); + assert!(!active_item.is_singleton(cx)); + }) + .unwrap(); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point