Make `copilot::SignIn` open sign-in modal when needed (#30239)

Michael Sloan created

Also:

* Makes sign out show status notifications and errors.
* Reinstall now prompts for sign-in after start.

Addresses some of #29250, but not all of it.

Release Notes:

- N/A

Change summary

crates/copilot/src/copilot.rs                                   |  53 
crates/copilot/src/sign_in.rs                                   | 155 ++
crates/inline_completion_button/src/inline_completion_button.rs |  10 
3 files changed, 142 insertions(+), 76 deletions(-)

Detailed changes

crates/copilot/src/copilot.rs 🔗

@@ -3,6 +3,7 @@ mod copilot_completion_provider;
 pub mod request;
 mod sign_in;
 
+use crate::sign_in::initiate_sign_in_within_workspace;
 use ::fs::Fs;
 use anyhow::{Result, anyhow};
 use collections::{HashMap, HashSet};
@@ -24,6 +25,7 @@ use node_runtime::NodeRuntime;
 use parking_lot::Mutex;
 use request::StatusNotification;
 use settings::SettingsStore;
+use sign_in::{reinstall_and_sign_in_within_workspace, sign_out_within_workspace};
 use std::{
     any::TypeId,
     env,
@@ -34,9 +36,10 @@ use std::{
     sync::Arc,
 };
 use util::{ResultExt, fs::remove_matching};
+use workspace::Workspace;
 
 pub use crate::copilot_completion_provider::CopilotCompletionProvider;
-pub use crate::sign_in::{CopilotCodeVerification, initiate_sign_in};
+pub use crate::sign_in::{CopilotCodeVerification, initiate_sign_in, reinstall_and_sign_in};
 
 actions!(
     copilot,
@@ -99,27 +102,25 @@ pub fn init(
     })
     .detach();
 
-    cx.on_action(|_: &SignIn, cx| {
-        if let Some(copilot) = Copilot::global(cx) {
-            copilot
-                .update(cx, |copilot, cx| copilot.sign_in(cx))
-                .detach_and_log_err(cx);
-        }
-    });
-    cx.on_action(|_: &SignOut, cx| {
-        if let Some(copilot) = Copilot::global(cx) {
-            copilot
-                .update(cx, |copilot, cx| copilot.sign_out(cx))
-                .detach_and_log_err(cx);
-        }
-    });
-    cx.on_action(|_: &Reinstall, cx| {
-        if let Some(copilot) = Copilot::global(cx) {
-            copilot
-                .update(cx, |copilot, cx| copilot.reinstall(cx))
-                .detach();
-        }
-    });
+    cx.observe_new(|workspace: &mut Workspace, _window, _cx| {
+        workspace.register_action(|workspace, _: &SignIn, window, cx| {
+            if let Some(copilot) = Copilot::global(cx) {
+                let is_reinstall = false;
+                initiate_sign_in_within_workspace(workspace, copilot, is_reinstall, window, cx);
+            }
+        });
+        workspace.register_action(|workspace, _: &Reinstall, window, cx| {
+            if let Some(copilot) = Copilot::global(cx) {
+                reinstall_and_sign_in_within_workspace(workspace, copilot, window, cx);
+            }
+        });
+        workspace.register_action(|workspace, _: &SignOut, _window, cx| {
+            if let Some(copilot) = Copilot::global(cx) {
+                sign_out_within_workspace(workspace, copilot, cx);
+            }
+        });
+    })
+    .detach();
 }
 
 enum CopilotServer {
@@ -563,7 +564,7 @@ impl Copilot {
         .ok();
     }
 
-    pub fn sign_in(&mut self, cx: &mut Context<Self>) -> Task<Result<()>> {
+    pub(crate) fn sign_in(&mut self, cx: &mut Context<Self>) -> Task<Result<()>> {
         if let CopilotServer::Running(server) = &mut self.server {
             let task = match &server.sign_in_status {
                 SignInStatus::Authorized { .. } => Task::ready(Ok(())).shared(),
@@ -647,7 +648,7 @@ impl Copilot {
         }
     }
 
-    pub fn sign_out(&mut self, cx: &mut Context<Self>) -> Task<Result<()>> {
+    pub(crate) fn sign_out(&mut self, cx: &mut Context<Self>) -> Task<Result<()>> {
         self.update_sign_in_status(request::SignInStatus::NotSignedIn, cx);
         match &self.server {
             CopilotServer::Running(RunningCopilotServer { lsp: server, .. }) => {
@@ -667,7 +668,7 @@ impl Copilot {
         }
     }
 
-    pub fn reinstall(&mut self, cx: &mut Context<Self>) -> Task<()> {
+    pub(crate) fn reinstall(&mut self, cx: &mut Context<Self>) -> Shared<Task<()>> {
         let language_settings = all_language_settings(None, cx);
         let env = self.build_env(&language_settings.edit_predictions.copilot);
         let start_task = cx
@@ -689,7 +690,7 @@ impl Copilot {
 
         cx.notify();
 
-        cx.background_spawn(start_task)
+        start_task
     }
 
     pub fn language_server(&self) -> Option<&Arc<LanguageServer>> {

crates/copilot/src/sign_in.rs 🔗

@@ -12,7 +12,7 @@ use workspace::{ModalView, Toast, Workspace};
 
 const COPILOT_SIGN_UP_URL: &str = "https://github.com/features/copilot";
 
-struct CopilotStartingToast;
+struct CopilotStatusToast;
 
 pub fn initiate_sign_in(window: &mut Window, cx: &mut App) {
     let Some(copilot) = Copilot::global(cx) else {
@@ -21,50 +21,83 @@ pub fn initiate_sign_in(window: &mut Window, cx: &mut App) {
     let Some(workspace) = window.root::<Workspace>().flatten() else {
         return;
     };
+    workspace.update(cx, |workspace, cx| {
+        let is_reinstall = false;
+        initiate_sign_in_within_workspace(workspace, copilot, is_reinstall, window, cx)
+    });
+}
+
+pub fn reinstall_and_sign_in(window: &mut Window, cx: &mut App) {
+    let Some(copilot) = Copilot::global(cx) else {
+        return;
+    };
+    let Some(workspace) = window.root::<Workspace>().flatten() else {
+        return;
+    };
+    workspace.update(cx, |workspace, cx| {
+        reinstall_and_sign_in_within_workspace(workspace, copilot, window, cx);
+    });
+}
+
+pub fn reinstall_and_sign_in_within_workspace(
+    workspace: &mut Workspace,
+    copilot: Entity<Copilot>,
+    window: &mut Window,
+    cx: &mut Context<Workspace>,
+) {
+    let _ = copilot.update(cx, |copilot, cx| copilot.reinstall(cx));
+    let is_reinstall = true;
+    initiate_sign_in_within_workspace(workspace, copilot, is_reinstall, window, cx);
+}
+
+pub fn initiate_sign_in_within_workspace(
+    workspace: &mut Workspace,
+    copilot: Entity<Copilot>,
+    is_reinstall: bool,
+    window: &mut Window,
+    cx: &mut Context<Workspace>,
+) {
     if matches!(copilot.read(cx).status(), Status::Disabled) {
-        copilot.update(cx, |this, cx| this.start_copilot(false, true, cx));
+        copilot.update(cx, |copilot, cx| copilot.start_copilot(false, true, cx));
     }
     match copilot.read(cx).status() {
         Status::Starting { task } => {
-            workspace.update(cx, |workspace, cx| {
-                workspace.show_toast(
-                    Toast::new(
-                        NotificationId::unique::<CopilotStartingToast>(),
-                        "Copilot is starting...",
-                    ),
-                    cx,
-                );
-            });
+            workspace.show_toast(
+                Toast::new(
+                    NotificationId::unique::<CopilotStatusToast>(),
+                    if is_reinstall {
+                        "Copilot is reinstalling..."
+                    } else {
+                        "Copilot is starting..."
+                    },
+                ),
+                cx,
+            );
 
-            let workspace = workspace.downgrade();
-            cx.spawn(async move |cx| {
+            cx.spawn_in(window, async move |workspace, cx| {
                 task.await;
-                if let Some(copilot) = cx.update(|cx| Copilot::global(cx)).ok().flatten() {
+                if let Some(copilot) = cx.update(|_window, cx| Copilot::global(cx)).ok().flatten() {
                     workspace
-                        .update(cx, |workspace, cx| match copilot.read(cx).status() {
-                            Status::Authorized => workspace.show_toast(
-                                Toast::new(
-                                    NotificationId::unique::<CopilotStartingToast>(),
-                                    "Copilot has started!",
-                                ),
-                                cx,
-                            ),
-                            _ => {
-                                workspace.dismiss_toast(
-                                    &NotificationId::unique::<CopilotStartingToast>(),
+                        .update_in(cx, |workspace, window, cx| {
+                            match copilot.read(cx).status() {
+                                Status::Authorized => workspace.show_toast(
+                                    Toast::new(
+                                        NotificationId::unique::<CopilotStatusToast>(),
+                                        "Copilot has started.",
+                                    ),
                                     cx,
-                                );
-                                copilot
-                                    .update(cx, |copilot, cx| copilot.sign_in(cx))
-                                    .detach_and_log_err(cx);
-                                if let Some(window_handle) = cx.active_window() {
-                                    window_handle
-                                        .update(cx, |_, window, cx| {
-                                            workspace.toggle_modal(window, cx, |_, cx| {
-                                                CopilotCodeVerification::new(&copilot, cx)
-                                            });
-                                        })
-                                        .log_err();
+                                ),
+                                _ => {
+                                    workspace.dismiss_toast(
+                                        &NotificationId::unique::<CopilotStatusToast>(),
+                                        cx,
+                                    );
+                                    copilot
+                                        .update(cx, |copilot, cx| copilot.sign_in(cx))
+                                        .detach_and_log_err(cx);
+                                    workspace.toggle_modal(window, cx, |_, cx| {
+                                        CopilotCodeVerification::new(&copilot, cx)
+                                    });
                                 }
                             }
                         })
@@ -74,16 +107,54 @@ pub fn initiate_sign_in(window: &mut Window, cx: &mut App) {
             .detach();
         }
         _ => {
-            copilot.update(cx, |this, cx| this.sign_in(cx)).detach();
-            workspace.update(cx, |this, cx| {
-                this.toggle_modal(window, cx, |_, cx| {
-                    CopilotCodeVerification::new(&copilot, cx)
-                });
+            copilot
+                .update(cx, |copilot, cx| copilot.sign_in(cx))
+                .detach();
+            workspace.toggle_modal(window, cx, |_, cx| {
+                CopilotCodeVerification::new(&copilot, cx)
             });
         }
     }
 }
 
+pub fn sign_out_within_workspace(
+    workspace: &mut Workspace,
+    copilot: Entity<Copilot>,
+    cx: &mut Context<Workspace>,
+) {
+    workspace.show_toast(
+        Toast::new(
+            NotificationId::unique::<CopilotStatusToast>(),
+            "Signing out of Copilot...",
+        ),
+        cx,
+    );
+    let sign_out_task = copilot.update(cx, |copilot, cx| copilot.sign_out(cx));
+    cx.spawn(async move |workspace, cx| match sign_out_task.await {
+        Ok(()) => {
+            workspace
+                .update(cx, |workspace, cx| {
+                    workspace.show_toast(
+                        Toast::new(
+                            NotificationId::unique::<CopilotStatusToast>(),
+                            "Signed out of Copilot.",
+                        ),
+                        cx,
+                    )
+                })
+                .ok();
+        }
+        Err(err) => {
+            workspace
+                .update(cx, |workspace, cx| {
+                    workspace.show_error(&err, cx);
+                })
+                .ok();
+        }
+    })
+    .detach();
+}
+
 pub struct CopilotCodeVerification {
     status: Status,
     connect_clicked: bool,

crates/inline_completion_button/src/inline_completion_button.rs 🔗

@@ -106,14 +106,8 @@ impl Render for InlineCompletionButton {
                                             )
                                             .on_click(
                                                 "Reinstall Copilot",
-                                                |_, cx| {
-                                                    if let Some(copilot) = Copilot::global(cx) {
-                                                        copilot
-                                                            .update(cx, |copilot, cx| {
-                                                                copilot.reinstall(cx)
-                                                            })
-                                                            .detach();
-                                                    }
+                                                |window, cx| {
+                                                    copilot::reinstall_and_sign_in(window, cx)
                                                 },
                                             ),
                                             cx,