From 4c7244790a075e862eeb4e5ccc12d6c8f5da6f7e Mon Sep 17 00:00:00 2001 From: Arnesh Date: Tue, 18 Aug 2026 23:00:25 +0000 Subject: [PATCH] workspace: Activate the right tab when restoring a workspace (#62844) @SomeoneToIgnore this is the follow up you asked for in #62692, done as discussed. `deserialize_to` keeps a `None` in `items` for every item that failed to deserialize, and those are never added to the pane. Any later tab therefore sits at a lower index in the pane than the one it was serialized with, so activating and previewing by serialized index lands on the tab that shifted into that slot. When the failing item is the last one, the index points past the end of the pane and nothing is activated or previewed. The serialized index is now mapped to the pane's index by counting the items before it that actually restored, and an index whose own item failed to restore is skipped. Closes #62843 Release Notes: - Fixed the wrong tab being activated when restoring a workspace containing items that fail to open --------- Co-authored-by: Kirill Bulatov --- crates/workspace/src/persistence/model.rs | 12 ++-- crates/workspace/src/workspace.rs | 76 +++++++++++++++++++++-- 2 files changed, 78 insertions(+), 10 deletions(-) diff --git a/crates/workspace/src/persistence/model.rs b/crates/workspace/src/persistence/model.rs index d3b0f6e971b..03e0e055f71 100644 --- a/crates/workspace/src/persistence/model.rs +++ b/crates/workspace/src/persistence/model.rs @@ -395,17 +395,17 @@ impl SerializedPane { } } - if let Some(active_item_index) = active_item_index { + if let Some(active_item) = active_item_index.and_then(|index| items.get(index)?.clone()) { pane.update_in(cx, |pane, window, cx| { - pane.activate_item(active_item_index, false, false, window, cx); + if let Some(index) = pane.index_for_item(active_item.as_ref()) { + pane.activate_item(index, false, false, window, cx); + } })?; } - if let Some(preview_item_index) = preview_item_index { + if let Some(preview_item) = preview_item_index.and_then(|index| items.get(index)?.clone()) { pane.update(cx, |pane, cx| { - if let Some(item) = pane.item_for_index(preview_item_index) { - pane.set_preview_item_id(Some(item.item_id()), cx); - } + pane.set_preview_item_id(Some(preview_item.item_id()), cx); })?; } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 3d421f4e883..d6c959d7812 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -16751,7 +16751,7 @@ mod tests { // Items whose kind has no registered descriptor always fail to deserialize, // which is what happens to a real item when its file or serialized state is // gone by the time the workspace is restored. - let (items_len, pinned_count) = restore_pane( + let pane = restore_pane( &workspace, SerializedPane::new( vec![ @@ -16766,13 +16766,15 @@ mod tests { cx, ) .await; + let (items_len, pinned_count) = + pane.read_with(cx, |pane, _| (pane.items_len(), pane.pinned_count())); assert_eq!(items_len, 3); assert_eq!( pinned_count, 1, "only the pinned item that was restored should stay pinned" ); - let (items_len, pinned_count) = restore_pane( + let pane = restore_pane( &workspace, SerializedPane::new( vec![ @@ -16786,6 +16788,8 @@ mod tests { cx, ) .await; + let (items_len, pinned_count) = + pane.read_with(cx, |pane, _| (pane.items_len(), pane.pinned_count())); assert_eq!(items_len, 2); assert_eq!( pinned_count, 2, @@ -16793,6 +16797,70 @@ mod tests { ); } + #[gpui::test] + async fn test_restoring_active_and_preview_tabs_when_items_fail_to_deserialize( + cx: &mut TestAppContext, + ) { + init_test(cx); + cx.update(|cx| { + register_serializable_item::(cx); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", json!({ "test.txt": "" })).await; + let project = Project::test(fs, ["root".as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + // The active and preview tabs both sit behind an item that fails to restore, so + // their serialized indices are one ahead of where those tabs end up. + let pane = restore_pane( + &workspace, + SerializedPane::new( + vec![ + SerializedItem::new("Unrestorable", 1, false, false), + SerializedItem::new("TestItem", 2, true, false), + SerializedItem::new("TestItem", 3, false, true), + ], + true, + 0, + ), + cx, + ) + .await; + pane.read_with(cx, |pane, _| { + assert_eq!(pane.items_len(), 2); + assert_eq!(pane.active_item_index(), 0); + assert_eq!( + pane.preview_item_id(), + pane.item_for_index(1).map(|item| item.item_id()), + "the preview tab should follow the item it was serialized with" + ); + }); + + // The active item itself fails to restore, so nothing should be activated in its + // place and the pane should keep the tab it settled on. + let pane = restore_pane( + &workspace, + SerializedPane::new( + vec![ + SerializedItem::new("TestItem", 1, false, false), + SerializedItem::new("Unrestorable", 2, true, true), + SerializedItem::new("TestItem", 3, false, false), + ], + true, + 0, + ), + cx, + ) + .await; + pane.read_with(cx, |pane, _| { + assert_eq!(pane.items_len(), 2); + assert_eq!(pane.active_item_index(), 1); + assert_eq!(pane.preview_item_id(), None); + }); + } + #[gpui::test] async fn test_serializing_pinned_tabs_when_items_are_not_serializable(cx: &mut TestAppContext) { init_test(cx); @@ -16868,7 +16936,7 @@ mod tests { workspace: &Entity, serialized_pane: SerializedPane, cx: &mut VisualTestContext, - ) -> (usize, usize) { + ) -> Entity { let (pane, task) = workspace.update_in(cx, |workspace, window, cx| { let pane = workspace.add_pane(window, cx); let weak_pane = pane.downgrade(); @@ -16888,7 +16956,7 @@ mod tests { (pane, task) }); task.await.unwrap(); - pane.read_with(cx, |pane, _| (pane.items_len(), pane.pinned_count())) + pane } mod register_project_item_tests {