From f7f3b90dca57c9e4f54befa4cfad3e05138e8e4e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 21 Aug 2026 17:26:29 +0000 Subject: [PATCH] Register terminal panel before restoring serialized terminals (#62712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/zed-industries/zed/issues/62435 Part of https://github.com/zed-industries/zed/issues/60548 Before: https://github.com/user-attachments/assets/6b5147da-44cb-40c7-9da5-c50145e2ab4e After: https://github.com/user-attachments/assets/c58423cc-ea99-40d9-ad18-23b31dfb8e5d Previously, `TerminalPanel::load` awaited full deserialization of serialized terminals (each awaiting directory environment capture via a login shell) before the panel was registered in the dock. Until then, `terminal_panel::Toggle{Focus}` was a silent no-op: the workspace finds no panel in any dock, the keystroke is eaten, and nothing happens. When it got eventually restored, it would close whatever panel in the dock was open manually instead, wrecking the workflow. Repro: 1. Add `case "$ZSH_EXECUTION_STRING" in *--printenv*) sleep 10;; esac` to `~/.zshrc`. 2. Open a project, open at least one terminal in the panel, quit Zed (Cmd+Q). 3. Relaunch via Dock/Spotlight (not the `zed` CLI, which skips env capture via `cli_environment`). 4. Press the terminal shortcut within 10s: nothing happens; after ~10s the panel appears and the shortcut works. * `Register terminal panel before restoring serialized terminals` — the panel is now created and registered immediately, and restoration runs in panel-owned tasks (cancelled on panel drop, nothing detached). Terminal-adding paths were initially queued behind restoration (later replaced by grafting, see below); the default shell spawns only if the panel ends up active and empty after restore. * `Stop replaying serialized dock state over manual panel changes` — fixes a related race: `Dock::add_panel` replayed the serialized dock state (active panel, visibility, zoom) on every panel insertion, so a late-loading panel would re-activate the serialized panel over one the user had switched to in the meantime (e.g. the terminal panel yanking the dock away from the agent panel), or re-open a dock the user had closed. Serialized state is now applied at most once and is discarded as soon as the user changes the dock state manually. The new tests fail without this change and pass with it. * `Show restoration progress in the terminal panel and allow spawning terminals during it` — while terminals restore, the panel shows a spinner with "Restoring terminals…" instead of a blank pane, and logs the restored count and elapsed time. Supersedes the queuing from the first commit: new terminals spawn immediately, and any that land mid-restore are grafted into the restored layout instead of being lost. Serialization is paused during restoration, so quitting mid-restore can no longer persist a partial tab list and silently drop terminals. Release Notes: - Fixed the terminal panel not opening at all under certain circumstances, made panels no longer switching back to the serialized one when opened during startup. --- crates/terminal_view/src/persistence.rs | 63 +- crates/terminal_view/src/terminal_panel.rs | 850 +++++++++++++++++---- crates/terminal_view/src/terminal_view.rs | 5 + crates/workspace/src/dock.rs | 125 ++- crates/workspace/src/workspace.rs | 364 ++++++++- crates/zed/src/zed.rs | 6 +- 6 files changed, 1223 insertions(+), 190 deletions(-) diff --git a/crates/terminal_view/src/persistence.rs b/crates/terminal_view/src/persistence.rs index 5c1e659b0f..3dfa5bbb4f 100644 --- a/crates/terminal_view/src/persistence.rs +++ b/crates/terminal_view/src/persistence.rs @@ -15,8 +15,8 @@ use db::{ sqlez_macros::sql, }; use workspace::{ - ItemHandle, ItemId, Member, Pane, PaneAxis, PaneGroup, SerializableItem as _, Workspace, - WorkspaceDb, WorkspaceId, + ItemHandle, ItemId, Member, Pane, PaneAxis, PaneGroup, SerializableItem as _, SplitDirection, + Workspace, WorkspaceDb, WorkspaceId, }; use crate::{ @@ -92,14 +92,12 @@ pub(crate) fn deserialize_terminal_panel( project: Entity, database_id: WorkspaceId, serialized_panel: SerializedTerminalPanel, + terminal_panel: WeakEntity, window: &mut Window, cx: &mut App, -) -> Task>> { +) -> Task> { window.spawn(cx, async move |cx| { - let terminal_panel = workspace.update_in(cx, |workspace, window, cx| { - cx.new(|cx| TerminalPanel::new(workspace, window, cx)) - })?; - match &serialized_panel.items { + let restored_items = match &serialized_panel.items { SerializedItems::NoSplits(item_ids) => { let items = deserialize_terminal_views( database_id, @@ -109,12 +107,14 @@ pub(crate) fn deserialize_terminal_panel( cx, ) .await; + let restored_items = items.len(); let active_item = serialized_panel.active_item_id; terminal_panel.update_in(cx, |terminal_panel, window, cx| { terminal_panel.active_pane.update(cx, |pane, cx| { populate_pane_items(pane, items, active_item, window, cx); }); })?; + restored_items } SerializedItems::WithSplits(serialized_pane_group) => { let center_pane = deserialize_pane_group( @@ -127,16 +127,48 @@ pub(crate) fn deserialize_terminal_panel( ) .await; if let Some((center_group, active_pane)) = center_pane { - terminal_panel.update(cx, |terminal_panel, _| { + terminal_panel.update_in(cx, |terminal_panel, window, cx| { + let interim_panes = terminal_panel + .center + .panes() + .into_iter() + .filter(|pane| pane.read(cx).items_len() > 0) + .cloned() + .collect::>(); + let focused_interim_pane = interim_panes + .iter() + .find(|pane| pane.read(cx).has_focus(window, cx)) + .cloned(); terminal_panel.center = PaneGroup::with_root(center_group); terminal_panel.active_pane = active_pane.unwrap_or_else(|| terminal_panel.center.first_pane()); - }); + let restored_items = terminal_panel + .center + .panes() + .into_iter() + .map(|pane| pane.read(cx).items_len()) + .sum::(); + let restored_pane = terminal_panel.active_pane.clone(); + for interim_pane in &interim_panes { + terminal_panel.center.split( + &restored_pane, + interim_pane, + SplitDirection::Right, + cx, + ); + } + if let Some(focused_interim_pane) = focused_interim_pane { + terminal_panel.active_pane = focused_interim_pane; + } + restored_items + })? + } else { + 0 } } - } + }; - Ok(terminal_panel) + Ok(restored_items) }) } @@ -147,6 +179,7 @@ fn populate_pane_items( window: &mut Window, cx: &mut Context, ) { + let interim_active_item = (pane.items_len() > 0).then(|| pane.active_item()).flatten(); let mut active_item_index = None; for (item_index, item) in (pane.items_len()..).zip(items) { if Some(item.item_id().as_u64()) == active_item { @@ -154,7 +187,11 @@ fn populate_pane_items( } pane.add_item(Box::new(item), false, false, None, window, cx); } - if let Some(index) = active_item_index { + if let Some(interim_active_item) = interim_active_item { + if let Some(index) = pane.index_for_item(interim_active_item.as_ref()) { + pane.activate_item(index, false, false, window, cx); + } + } else if let Some(index) = active_item_index { pane.activate_item(index, false, false, window, cx); } } @@ -163,7 +200,7 @@ fn populate_pane_items( async fn deserialize_pane_group( workspace: WeakEntity, project: Entity, - panel: Entity, + panel: WeakEntity, workspace_id: WorkspaceId, serialized: &SerializedPaneGroup, cx: &mut AsyncWindowContext, diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index f24561992e..ec6fcf9606 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -22,10 +22,10 @@ use settings::{Settings, TerminalDockPosition}; use task::{RevealStrategy, RevealTarget, Shell, ShellBuilder, SpawnInTerminal, TaskId}; use terminal::{Terminal, terminal_settings::TerminalSettings}; use ui::{ - ButtonLike, Clickable, ContextMenu, FluentBuilder, PopoverMenu, SplitButton, Toggleable, - Tooltip, prelude::*, + ButtonLike, Clickable, CommonAnimationExt, ContextMenu, FluentBuilder, PopoverMenu, + SplitButton, Toggleable, Tooltip, prelude::*, }; -use util::{ResultExt, TryFutureExt}; +use util::{ResultExt, TryFutureExt, defer}; use workspace::{ ActivateNextPane, ActivatePane, ActivatePaneDown, ActivatePaneLeft, ActivatePaneRight, ActivatePaneUp, ActivatePreviousPane, DraggedTab, ItemId, MoveItemToPane, @@ -82,6 +82,8 @@ pub struct TerminalPanel { workspace: WeakEntity, pending_serialization: Task>, pending_terminals_to_add: usize, + restoring: bool, + _restoration: Task<()>, deferred_tasks: HashMap>, assistant_enabled: bool, active: bool, @@ -100,6 +102,8 @@ impl TerminalPanel { workspace: workspace.weak_handle(), pending_serialization: Task::ready(None), pending_terminals_to_add: 0, + restoring: false, + _restoration: Task::ready(()), deferred_tasks: HashMap::default(), assistant_enabled: false, active: false, @@ -234,10 +238,84 @@ impl TerminalPanel { workspace: WeakEntity, mut cx: AsyncWindowContext, ) -> Result> { - let mut terminal_panel = None; + let terminal_panel = workspace.update_in(&mut cx, |workspace, window, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + })?; + workspace + .update(&mut cx, |workspace, _| { + workspace.set_terminal_provider(TerminalProvider(terminal_panel.clone())) + }) + .ok(); + + terminal_panel.update_in(&mut cx, |panel, window, cx| { + panel.restoring = true; + panel._restoration = cx.spawn_in(window, { + let workspace = workspace.clone(); + async move |terminal_panel, cx| { + let restored = + Self::restore_serialized_state(workspace, terminal_panel.clone(), cx) + .await + .log_err() + .unwrap_or(false); + let default_shell_task = terminal_panel + .update_in(cx, |terminal_panel, window, cx| { + terminal_panel.finish_restoration(restored, window, cx) + }) + .ok() + .flatten(); + if let Some(task) = default_shell_task { + task.await.log_err(); + } + } + }); + })?; + + Ok(terminal_panel) + } + + fn finish_restoration( + &mut self, + restored: bool, + window: &mut Window, + cx: &mut Context, + ) -> Option>>> { + self.restoring = false; + let has_terminals = self + .center + .panes() + .into_iter() + .any(|pane| pane.read(cx).items_len() > 0); + if restored || has_terminals { + self.serialize(cx); + } + cx.notify(); + if self.active && self.has_no_terminals(cx) { + let working_directory = self + .workspace + .update(cx, |workspace, cx| default_working_directory(workspace, cx)) + .ok() + .flatten(); + Some(self.add_terminal_shell( + false, + working_directory, + RevealStrategy::Always, + window, + cx, + )) + } else { + None + } + } + + async fn restore_serialized_state( + workspace: WeakEntity, + terminal_panel: WeakEntity, + cx: &mut AsyncWindowContext, + ) -> Result { + let mut restored = false; if let Some((database_id, serialization_key, kvp)) = workspace - .read_with(&cx, |workspace, cx| { + .read_with(cx, |workspace, cx| { workspace .database_id() .zip(TerminalPanel::serialization_key(workspace)) @@ -254,76 +332,96 @@ impl TerminalPanel { .transpose() .log_err() .flatten() - && let Ok(serialized) = workspace - .update_in(&mut cx, |workspace, window, cx| { + { + let started_at = std::time::Instant::now(); + let deserialized = workspace + .update_in(cx, |workspace, window, cx| { deserialize_terminal_panel( workspace.weak_handle(), workspace.project().clone(), database_id, serialized_panel, + terminal_panel.clone(), window, cx, ) })? - .await - { - terminal_panel = Some(serialized); - } - - let terminal_panel = if let Some(panel) = terminal_panel { - panel - } else { - workspace.update_in(&mut cx, |workspace, window, cx| { - cx.new(|cx| TerminalPanel::new(workspace, window, cx)) - })? - }; - - if let Some(workspace) = workspace.upgrade() { - workspace.update(&mut cx, |workspace, _| { - workspace.set_terminal_provider(TerminalProvider(terminal_panel.clone())) - }); + .await; + if let Some(restored_terminals) = deserialized.log_err() { + restored = restored_terminals > 0; + log::debug!( + "terminal panel: restored {restored_terminals} serialized terminal(s) in {:?}", + started_at.elapsed() + ); + } } // Since panels/docks are loaded outside from the workspace, we cleanup here, instead of through the workspace. - if let Some(workspace) = workspace.upgrade() { - let cleanup_task = workspace.update_in(&mut cx, |workspace, window, cx| { - let alive_item_ids = terminal_panel + let cleanup = workspace.update_in(cx, |workspace, window, cx| { + let alive_item_ids = terminal_panel.upgrade().map(|terminal_panel| { + terminal_panel .read(cx) .center .panes() .into_iter() .flat_map(|pane| pane.read(cx).items()) .map(|item| item.item_id().as_u64() as ItemId) - .collect(); - workspace.database_id().map(|workspace_id| { - TerminalView::cleanup(workspace_id, alive_item_ids, window, cx) + .collect::>() + }); + alive_item_ids + .zip(workspace.database_id()) + .map(|(alive_item_ids, workspace_id)| { + let cleanup_task = + TerminalView::cleanup(workspace_id, alive_item_ids.clone(), window, cx); + (cleanup_task, alive_item_ids) }) - })?; - if let Some(task) = cleanup_task { - task.await.log_err(); - } + })?; + if let Some((cleanup_task, alive_item_ids)) = cleanup { + cleanup_task.await.log_err(); + terminal_panel + .update(cx, |terminal_panel, cx| { + let terminals_to_reserialize = terminal_panel + .center + .panes() + .into_iter() + .flat_map(|pane| { + pane.read(cx) + .items() + .filter(|item| { + !alive_item_ids.contains(&(item.item_id().as_u64() as ItemId)) + }) + .filter_map(|item| item.act_as::(cx)) + .collect::>() + }) + .collect::>(); + for terminal_view in terminals_to_reserialize { + terminal_view.update(cx, |terminal_view, cx| { + terminal_view.mark_needs_serialize(cx) + }); + } + }) + .ok(); } - if let Some(workspace) = workspace.upgrade() { - let should_focus = workspace - .update_in(&mut cx, |workspace, window, cx| { + let should_focus = workspace + .update_in(cx, |workspace, window, cx| { + terminal_panel.upgrade().is_some_and(|terminal_panel| { workspace.active_item(cx).is_none() && workspace .is_dock_at_position_open(terminal_panel.position(window, cx), cx) }) - .unwrap_or(false); - - if should_focus { - terminal_panel - .update_in(&mut cx, |panel, window, cx| { - panel.active_pane.update(cx, |pane, cx| { - pane.focus_active_item(window, cx); - }); - }) - .ok(); - } + }) + .unwrap_or(false); + if should_focus { + terminal_panel + .update_in(cx, |panel, window, cx| { + panel.active_pane.update(cx, |pane, cx| { + pane.focus_active_item(window, cx); + }); + }) + .ok(); } - Ok(terminal_panel) + Ok(restored) } fn handle_pane_event( @@ -519,16 +617,13 @@ impl TerminalPanel { terminal_panel .update(cx, |panel, cx| { - if action.local { - panel.add_local_terminal_shell(RevealStrategy::Always, window, cx) - } else { - panel.add_terminal_shell( - Some(action.working_directory.clone()), - RevealStrategy::Always, - window, - cx, - ) - } + panel.add_terminal_shell( + action.local, + Some(action.working_directory.clone()), + RevealStrategy::Always, + window, + cx, + ) }) .detach_and_log_err(cx); } @@ -670,16 +765,13 @@ impl TerminalPanel { terminal_panel .update(cx, |this, cx| { - if action.local { - this.add_local_terminal_shell(RevealStrategy::Always, window, cx) - } else { - this.add_terminal_shell( - default_working_directory(workspace, cx), - RevealStrategy::Always, - window, - cx, - ) - } + this.add_terminal_shell( + action.local, + default_working_directory(workspace, cx), + RevealStrategy::Always, + window, + cx, + ) }) .detach_and_log_err(cx); } @@ -793,19 +885,17 @@ impl TerminalPanel { cx: &mut Context, ) -> Task>> { let workspace = self.workspace.clone(); - cx.spawn_in(window, async move |terminal_panel, cx| { + self.spawn_pending_terminal(window, cx, async move |terminal_panel, cx| { if workspace.update(cx, |workspace, cx| !is_enabled_in_workspace(workspace, cx))? { anyhow::bail!("terminal not yet supported for remote projects"); } - let pane = terminal_panel.update(cx, |terminal_panel, _| { - terminal_panel.pending_terminals_to_add += 1; - terminal_panel.active_pane.clone() - })?; let project = workspace.read_with(cx, |workspace, _| workspace.project().clone())?; let terminal = project .update(cx, |project, cx| project.create_terminal_task(task, cx)) .await?; - let result = workspace.update_in(cx, |workspace, window, cx| { + let pane = terminal_panel + .read_with(cx, |terminal_panel, _| terminal_panel.active_pane.clone())?; + workspace.update_in(cx, |workspace, window, cx| { let terminal_view = Box::new(cx.new(|cx| { TerminalView::new( terminal.clone(), @@ -828,41 +918,16 @@ impl TerminalPanel { } pane.update(cx, |pane, cx| { - let focus = matches!(reveal_strategy, RevealStrategy::Always); + let focus = reveal_strategy == RevealStrategy::Always; pane.add_item(terminal_view, true, focus, None, window, cx); }); - Ok(terminal.downgrade()) - })?; - terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.pending_terminals_to_add = - terminal_panel.pending_terminals_to_add.saturating_sub(1); - terminal_panel.serialize(cx) - })?; - result + terminal.downgrade() + }) }) } fn add_terminal_shell( - &mut self, - cwd: Option, - reveal_strategy: RevealStrategy, - window: &mut Window, - cx: &mut Context, - ) -> Task>> { - self.add_terminal_shell_internal(false, cwd, reveal_strategy, window, cx) - } - - fn add_local_terminal_shell( - &mut self, - reveal_strategy: RevealStrategy, - window: &mut Window, - cx: &mut Context, - ) -> Task>> { - self.add_terminal_shell_internal(true, None, reveal_strategy, window, cx) - } - - fn add_terminal_shell_internal( &mut self, force_local: bool, cwd: Option, @@ -871,15 +936,10 @@ impl TerminalPanel { cx: &mut Context, ) -> Task>> { let workspace = self.workspace.clone(); - - cx.spawn_in(window, async move |terminal_panel, cx| { + self.spawn_pending_terminal(window, cx, async move |terminal_panel, cx| { if workspace.update(cx, |workspace, cx| !is_enabled_in_workspace(workspace, cx))? { anyhow::bail!("terminal not yet supported for collaborative projects"); } - let pane = terminal_panel.update(cx, |terminal_panel, _| { - terminal_panel.pending_terminals_to_add += 1; - terminal_panel.active_pane.clone() - })?; let project = workspace.read_with(cx, |workspace, _| workspace.project().clone())?; let terminal = if force_local { project @@ -891,44 +951,38 @@ impl TerminalPanel { .await }; + let pane = terminal_panel + .read_with(cx, |terminal_panel, _| terminal_panel.active_pane.clone())?; match terminal { - Ok(terminal) => { - let result = workspace.update_in(cx, |workspace, window, cx| { - let terminal_view = Box::new(cx.new(|cx| { - TerminalView::new( - terminal.clone(), - workspace.weak_handle(), - workspace.database_id(), - workspace.project().downgrade(), - window, - cx, - ) - })); + Ok(terminal) => workspace.update_in(cx, |workspace, window, cx| { + let terminal_view = Box::new(cx.new(|cx| { + TerminalView::new( + terminal.clone(), + workspace.weak_handle(), + workspace.database_id(), + workspace.project().downgrade(), + window, + cx, + ) + })); - match reveal_strategy { - RevealStrategy::Always => { - workspace.focus_panel::(window, cx); - } - RevealStrategy::NoFocus => { - workspace.open_panel::(window, cx); - } - RevealStrategy::Never => {} + match reveal_strategy { + RevealStrategy::Always => { + workspace.focus_panel::(window, cx); } + RevealStrategy::NoFocus => { + workspace.open_panel::(window, cx); + } + RevealStrategy::Never => {} + } - pane.update(cx, |pane, cx| { - let focus = matches!(reveal_strategy, RevealStrategy::Always); - pane.add_item(terminal_view, true, focus, None, window, cx); - }); + pane.update(cx, |pane, cx| { + let focus = reveal_strategy == RevealStrategy::Always; + pane.add_item(terminal_view, true, focus, None, window, cx); + }); - Ok(terminal.downgrade()) - })?; - terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.pending_terminals_to_add = - terminal_panel.pending_terminals_to_add.saturating_sub(1); - terminal_panel.serialize(cx) - })?; - result - } + terminal.downgrade() + }), Err(error) => { pane.update_in(cx, |pane, window, cx| { let focus = pane.has_focus(window, cx); @@ -944,7 +998,56 @@ impl TerminalPanel { }) } + fn spawn_pending_terminal( + &mut self, + window: &mut Window, + cx: &mut Context, + create_terminal: impl AsyncFnOnce( + WeakEntity, + &mut AsyncWindowContext, + ) -> Result> + + 'static, + ) -> Task>> { + self.pending_terminals_to_add += 1; + cx.notify(); + let decrement_when_cancelled = defer({ + let terminal_panel = cx.weak_entity(); + let cx = cx.to_async(); + move || { + cx.spawn(async move |cx| { + terminal_panel + .update(cx, |terminal_panel, cx| { + terminal_panel.finish_pending_terminal(cx) + }) + .ok(); + }) + .detach(); + } + }); + cx.spawn_in(window, async move |terminal_panel, cx| { + let result = create_terminal(terminal_panel.clone(), cx).await; + decrement_when_cancelled.abort(); + terminal_panel + .update(cx, |terminal_panel, cx| { + terminal_panel.finish_pending_terminal(cx); + if result.is_ok() { + terminal_panel.serialize(cx); + } + }) + .ok(); + result + }) + } + + fn finish_pending_terminal(&mut self, cx: &mut Context) { + self.pending_terminals_to_add = self.pending_terminals_to_add.saturating_sub(1); + cx.notify(); + } + fn serialize(&mut self, cx: &mut Context) { + if self.restoring { + return; + } let Some(serialization_key) = self .workspace .read_with(cx, |workspace, _| { @@ -1372,11 +1475,37 @@ impl Render for TerminalPanel { (callbacks.wrap_div_with_search_actions)(div(), self.active_pane.clone()) }) .unwrap_or_else(div); + let no_items_in_panes = self + .center + .panes() + .into_iter() + .all(|pane| pane.read(cx).items_len() == 0); + let waiting_for_terminals = self.restoring || self.pending_terminals_to_add > 0; + let restoring_placeholder = (waiting_for_terminals && no_items_in_panes).then(|| { + let label = if self.restoring { + "Restoring terminals…" + } else { + "Starting terminal…" + }; + h_flex() + .absolute() + .inset_0() + .justify_center() + .gap_2() + .child( + Icon::new(IconName::ArrowCircle) + .color(Color::Muted) + .size(IconSize::Small) + .with_rotate_animation(2), + ) + .child(Label::new(label).color(Color::Muted)) + }); self.workspace .update(cx, |workspace, cx| { registrar .track_focus(&self.focus_handle) .size_full() + .relative() .child(self.center.render( workspace.zoomed_item(), None, @@ -1391,6 +1520,7 @@ impl Render for TerminalPanel { window, cx, )) + .children(restoring_placeholder) }) .ok() .map(|div| { @@ -1612,7 +1742,7 @@ impl Panel for TerminalPanel { fn set_active(&mut self, active: bool, window: &mut Window, cx: &mut Context) { let old_active = self.active; self.active = active; - if !active || old_active == active || !self.has_no_terminals(cx) { + if !active || old_active == active || self.restoring || !self.has_no_terminals(cx) { return; } cx.defer_in(window, |this, window, cx| { @@ -1623,7 +1753,7 @@ impl Panel for TerminalPanel { return; }; - this.add_terminal_shell(kind, RevealStrategy::Always, window, cx) + this.add_terminal_shell(false, kind, RevealStrategy::Always, window, cx) .detach_and_log_err(cx) }) } @@ -1744,11 +1874,12 @@ mod tests { use std::num::NonZero; use super::*; + use crate::persistence::{SerializedPane, SerializedPaneGroup}; use gpui::{Modifiers, TestAppContext, UpdateGlobal as _, VisualTestContext}; use pretty_assertions::assert_eq; use project::FakeFs; use settings::SettingsStore; - use workspace::MultiWorkspace; + use workspace::{MultiWorkspace, WorkspaceId}; #[test] fn test_prepare_empty_task() { @@ -1797,7 +1928,7 @@ mod tests { let task = window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |panel, cx| { - panel.add_terminal_shell(None, RevealStrategy::Always, window, cx) + panel.add_terminal_shell(false, None, RevealStrategy::Always, window, cx) }) }) .unwrap(); @@ -1880,7 +2011,13 @@ mod tests { window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.add_terminal_shell(None, RevealStrategy::Always, window, cx) + terminal_panel.add_terminal_shell( + false, + None, + RevealStrategy::Always, + window, + cx, + ) }) }) .unwrap() @@ -1898,11 +2035,416 @@ mod tests { .any(|item| item.downcast::().is_some()), "should spawn `FailedToSpawnTerminal` pane" ); + assert_eq!(terminal_panel.pending_terminals_to_add, 0); }) }) .unwrap(); } + #[gpui::test] + async fn test_failed_task_spawn_does_not_leak_pending_terminal_count(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + terminal_panel.add_terminal_task( + SpawnInTerminal { + command: Some("__nonexistent_program__".to_owned()), + ..SpawnInTerminal::default() + }, + RevealStrategy::Never, + window, + cx, + ) + }) + }) + .unwrap() + .await + .unwrap_err(); + + cx.run_until_parked(); + terminal_panel.read_with(cx, |terminal_panel, cx| { + assert_eq!(terminal_panel.pending_terminals_to_add, 0); + assert_eq!(terminal_panel.active_pane.read(cx).items_len(), 0); + }); + } + + #[gpui::test] + async fn test_pending_terminal_count_tracks_spawn_lifecycle(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + let task = window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + let task = + terminal_panel.add_terminal_shell(false, None, RevealStrategy::Never, window, cx); + assert_eq!( + terminal_panel.pending_terminals_to_add, 1, + "pending count should be incremented synchronously to avoid double default terminal spawns" + ); + assert!(!terminal_panel.has_no_terminals(cx)); + task + }) + }) + .unwrap(); + task.await.unwrap(); + + cx.run_until_parked(); + terminal_panel.read_with(cx, |terminal_panel, cx| { + assert_eq!(terminal_panel.pending_terminals_to_add, 0); + assert_eq!(terminal_panel.active_pane.read(cx).items_len(), 1); + }); + } + + #[gpui::test] + async fn test_pending_terminal_count_resets_when_spawn_cancelled(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + let task = terminal_panel.add_terminal_shell( + false, + None, + RevealStrategy::Never, + window, + cx, + ); + assert_eq!(terminal_panel.pending_terminals_to_add, 1); + drop(task); + }) + }) + .unwrap(); + + cx.run_until_parked(); + terminal_panel.read_with(cx, |terminal_panel, _| { + assert_eq!(terminal_panel.pending_terminals_to_add, 0); + }); + } + + #[gpui::test] + async fn test_load_without_serialized_state_does_not_persist_empty_state( + cx: &mut TestAppContext, + ) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let serialization_key = window_handle + .update(cx, |multi_workspace, _, cx| { + TerminalPanel::serialization_key(multi_workspace.workspace().read(cx)).unwrap() + }) + .unwrap(); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + let workspace = multi_workspace.workspace().downgrade(); + window.spawn(cx, async move |cx| { + TerminalPanel::load(workspace, cx.clone()).await + }) + }) + .unwrap() + .await + .unwrap(); + + cx.executor().advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + + terminal_panel.read_with(cx, |terminal_panel, cx| { + assert!(!terminal_panel.restoring); + assert_eq!(terminal_panel.active_pane.read(cx).items_len(), 0); + }); + let serialized_state = cx + .update(|cx| KeyValueStore::global(cx)) + .read_kvp(&serialization_key) + .unwrap(); + assert_eq!(serialized_state, None); + } + + #[gpui::test] + async fn test_terminal_added_during_restore_is_serialized_after_restore( + cx: &mut TestAppContext, + ) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let serialization_key = window_handle + .update(cx, |multi_workspace, _, cx| { + TerminalPanel::serialization_key(multi_workspace.workspace().read(cx)).unwrap() + }) + .unwrap(); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + terminal_panel.restoring = true; + terminal_panel.add_terminal_shell( + false, + None, + RevealStrategy::Never, + window, + cx, + ) + }) + }) + .unwrap() + .await + .unwrap(); + + cx.executor().advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + let suppressed_state = cx + .update(|cx| KeyValueStore::global(cx)) + .read_kvp(&serialization_key) + .unwrap(); + assert_eq!( + suppressed_state, None, + "serialization should stay suppressed while the panel is restoring" + ); + + let default_shell_task = window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + terminal_panel.finish_restoration(false, window, cx) + }) + }) + .unwrap(); + assert!( + default_shell_task.is_none(), + "no default terminal should spawn when a terminal already exists" + ); + + cx.executor().advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + let serialized_state = cx + .update(|cx| KeyValueStore::global(cx)) + .read_kvp(&serialization_key) + .unwrap(); + assert!( + serialized_state.is_some(), + "terminal added during restore must be serialized once restoration finishes" + ); + } + + #[gpui::test] + async fn test_legacy_serialized_restore_keeps_interim_terminal_active(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + terminal_panel.add_terminal_shell( + false, + None, + RevealStrategy::Never, + window, + cx, + ) + }) + }) + .unwrap() + .await + .unwrap(); + let interim_item_id = terminal_panel.read_with(cx, |terminal_panel, cx| { + let pane = terminal_panel.active_pane.read(cx); + assert_eq!(pane.items_len(), 1); + pane.active_item().unwrap().item_id() + }); + + let restored_items = window_handle + .update(cx, |multi_workspace, window, cx| { + let workspace = multi_workspace.workspace().clone(); + let project = workspace.read(cx).project().clone(); + deserialize_terminal_panel( + workspace.downgrade(), + project, + WorkspaceId::default(), + SerializedTerminalPanel { + items: SerializedItems::NoSplits(vec![12345]), + active_item_id: Some(12345), + }, + terminal_panel.downgrade(), + window, + cx, + ) + }) + .unwrap() + .await + .unwrap(); + cx.run_until_parked(); + + assert_eq!(restored_items, 1); + terminal_panel.read_with(cx, |terminal_panel, cx| { + let pane = terminal_panel.active_pane.read(cx); + assert_eq!(pane.items_len(), 2); + assert_eq!( + pane.active_item().map(|item| item.item_id()), + Some(interim_item_id), + "interim terminal must stay active after a legacy format restore" + ); + }); + } + + #[gpui::test] + async fn test_split_restore_grafts_interim_pane(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + + let terminal_panel = window_handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + cx.new(|cx| TerminalPanel::new(workspace, window, cx)) + }) + }) + .unwrap(); + + window_handle + .update(cx, |_, window, cx| { + terminal_panel.update(cx, |terminal_panel, cx| { + terminal_panel.add_terminal_shell( + false, + None, + RevealStrategy::Never, + window, + cx, + ) + }) + }) + .unwrap() + .await + .unwrap(); + let interim_pane = + terminal_panel.read_with(cx, |terminal_panel, _| terminal_panel.active_pane.clone()); + + let restored_items = window_handle + .update(cx, |multi_workspace, window, cx| { + let workspace = multi_workspace.workspace().clone(); + let project = workspace.read(cx).project().clone(); + deserialize_terminal_panel( + workspace.downgrade(), + project, + WorkspaceId::default(), + SerializedTerminalPanel { + items: SerializedItems::WithSplits(SerializedPaneGroup::Pane( + SerializedPane { + active: true, + children: vec![12345], + active_item: Some(12345), + pinned_count: 0, + }, + )), + active_item_id: None, + }, + terminal_panel.downgrade(), + window, + cx, + ) + }) + .unwrap() + .await + .unwrap(); + cx.run_until_parked(); + + assert_eq!(restored_items, 1); + terminal_panel.read_with(cx, |terminal_panel, cx| { + let panes = terminal_panel.center.panes(); + assert_eq!( + panes.len(), + 2, + "both the restored pane and the interim pane must survive" + ); + let interim_panes_kept = panes.iter().filter(|pane| **pane == &interim_pane).count(); + assert_eq!( + interim_panes_kept, 1, + "interim pane must be grafted into the restored center" + ); + assert_eq!(interim_pane.read(cx).items_len(), 1); + assert_ne!( + terminal_panel.active_pane, interim_pane, + "unfocused interim pane must not become the active pane" + ); + }); + } + #[gpui::test] async fn test_local_terminal_in_local_project(cx: &mut TestAppContext) { cx.executor().allow_parking(); @@ -1924,7 +2466,13 @@ mod tests { let result = window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.add_local_terminal_shell(RevealStrategy::Always, window, cx) + terminal_panel.add_terminal_shell( + true, + None, + RevealStrategy::Always, + window, + cx, + ) }) }) .unwrap() @@ -2075,7 +2623,7 @@ mod tests { terminal_panel.update(cx, |panel, cx| panel.set_assistant_enabled(true, cx)); terminal_panel .update_in(cx, |panel, window, cx| { - panel.add_terminal_shell(None, RevealStrategy::Always, window, cx) + panel.add_terminal_shell(false, None, RevealStrategy::Always, window, cx) }) .await .unwrap(); @@ -2313,7 +2861,7 @@ mod tests { window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |panel, cx| { - panel.add_terminal_shell(None, RevealStrategy::Always, window, cx) + panel.add_terminal_shell(false, None, RevealStrategy::Always, window, cx) }) }) .expect("Failed to update workspace") @@ -2495,7 +3043,7 @@ mod tests { window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |panel, cx| { - panel.add_terminal_shell(None, RevealStrategy::Always, window, cx) + panel.add_terminal_shell(false, None, RevealStrategy::Always, window, cx) }) }) .expect("Failed to update workspace") diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 1a9b84a714..8c8a0c6619 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -428,6 +428,11 @@ impl TerminalView { } } + pub(crate) fn mark_needs_serialize(&mut self, cx: &mut Context) { + self.needs_serialize = true; + cx.emit(ItemEvent::UpdateTab); + } + pub fn is_renaming(&self) -> bool { self.rename_editor.is_some() } diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index a07a9035a2..918607c0cd 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -288,12 +288,32 @@ pub struct Dock { active_panel_index: Option, focus_handle: FocusHandle, focus_follows_mouse: FocusFollowsMouse, - pub(crate) serialized_dock: Option, + restoration: DockRestoreState, zoom_layer_open: bool, modal_layer: Entity, _subscriptions: [Subscription; 2], } +enum DockRestoreState { + Restoring { pending: Option }, + Finished, +} + +impl DockRestoreState { + fn pending(&self) -> Option<&DockData> { + match self { + Self::Restoring { pending } => pending.as_ref(), + Self::Finished => None, + } + } + + fn discard_pending(&mut self) { + if let Self::Restoring { pending } = self { + *pending = None; + } + } +} + impl Focusable for Dock { fn focus_handle(&self, _: &App) -> FocusHandle { self.focus_handle.clone() @@ -436,7 +456,7 @@ impl Dock { focus_handle: focus_handle.clone(), focus_follows_mouse: WorkspaceSettings::get_global(cx).focus_follows_mouse, _subscriptions: [focus_subscription, zoom_subscription], - serialized_dock: None, + restoration: DockRestoreState::Restoring { pending: None }, zoom_layer_open: false, modal_layer, } @@ -556,6 +576,13 @@ impl Dock { } pub fn set_open(&mut self, open: bool, window: &mut Window, cx: &mut Context) { + if open != self.is_open { + self.restoration.discard_pending(); + } + self.set_open_internal(open, window, cx); + } + + fn set_open_internal(&mut self, open: bool, window: &mut Window, cx: &mut Context) { if open != self.is_open { self.is_open = open; if let Some(active_panel) = self.active_panel_entry() { @@ -787,34 +814,82 @@ impl Dock { }, ); - self.restore_state(window, cx); + self.replay_pending_serialized_state(window, cx); if panel.read(cx).starts_open(window, cx) { - self.activate_panel(index, window, cx); - self.set_open(true, window, cx); + self.activate_panel_internal(index, window, cx); + self.set_open_internal(true, window, cx); } cx.notify(); index } - pub fn restore_state(&mut self, window: &mut Window, cx: &mut Context) -> bool { - if let Some(serialized) = self.serialized_dock.clone() { - if let Some(active_panel) = serialized.active_panel.filter(|_| serialized.visible) - && let Some(idx) = self.panel_index_for_persistent_name(active_panel.as_str(), cx) - { - self.activate_panel(idx, window, cx); + pub(crate) fn restore_serialized_state( + &mut self, + serialized: DockData, + window: &mut Window, + cx: &mut Context, + ) { + match &mut self.restoration { + DockRestoreState::Restoring { pending } => { + *pending = Some(serialized); + self.replay_pending_serialized_state(window, cx); } - - if serialized.zoom - && let Some(panel) = self.active_panel() - { - panel.set_zoomed(true, window, cx) + DockRestoreState::Finished => { + let active_panel_missing = serialized + .active_panel + .as_deref() + .filter(|_| serialized.visible) + .is_some_and(|name| self.panel_index_for_persistent_name(name, cx).is_none()); + if !active_panel_missing { + self.apply_serialized_state(&serialized, window, cx); + } } - self.set_open(serialized.visible, window, cx); - return true; } - false + } + + fn replay_pending_serialized_state(&mut self, window: &mut Window, cx: &mut Context) { + let Some(serialized) = self.restoration.pending().cloned() else { + return; + }; + let waiting_for_active_panel = serialized + .active_panel + .as_deref() + .filter(|_| serialized.visible) + .is_some_and(|name| self.panel_index_for_persistent_name(name, cx).is_none()); + if waiting_for_active_panel { + self.set_open_internal(serialized.visible, window, cx); + } else { + self.apply_serialized_state(&serialized, window, cx); + self.restoration.discard_pending(); + } + } + + fn apply_serialized_state( + &mut self, + serialized: &DockData, + window: &mut Window, + cx: &mut Context, + ) { + if let Some(active_panel) = serialized + .active_panel + .as_deref() + .filter(|_| serialized.visible) + && let Some(idx) = self.panel_index_for_persistent_name(active_panel, cx) + { + self.activate_panel_internal(idx, window, cx); + } + if serialized.zoom + && let Some(panel) = self.active_panel() + { + panel.set_zoomed(true, window, cx) + } + self.set_open_internal(serialized.visible, window, cx); + } + + pub(crate) fn finish_restoration(&mut self) { + self.restoration = DockRestoreState::Finished; } pub fn remove_panel( @@ -861,6 +936,18 @@ impl Dock { } pub fn activate_panel(&mut self, panel_ix: usize, window: &mut Window, cx: &mut Context) { + if Some(panel_ix) != self.active_panel_index { + self.restoration.discard_pending(); + } + self.activate_panel_internal(panel_ix, window, cx); + } + + fn activate_panel_internal( + &mut self, + panel_ix: usize, + window: &mut Window, + cx: &mut Context, + ) { if Some(panel_ix) != self.active_panel_index { if let Some(active_panel) = self.active_panel_entry() { active_panel.panel.set_active(false, window, cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 2f8d54d60f..b5647de453 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2151,8 +2151,11 @@ impl Workspace { (&workspace.bottom_dock, &default_docks.bottom), ] { dock.update(cx, |dock, cx| { - dock.serialized_dock = Some(serialized_dock.clone()); - dock.restore_state(window, cx); + dock.restore_serialized_state( + serialized_dock.clone(), + window, + cx, + ); }); } cx.notify(); @@ -2289,8 +2292,15 @@ impl Workspace { (&self.right_dock, docks.right), ] { dock.update(cx, |dock, cx| { - dock.serialized_dock = Some(data); - dock.restore_state(window, cx); + dock.restore_serialized_state(data, window, cx); + }); + } + } + + pub fn finish_dock_restoration(&self, cx: &mut App) { + for dock in [&self.left_dock, &self.bottom_dock, &self.right_dock] { + dock.update(cx, |dock, _| { + dock.finish_restoration(); }); } } @@ -7477,8 +7487,7 @@ impl Workspace { .iter_mut() { dock.update(cx, |dock, cx| { - dock.serialized_dock = Some(serialized_dock.clone()); - dock.restore_state(window, cx); + dock.restore_serialized_state(serialized_dock.clone(), window, cx); }); } @@ -11763,6 +11772,7 @@ mod tests { ItemBufferKind, ItemEvent, test::{TestItem, TestProjectItem}, }, + persistence::model::DockData, }; use fs::FakeFs; use gpui::{ @@ -18452,4 +18462,346 @@ mod tests { }); assert_eq!(cx.opened_url(), Some("nonexistent.txt".to_string())); } + + #[gpui::test] + async fn test_serialized_dock_state_not_replayed_over_manual_panel_changes( + cx: &mut gpui::TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.bottom_dock().update(cx, |dock, cx| { + dock.restore_serialized_state( + DockData { + visible: true, + active_panel: Some(SecondTestPanel::persistent_name().to_string()), + zoom: false, + }, + window, + cx, + ); + }); + }); + + let first_panel = workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| TestPanel::new(DockPosition::Bottom, 100, cx)); + workspace.add_panel(panel.clone(), window, cx); + panel + }); + let second_panel = workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| SecondTestPanel { + focus_handle: cx.focus_handle(), + zoomed: false, + }); + workspace.add_panel(panel.clone(), window, cx); + panel + }); + workspace.read_with(cx, |workspace, cx| { + let dock = workspace.bottom_dock().read(cx); + assert!(dock.is_open()); + assert_eq!( + dock.active_panel().map(|panel| panel.panel_id()), + Some(second_panel.entity_id()), + ); + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_panel_focus::(window, cx); + }); + workspace.read_with(cx, |workspace, cx| { + assert_eq!( + workspace + .bottom_dock() + .read(cx) + .active_panel() + .map(|panel| panel.panel_id()), + Some(first_panel.entity_id()), + ); + }); + + workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| TestPanel::new(DockPosition::Bottom, 300, cx)); + workspace.add_panel(panel, window, cx); + }); + workspace.read_with(cx, |workspace, cx| { + let dock = workspace.bottom_dock().read(cx); + assert!(dock.is_open()); + assert_eq!( + dock.active_panel().map(|panel| panel.panel_id()), + Some(first_panel.entity_id()), + ); + }); + } + + #[gpui::test] + async fn test_closed_dock_not_reopened_by_late_serialized_panel(cx: &mut gpui::TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.bottom_dock().update(cx, |dock, cx| { + dock.restore_serialized_state( + DockData { + visible: true, + active_panel: Some(SecondTestPanel::persistent_name().to_string()), + zoom: false, + }, + window, + cx, + ); + }); + }); + workspace.read_with(cx, |workspace, cx| { + assert!(workspace.bottom_dock().read(cx).is_open()); + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace + .bottom_dock() + .update(cx, |dock, cx| dock.set_open(false, window, cx)); + }); + + workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| SecondTestPanel { + focus_handle: cx.focus_handle(), + zoomed: false, + }); + workspace.add_panel(panel, window, cx); + }); + + workspace.read_with(cx, |workspace, cx| { + assert!(!workspace.bottom_dock().read(cx).is_open()); + }); + } + + #[gpui::test] + async fn test_stale_serialized_dock_state_discarded_when_panel_loading_finishes( + cx: &mut gpui::TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.bottom_dock().update(cx, |dock, cx| { + dock.restore_serialized_state( + DockData { + visible: true, + active_panel: Some(SecondTestPanel::persistent_name().to_string()), + zoom: false, + }, + window, + cx, + ); + }); + }); + + workspace.update(cx, |workspace, cx| { + workspace.finish_dock_restoration(cx); + }); + + workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| SecondTestPanel { + focus_handle: cx.focus_handle(), + zoomed: false, + }); + workspace.add_panel(panel, window, cx); + }); + workspace.read_with(cx, |workspace, cx| { + assert_eq!( + workspace + .bottom_dock() + .read(cx) + .active_panel() + .map(|panel| panel.panel_id()), + None, + "stale dock restoration state should not replay after panel loading finished", + ); + }); + } + + #[gpui::test] + async fn test_serialized_state_for_missing_panel_dropped_after_restoration_finishes( + cx: &mut gpui::TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + workspace.update(cx, |workspace, cx| { + workspace.finish_dock_restoration(cx); + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.bottom_dock().update(cx, |dock, cx| { + dock.restore_serialized_state( + DockData { + visible: true, + active_panel: Some(SecondTestPanel::persistent_name().to_string()), + zoom: false, + }, + window, + cx, + ); + }); + }); + workspace.read_with(cx, |workspace, cx| { + let dock = workspace.bottom_dock().read(cx); + assert!( + !dock.is_open(), + "dock must not open for a panel that can no longer register", + ); + }); + + workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| SecondTestPanel { + focus_handle: cx.focus_handle(), + zoomed: false, + }); + workspace.add_panel(panel, window, cx); + }); + workspace.read_with(cx, |workspace, cx| { + assert!( + !workspace.bottom_dock().read(cx).is_open(), + "stale dock state must not replay when the panel registers much later", + ); + }); + } + + #[gpui::test] + async fn test_serialized_zoom_not_applied_while_waiting_for_serialized_panel( + cx: &mut gpui::TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + let first_panel = workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| TestPanel::new(DockPosition::Bottom, 100, cx)); + workspace.add_panel(panel.clone(), window, cx); + workspace.toggle_panel_focus::(window, cx); + panel + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.bottom_dock().update(cx, |dock, cx| { + dock.restore_serialized_state( + DockData { + visible: true, + active_panel: Some(SecondTestPanel::persistent_name().to_string()), + zoom: true, + }, + window, + cx, + ); + }); + }); + workspace.read_with(cx, |_, cx| { + assert!( + !first_panel.read(cx).zoomed, + "serialized zoom should not apply to another panel while the serialized active panel is loading", + ); + }); + + let second_panel = workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| SecondTestPanel { + focus_handle: cx.focus_handle(), + zoomed: false, + }); + workspace.add_panel(panel.clone(), window, cx); + panel + }); + workspace.read_with(cx, |workspace, cx| { + let dock = workspace.bottom_dock().read(cx); + assert!(dock.is_open()); + assert_eq!( + dock.active_panel().map(|panel| panel.panel_id()), + Some(second_panel.entity_id()), + ); + assert!(!first_panel.read(cx).zoomed); + assert!(second_panel.read(cx).zoomed); + }); + } + + struct SecondTestPanel { + focus_handle: FocusHandle, + zoomed: bool, + } + + impl EventEmitter for SecondTestPanel {} + + impl Focusable for SecondTestPanel { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } + } + + impl Render for SecondTestPanel { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + div() + .id("second-test-panel") + .track_focus(&self.focus_handle(cx)) + } + } + + impl Panel for SecondTestPanel { + fn persistent_name() -> &'static str { + "SecondTestPanel" + } + + fn panel_key() -> &'static str { + "SecondTestPanel" + } + + fn position(&self, _: &Window, _: &App) -> DockPosition { + DockPosition::Bottom + } + + fn position_is_valid(&self, _: DockPosition) -> bool { + true + } + + fn set_position(&mut self, _: DockPosition, _: &mut Window, _: &mut Context) {} + + fn default_size(&self, _: &Window, _: &App) -> Pixels { + px(300.) + } + + fn icon(&self, _: &Window, _: &App) -> Option { + None + } + + fn icon_tooltip(&self, _: &Window, _: &App) -> Option<&'static str> { + None + } + + fn toggle_action(&self) -> Box { + crate::dock::test::ToggleTestPanel.boxed_clone() + } + + fn activation_priority(&self) -> u32 { + 200 + } + + fn is_zoomed(&self, _: &Window, _: &App) -> bool { + self.zoomed + } + + fn set_zoomed(&mut self, zoomed: bool, _: &mut Window, _: &mut Context) { + self.zoomed = zoomed; + } + } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 85fc30055d..1223bb1cc6 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -808,9 +808,13 @@ fn initialize_panels(window: &mut Window, cx: &mut Context) -> Task