Add each panel to the workspace as soon as it's ready (#43414)

Agus Zubiaga created 3 weeks ago

We'll now add panels to the workspace as soon as they're ready rather
than waiting for all the rest to complete. We should strive to make all
panels fast, but given that their load tasks are fallible and do IO,
this approach seems more resilient.

Additionally, we'll now start loading the agent panel at the same time
as the rest.

Release Notes:

- workspace: Add panels as soon as they are ready

Change summary

crates/collab/src/tests/following_tests.rs |   2 
crates/workspace/src/dock.rs               |  17 +
crates/workspace/src/workspace.rs          |  12 
crates/zed/src/zed.rs                      | 195 ++++++++++++-----------
4 files changed, 122 insertions(+), 104 deletions(-)

Detailed changes

crates/collab/src/tests/following_tests.rs

@@ -529,7 +529,7 @@ async fn test_basic_following(
         });
 
         // Client B activates a panel, and the previously-opened screen-sharing item gets activated.
-        let panel = cx_b.new(|cx| TestPanel::new(DockPosition::Left, cx));
+        let panel = cx_b.new(|cx| TestPanel::new(DockPosition::Left, 100, cx));
         workspace_b.update_in(cx_b, |workspace, window, cx| {
             workspace.add_panel(panel, window, cx);
             workspace.toggle_panel_focus::<TestPanel>(window, cx);

crates/workspace/src/dock.rs

@@ -560,7 +560,16 @@ impl Dock {
             .binary_search_by_key(&panel.read(cx).activation_priority(), |entry| {
                 entry.panel.activation_priority(cx)
             }) {
-            Ok(ix) => ix,
+            Ok(ix) => {
+                if cfg!(debug_assertions) {
+                    panic!(
+                        "Panels `{}` and `{}` have the same activation priority. Each panel must have a unique priority so the status bar order is deterministic.",
+                        T::panel_key(),
+                        self.panel_entries[ix].panel.panel_key()
+                    );
+                }
+                ix
+            }
             Err(ix) => ix,
         };
         if let Some(active_index) = self.active_panel_index.as_mut()
@@ -994,19 +1003,21 @@ pub mod test {
         pub active: bool,
         pub focus_handle: FocusHandle,
         pub size: Pixels,
+        pub activation_priority: u32,
     }
     actions!(test_only, [ToggleTestPanel]);
 
     impl EventEmitter<PanelEvent> for TestPanel {}
 
     impl TestPanel {
-        pub fn new(position: DockPosition, cx: &mut App) -> Self {
+        pub fn new(position: DockPosition, activation_priority: u32, cx: &mut App) -> Self {
             Self {
                 position,
                 zoomed: false,
                 active: false,
                 focus_handle: cx.focus_handle(),
                 size: px(300.),
+                activation_priority,
             }
         }
     }
@@ -1072,7 +1083,7 @@ pub mod test {
         }
 
         fn activation_priority(&self) -> u32 {
-            100
+            self.activation_priority
         }
     }
 

crates/workspace/src/workspace.rs

@@ -9176,7 +9176,7 @@ mod tests {
             cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
 
         let panel = workspace.update_in(cx, |workspace, window, cx| {
-            let panel = cx.new(|cx| TestPanel::new(DockPosition::Right, cx));
+            let panel = cx.new(|cx| TestPanel::new(DockPosition::Right, 100, cx));
             workspace.add_panel(panel.clone(), window, cx);
 
             workspace
@@ -9409,10 +9409,10 @@ mod tests {
 
         // Open two docks (left and right) with one panel each
         let (left_panel, right_panel) = workspace.update_in(cx, |workspace, window, cx| {
-            let left_panel = cx.new(|cx| TestPanel::new(DockPosition::Left, cx));
+            let left_panel = cx.new(|cx| TestPanel::new(DockPosition::Left, 100, cx));
             workspace.add_panel(left_panel.clone(), window, cx);
 
-            let right_panel = cx.new(|cx| TestPanel::new(DockPosition::Right, cx));
+            let right_panel = cx.new(|cx| TestPanel::new(DockPosition::Right, 101, cx));
             workspace.add_panel(right_panel.clone(), window, cx);
 
             workspace.toggle_dock(DockPosition::Left, window, cx);
@@ -9840,10 +9840,10 @@ mod tests {
             cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
 
         let (panel_1, panel_2) = workspace.update_in(cx, |workspace, window, cx| {
-            let panel_1 = cx.new(|cx| TestPanel::new(DockPosition::Left, cx));
+            let panel_1 = cx.new(|cx| TestPanel::new(DockPosition::Left, 100, cx));
             workspace.add_panel(panel_1.clone(), window, cx);
             workspace.toggle_dock(DockPosition::Left, window, cx);
-            let panel_2 = cx.new(|cx| TestPanel::new(DockPosition::Right, cx));
+            let panel_2 = cx.new(|cx| TestPanel::new(DockPosition::Right, 101, cx));
             workspace.add_panel(panel_2.clone(), window, cx);
             workspace.toggle_dock(DockPosition::Right, window, cx);
 
@@ -10750,7 +10750,7 @@ mod tests {
         // Add a new panel to the right dock, opening the dock and setting the
         // focus to the new panel.
         let panel = workspace.update_in(cx, |workspace, window, cx| {
-            let panel = cx.new(|cx| TestPanel::new(DockPosition::Right, cx));
+            let panel = cx.new(|cx| TestPanel::new(DockPosition::Right, 100, cx));
             workspace.add_panel(panel.clone(), window, cx);
 
             workspace

crates/zed/src/zed.rs

@@ -22,16 +22,17 @@ use editor::{Editor, MultiBuffer};
 use extension_host::ExtensionStore;
 use feature_flags::{FeatureFlagAppExt, PanicFeatureFlag};
 use fs::Fs;
+use futures::FutureExt as _;
 use futures::future::Either;
 use futures::{StreamExt, channel::mpsc, select_biased};
 use git_ui::commit_view::CommitViewToolbar;
 use git_ui::git_panel::GitPanel;
 use git_ui::project_diff::ProjectDiffToolbar;
 use gpui::{
-    Action, App, AppContext as _, Context, DismissEvent, Element, Entity, Focusable, KeyBinding,
-    ParentElement, PathPromptOptions, PromptLevel, ReadGlobal, SharedString, Styled, Task,
-    TitlebarOptions, UpdateGlobal, Window, WindowKind, WindowOptions, actions, image_cache, point,
-    px, retain_all,
+    Action, App, AppContext as _, AsyncWindowContext, Context, DismissEvent, Element, Entity,
+    Focusable, KeyBinding, ParentElement, PathPromptOptions, PromptLevel, ReadGlobal, SharedString,
+    Styled, Task, TitlebarOptions, UpdateGlobal, WeakEntity, Window, WindowKind, WindowOptions,
+    actions, image_cache, point, px, retain_all,
 };
 use image_viewer::ImageInfo;
 use language::Capability;
@@ -655,105 +656,111 @@ fn initialize_panels(
         );
         let debug_panel = DebugPanel::load(workspace_handle.clone(), cx);
 
-        let (
-            project_panel,
-            outline_panel,
-            terminal_panel,
-            git_panel,
-            channels_panel,
-            notification_panel,
-            debug_panel,
-        ) = futures::try_join!(
-            project_panel,
-            outline_panel,
-            git_panel,
-            terminal_panel,
-            channels_panel,
-            notification_panel,
-            debug_panel,
-        )?;
-
-        workspace_handle.update_in(cx, |workspace, window, cx| {
-            workspace.add_panel(project_panel, window, cx);
-            workspace.add_panel(outline_panel, window, cx);
-            workspace.add_panel(terminal_panel, window, cx);
-            workspace.add_panel(git_panel, window, cx);
-            workspace.add_panel(channels_panel, window, cx);
-            workspace.add_panel(notification_panel, window, cx);
-            workspace.add_panel(debug_panel, window, cx);
-        })?;
-
-        fn setup_or_teardown_agent_panel(
-            workspace: &mut Workspace,
-            prompt_builder: Arc<PromptBuilder>,
-            window: &mut Window,
-            cx: &mut Context<Workspace>,
-        ) -> Task<anyhow::Result<()>> {
-            let disable_ai = SettingsStore::global(cx)
-                .get::<DisableAiSettings>(None)
-                .disable_ai
-                || cfg!(test);
-            let existing_panel = workspace.panel::<agent_ui::AgentPanel>(cx);
-            match (disable_ai, existing_panel) {
-                (false, None) => cx.spawn_in(window, async move |workspace, cx| {
-                    let panel =
-                        agent_ui::AgentPanel::load(workspace.clone(), prompt_builder, cx.clone())
-                            .await?;
-                    workspace.update_in(cx, |workspace, window, cx| {
-                        let disable_ai = SettingsStore::global(cx)
-                            .get::<DisableAiSettings>(None)
-                            .disable_ai;
-                        let have_panel = workspace.panel::<agent_ui::AgentPanel>(cx).is_some();
-                        if !disable_ai && !have_panel {
-                            workspace.add_panel(panel, window, cx);
-                        }
+        async fn add_panel_when_ready(
+            panel_task: impl Future<Output = anyhow::Result<Entity<impl workspace::Panel>>> + 'static,
+            workspace_handle: WeakEntity<Workspace>,
+            mut cx: gpui::AsyncWindowContext,
+        ) {
+            if let Some(panel) = panel_task.await.context("failed to load panel").log_err()
+            {
+                workspace_handle
+                    .update_in(&mut cx, |workspace, window, cx| {
+                        workspace.add_panel(panel, window, cx);
                     })
-                }),
-                (true, Some(existing_panel)) => {
-                    workspace.remove_panel::<agent_ui::AgentPanel>(&existing_panel, window, cx);
-                    Task::ready(Ok(()))
-                }
-                _ => Task::ready(Ok(())),
+                    .log_err();
             }
         }
 
-        workspace_handle
-            .update_in(cx, |workspace, window, cx| {
-                setup_or_teardown_agent_panel(workspace, prompt_builder.clone(), window, cx)
-            })?
-            .await?;
+        futures::join!(
+            add_panel_when_ready(project_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(outline_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(terminal_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(git_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(channels_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(notification_panel, workspace_handle.clone(), cx.clone()),
+            add_panel_when_ready(debug_panel, workspace_handle.clone(), cx.clone()),
+            initialize_agent_panel(workspace_handle, prompt_builder, cx.clone()).map(|r| r.log_err())
+        );
 
-        workspace_handle.update_in(cx, |workspace, window, cx| {
-            cx.observe_global_in::<SettingsStore>(window, {
-                let prompt_builder = prompt_builder.clone();
-                move |workspace, window, cx| {
-                    setup_or_teardown_agent_panel(workspace, prompt_builder.clone(), window, cx)
-                        .detach_and_log_err(cx);
-                }
-            })
-            .detach();
+        anyhow::Ok(())
+    })
+    .detach();
+}
 
-            // Register the actions that are shared between `assistant` and `assistant2`.
-            //
-            // We need to do this here instead of within the individual `init`
-            // functions so that we only register the actions once.
-            //
-            // Once we ship `assistant2` we can push this back down into `agent::agent_panel::init`.
-            if !cfg!(test) {
-                <dyn AgentPanelDelegate>::set_global(
-                    Arc::new(agent_ui::ConcreteAssistantPanelDelegate),
-                    cx,
-                );
+async fn initialize_agent_panel(
+    workspace_handle: WeakEntity<Workspace>,
+    prompt_builder: Arc<PromptBuilder>,
+    mut cx: AsyncWindowContext,
+) -> anyhow::Result<()> {
+    fn setup_or_teardown_agent_panel(
+        workspace: &mut Workspace,
+        prompt_builder: Arc<PromptBuilder>,
+        window: &mut Window,
+        cx: &mut Context<Workspace>,
+    ) -> Task<anyhow::Result<()>> {
+        let disable_ai = SettingsStore::global(cx)
+            .get::<DisableAiSettings>(None)
+            .disable_ai
+            || cfg!(test);
+        let existing_panel = workspace.panel::<agent_ui::AgentPanel>(cx);
+        match (disable_ai, existing_panel) {
+            (false, None) => cx.spawn_in(window, async move |workspace, cx| {
+                let panel =
+                    agent_ui::AgentPanel::load(workspace.clone(), prompt_builder, cx.clone())
+                        .await?;
+                workspace.update_in(cx, |workspace, window, cx| {
+                    let disable_ai = SettingsStore::global(cx)
+                        .get::<DisableAiSettings>(None)
+                        .disable_ai;
+                    let have_panel = workspace.panel::<agent_ui::AgentPanel>(cx).is_some();
+                    if !disable_ai && !have_panel {
+                        workspace.add_panel(panel, window, cx);
+                    }
+                })
+            }),
+            (true, Some(existing_panel)) => {
+                workspace.remove_panel::<agent_ui::AgentPanel>(&existing_panel, window, cx);
+                Task::ready(Ok(()))
+            }
+            _ => Task::ready(Ok(())),
+        }
+    }
 
-                workspace
-                    .register_action(agent_ui::AgentPanel::toggle_focus)
-                    .register_action(agent_ui::InlineAssistant::inline_assist);
+    workspace_handle
+        .update_in(&mut cx, |workspace, window, cx| {
+            setup_or_teardown_agent_panel(workspace, prompt_builder.clone(), window, cx)
+        })?
+        .await?;
+
+    workspace_handle.update_in(&mut cx, |workspace, window, cx| {
+        cx.observe_global_in::<SettingsStore>(window, {
+            let prompt_builder = prompt_builder.clone();
+            move |workspace, window, cx| {
+                setup_or_teardown_agent_panel(workspace, prompt_builder.clone(), window, cx)
+                    .detach_and_log_err(cx);
             }
-        })?;
+        })
+        .detach();
 
-        anyhow::Ok(())
-    })
-    .detach();
+        // Register the actions that are shared between `assistant` and `assistant2`.
+        //
+        // We need to do this here instead of within the individual `init`
+        // functions so that we only register the actions once.
+        //
+        // Once we ship `assistant2` we can push this back down into `agent::agent_panel::init`.
+        if !cfg!(test) {
+            <dyn AgentPanelDelegate>::set_global(
+                Arc::new(agent_ui::ConcreteAssistantPanelDelegate),
+                cx,
+            );
+
+            workspace
+                .register_action(agent_ui::AgentPanel::toggle_focus)
+                .register_action(agent_ui::InlineAssistant::inline_assist);
+        }
+    })?;
+
+    anyhow::Ok(())
 }
 
 fn register_actions(