From f5d4bcd934e991143541f3f1fcdfb83afaec6f3a Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 30 Mar 2023 14:10:57 -0700 Subject: [PATCH] Added erorr states and first-pass error handling to the copilot status bar item. Added correct icons Added a new 'Toast' action which allows other crates to easily pop toasts with an optional click action --- .../{maybe_link_out.svg => link_out_12.svg} | 0 crates/copilot/src/copilot.rs | 76 +++++++++++++--- crates/copilot/src/sign_in.rs | 20 ++--- crates/copilot_button/src/copilot_button.rs | 87 ++++++++++++++---- crates/theme/src/theme.rs | 4 +- crates/workspace/src/notifications.rs | 40 +++++++-- crates/workspace/src/workspace.rs | 89 ++++++++++++++++++- styles/src/styleTree/copilot.ts | 20 +---- .../styleTree/simpleMessageNotification.ts | 16 +++- 9 files changed, 279 insertions(+), 73 deletions(-) rename assets/icons/{maybe_link_out.svg => link_out_12.svg} (100%) diff --git a/assets/icons/maybe_link_out.svg b/assets/icons/link_out_12.svg similarity index 100% rename from assets/icons/maybe_link_out.svg rename to assets/icons/link_out_12.svg diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 8a72f57b95..727b9d2d4f 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -28,7 +28,10 @@ const COPILOT_AUTH_NAMESPACE: &'static str = "copilot_auth"; actions!(copilot_auth, [SignIn, SignOut]); const COPILOT_NAMESPACE: &'static str = "copilot"; -actions!(copilot, [NextSuggestion, PreviousSuggestion, Toggle]); +actions!( + copilot, + [NextSuggestion, PreviousSuggestion, Toggle, Reinstall] +); pub fn init(client: Arc, node_runtime: Arc, cx: &mut MutableAppContext) { let copilot = cx.add_model(|cx| Copilot::start(client.http_client(), node_runtime, cx)); @@ -46,6 +49,13 @@ pub fn init(client: Arc, node_runtime: Arc, cx: &mut Mutabl .detach_and_log_err(cx); }); + cx.add_global_action(|_: &Reinstall, cx| { + let copilot = Copilot::global(cx).unwrap(); + copilot + .update(cx, |copilot, cx| copilot.reinstall(cx)) + .detach(); + }); + cx.observe(&copilot, |handle, cx| { let status = handle.read(cx).status(); cx.update_global::( @@ -73,7 +83,7 @@ pub fn init(client: Arc, node_runtime: Arc, cx: &mut Mutabl enum CopilotServer { Disabled, Starting { - _task: Shared>, + task: Shared>, }, Error(Arc), Started { @@ -97,9 +107,11 @@ enum SignInStatus { SignedOut, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone)] pub enum Status { - Starting, + Starting { + task: Shared>, + }, Error(Arc), Disabled, SignedOut, @@ -123,6 +135,8 @@ pub struct Completion { } pub struct Copilot { + http: Arc, + node_runtime: Arc, server: CopilotServer, } @@ -131,6 +145,13 @@ impl Entity for Copilot { } impl Copilot { + pub fn starting_task(&self) -> Option>> { + match self.server { + CopilotServer::Starting { ref task } => Some(task.clone()), + _ => None, + } + } + pub fn global(cx: &AppContext) -> Option> { if cx.has_global::>() { Some(cx.global::>().clone()) @@ -159,10 +180,12 @@ impl Copilot { } }) .shared(); - this.server = CopilotServer::Starting { _task: start_task } + this.server = CopilotServer::Starting { task: start_task }; + cx.notify(); } } else { - this.server = CopilotServer::Disabled + this.server = CopilotServer::Disabled; + cx.notify(); } } }) @@ -178,10 +201,14 @@ impl Copilot { .shared(); Self { - server: CopilotServer::Starting { _task: start_task }, + http, + node_runtime, + server: CopilotServer::Starting { task: start_task }, } } else { Self { + http, + node_runtime, server: CopilotServer::Disabled, } } @@ -332,6 +359,27 @@ impl Copilot { } } + fn reinstall(&mut self, cx: &mut ModelContext) -> Task<()> { + let start_task = cx + .spawn({ + let http = self.http.clone(); + let node_runtime = self.node_runtime.clone(); + move |this, cx| async move { + clear_copilot_dir().await; + Self::start_language_server(http, node_runtime, this, cx).await + } + }) + .shared(); + + self.server = CopilotServer::Starting { + task: start_task.clone(), + }; + + cx.notify(); + + cx.foreground().spawn(start_task) + } + pub fn completion( &self, buffer: &ModelHandle, @@ -391,7 +439,7 @@ impl Copilot { pub fn status(&self) -> Status { match &self.server { - CopilotServer::Starting { .. } => Status::Starting, + CopilotServer::Starting { task } => Status::Starting { task: task.clone() }, CopilotServer::Disabled => Status::Disabled, CopilotServer::Error(error) => Status::Error(error.clone()), CopilotServer::Started { status, .. } => match status { @@ -501,8 +549,12 @@ fn completion_from_lsp(completion: request::Completion, buffer: &BufferSnapshot) } } +async fn clear_copilot_dir() { + remove_matching(&paths::COPILOT_DIR, |_| true).await +} + async fn get_copilot_lsp(http: Arc) -> anyhow::Result { - const SERVER_PATH: &'static str = "agent.js"; + const SERVER_PATH: &'static str = "dist/agent.js"; ///Check for the latest copilot language server and download it if we haven't already async fn fetch_latest(http: Arc) -> anyhow::Result { @@ -514,6 +566,10 @@ async fn get_copilot_lsp(http: Arc) -> anyhow::Result { let server_path = version_dir.join(SERVER_PATH); if fs::metadata(&server_path).await.is_err() { + // Copilot LSP looks for this dist dir specifcially, so lets add it in. + let dist_dir = version_dir.join("dist"); + fs::create_dir_all(dist_dir.as_path()).await?; + let url = &release .assets .get(0) @@ -526,7 +582,7 @@ async fn get_copilot_lsp(http: Arc) -> anyhow::Result { .map_err(|err| anyhow!("error downloading copilot release: {}", err))?; let decompressed_bytes = GzipDecoder::new(BufReader::new(response.body_mut())); let archive = Archive::new(decompressed_bytes); - archive.unpack(version_dir).await?; + archive.unpack(dist_dir).await?; remove_matching(&paths::COPILOT_DIR, |entry| entry != version_dir).await; } diff --git a/crates/copilot/src/sign_in.rs b/crates/copilot/src/sign_in.rs index cce064160e..168ba712ce 100644 --- a/crates/copilot/src/sign_in.rs +++ b/crates/copilot/src/sign_in.rs @@ -298,9 +298,7 @@ impl CopilotCodeVerification { .with_children([ Flex::row() .with_children([ - theme::ui::svg(&style.auth.copilot_icon).boxed(), - theme::ui::icon(&style.auth.plus_icon).boxed(), - theme::ui::svg(&style.auth.zed_icon).boxed(), + theme::ui::svg(&style.auth.copilot_plus_zed_icon).boxed() ]) .boxed(), Flex::column() @@ -362,9 +360,7 @@ impl CopilotCodeVerification { .with_children([ Flex::row() .with_children([ - theme::ui::svg(&style.auth.copilot_icon).boxed(), - theme::ui::icon(&style.auth.plus_icon).boxed(), - theme::ui::svg(&style.auth.zed_icon).boxed(), + theme::ui::svg(&style.auth.copilot_plus_zed_icon).boxed() ]) .boxed(), Label::new("Copilot Enabled!", style.auth.enable_text.clone()).boxed(), @@ -410,9 +406,7 @@ impl CopilotCodeVerification { .with_children([ Flex::row() .with_children([ - theme::ui::svg(&style.auth.copilot_icon).boxed(), - theme::ui::icon(&style.auth.plus_icon).boxed(), - theme::ui::svg(&style.auth.zed_icon).boxed(), + theme::ui::svg(&style.auth.copilot_plus_zed_icon).boxed() ]) .boxed(), Flex::column() @@ -483,13 +477,13 @@ impl View for CopilotCodeVerification { } fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> gpui::ElementBox { - let style = cx.global::().theme.copilot.clone(); + let style = cx.global::().theme.clone(); match &self.status { Status::SigningIn { prompt: Some(prompt), - } => Self::render_prompting_modal(&prompt, &style, cx), - Status::Unauthorized => Self::render_unauthorized_modal(&style, cx), - Status::Authorized => Self::render_enabled_modal(&style, cx), + } => Self::render_prompting_modal(&prompt, &style.copilot, cx), + Status::Unauthorized => Self::render_unauthorized_modal(&style.copilot, cx), + Status::Authorized => Self::render_enabled_modal(&style.copilot, cx), _ => Empty::new().boxed(), } } diff --git a/crates/copilot_button/src/copilot_button.rs b/crates/copilot_button/src/copilot_button.rs index e61a8814af..7a0a45da82 100644 --- a/crates/copilot_button/src/copilot_button.rs +++ b/crates/copilot_button/src/copilot_button.rs @@ -8,12 +8,15 @@ use gpui::{ }; use settings::{settings_file::SettingsFile, Settings}; use workspace::{ - item::ItemHandle, notifications::simple_message_notification::OsOpen, StatusItemView, + item::ItemHandle, notifications::simple_message_notification::OsOpen, DismissToast, + StatusItemView, }; -use copilot::{Copilot, SignIn, SignOut, Status}; +use copilot::{Copilot, Reinstall, SignIn, SignOut, Status}; const COPILOT_SETTINGS_URL: &str = "https://github.com/settings/copilot"; +const COPILOT_STARTING_TOAST_ID: usize = 1337; +const COPILOT_ERROR_TOAST_ID: usize = 1338; #[derive(Clone, PartialEq)] pub struct DeployCopilotMenu; @@ -36,7 +39,7 @@ impl_internal_actions!( DeployCopilotMenu, DeployCopilotModal, ToggleCopilotForLanguage, - ToggleCopilotGlobally + ToggleCopilotGlobally, ] ); @@ -93,14 +96,18 @@ impl View for CopilotButton { } let theme = settings.theme.clone(); - let active = self.popup_menu.read(cx).visible() /* || modal.is_shown */; - let authorized = Copilot::global(cx).unwrap().read(cx).status() == Status::Authorized; + let active = self.popup_menu.read(cx).visible(); + let status = Copilot::global(cx).unwrap().read(cx).status(); + let enabled = self.editor_enabled.unwrap_or(settings.copilot_on(None)); + let view_id = cx.view_id(); + Stack::new() .with_child( MouseEventHandler::::new(0, cx, { let theme = theme.clone(); + let status = status.clone(); move |state, _cx| { let style = theme .workspace @@ -112,14 +119,16 @@ impl View for CopilotButton { Flex::row() .with_child( Svg::new({ - if authorized { - if enabled { - "icons/copilot_16.svg" - } else { - "icons/copilot_disabled_16.svg" + match status { + Status::Error(_) => "icons/copilot_error_16.svg", + Status::Authorized => { + if enabled { + "icons/copilot_16.svg" + } else { + "icons/copilot_disabled_16.svg" + } } - } else { - "icons/copilot_init_16.svg" + _ => "icons/copilot_init_16.svg", } }) .with_color(style.icon_color) @@ -136,11 +145,50 @@ impl View for CopilotButton { } }) .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, move |_, cx| { - if authorized { - cx.dispatch_action(DeployCopilotMenu); - } else { - cx.dispatch_action(SignIn); + .on_click(MouseButton::Left, { + let status = status.clone(); + move |_, cx| match status { + Status::Authorized => cx.dispatch_action(DeployCopilotMenu), + Status::Starting { ref task } => { + cx.dispatch_action(workspace::Toast::new( + COPILOT_STARTING_TOAST_ID, + "Copilot is starting...", + )); + let window_id = cx.window_id(); + let task = task.to_owned(); + cx.spawn(|mut cx| async move { + task.await; + cx.update(|cx| { + let status = Copilot::global(cx).unwrap().read(cx).status(); + match status { + Status::Authorized => cx.dispatch_action_at( + window_id, + view_id, + workspace::Toast::new( + COPILOT_STARTING_TOAST_ID, + "Copilot has started!", + ), + ), + _ => { + cx.dispatch_action_at( + window_id, + view_id, + DismissToast::new(COPILOT_STARTING_TOAST_ID), + ); + cx.dispatch_global_action(SignIn) + } + } + }) + }) + .detach(); + } + Status::Error(ref e) => cx.dispatch_action(workspace::Toast::new_action( + COPILOT_ERROR_TOAST_ID, + format!("Copilot can't be started: {}", e), + "Reinstall Copilot", + Reinstall, + )), + _ => cx.dispatch_action(SignIn), } }) .with_tooltip::( @@ -195,9 +243,9 @@ impl CopilotButton { let locally_enabled = self.editor_enabled.unwrap_or(settings.copilot_on(None)); menu_options.push(ContextMenuItem::item_for_view( if locally_enabled { - "Pause Copilot for file" + "Pause Copilot for this file" } else { - "Resume Copilot for file" + "Resume Copilot for this file" }, *view_id, copilot::Toggle, @@ -244,6 +292,7 @@ impl CopilotButton { Label::new("Copilot Settings", style.label.clone()).boxed(), theme::ui::icon(icon_style.style_for(state, false)).boxed(), ]) + .align_children_center() .boxed() }, ), diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 7c9f42c2f5..1c7d0eba95 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -131,9 +131,7 @@ pub struct CopilotAuth { pub instruction_text: TextStyle, pub cta_button: ButtonStyle, pub content_width: f32, - pub copilot_icon: SvgStyle, - pub plus_icon: IconStyle, - pub zed_icon: SvgStyle, + pub copilot_plus_zed_icon: SvgStyle, pub device_code_group: ContainerStyle, pub github_group: ContainerStyle, pub header_group: ContainerStyle, diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index f19f876be5..1cb5d3f50d 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -97,7 +97,7 @@ impl Workspace { let notification = build_notification(cx); cx.subscribe(¬ification, move |this, handle, event, cx| { if handle.read(cx).should_dismiss_notification_on_event(event) { - this.dismiss_notification(type_id, id, cx); + this.dismiss_notification_internal(type_id, id, cx); } }) .detach(); @@ -107,7 +107,18 @@ impl Workspace { } } - fn dismiss_notification(&mut self, type_id: TypeId, id: usize, cx: &mut ViewContext) { + pub fn dismiss_notification(&mut self, id: usize, cx: &mut ViewContext) { + let type_id = TypeId::of::(); + + self.dismiss_notification_internal(type_id, id, cx) + } + + fn dismiss_notification_internal( + &mut self, + type_id: TypeId, + id: usize, + cx: &mut ViewContext, + ) { self.notifications .retain(|(existing_type_id, existing_id, _)| { if (*existing_type_id, *existing_id) == (type_id, id) { @@ -183,6 +194,18 @@ pub mod simple_message_notification { } } + pub fn new_boxed_action>, S2: Into>>( + message: S1, + click_action: Box, + click_message: S2, + ) -> Self { + Self { + message: message.into(), + click_action: Some(click_action), + click_message: Some(click_message.into()), + } + } + pub fn new>, A: Action, S2: Into>>( message: S1, click_action: A, @@ -270,9 +293,13 @@ pub mod simple_message_notification { let style = theme.action_message.style_for(state, false); if let Some(click_message) = click_message { Some( - Text::new(click_message, style.text.clone()) - .contained() - .with_style(style.container) + Flex::row() + .with_child( + Text::new(click_message, style.text.clone()) + .contained() + .with_style(style.container) + .boxed(), + ) .boxed(), ) } else { @@ -288,7 +315,8 @@ pub mod simple_message_notification { .on_up(MouseButton::Left, |_, _| {}) .on_click(MouseButton::Left, move |_, cx| { if let Some(click_action) = click_action.as_ref() { - cx.dispatch_any_action(click_action.boxed_clone()) + cx.dispatch_any_action(click_action.boxed_clone()); + cx.dispatch_action(CancelMessageNotification) } }) .with_cursor_style(if has_click_action { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 83b87b9221..3fffe57e3e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -41,10 +41,10 @@ use gpui::{ impl_actions, impl_internal_actions, keymap_matcher::KeymapContext, platform::{CursorStyle, WindowOptions}, - AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, - MouseButton, MutableAppContext, PathPromptOptions, Platform, PromptLevel, RenderContext, - SizeConstraint, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, - WindowBounds, + Action, AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, + ModelHandle, MouseButton, MutableAppContext, PathPromptOptions, Platform, PromptLevel, + RenderContext, SizeConstraint, Subscription, Task, View, ViewContext, ViewHandle, + WeakViewHandle, WindowBounds, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ProjectItem}; use language::LanguageRegistry; @@ -165,6 +165,67 @@ pub struct OpenProjectEntryInPane { project_entry: ProjectEntryId, } +pub struct Toast { + id: usize, + msg: Cow<'static, str>, + click: Option<(Cow<'static, str>, Box)>, +} + +impl Toast { + pub fn new>>(id: usize, msg: I) -> Self { + Toast { + id, + msg: msg.into(), + click: None, + } + } + + pub fn new_action>, I2: Into>>( + id: usize, + msg: I1, + click_msg: I2, + action: impl Action, + ) -> Self { + Toast { + id, + msg: msg.into(), + click: Some((click_msg.into(), Box::new(action))), + } + } +} + +impl PartialEq for Toast { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + && self.msg == other.msg + && self.click.is_some() == other.click.is_some() + } +} + +impl Clone for Toast { + fn clone(&self) -> Self { + Toast { + id: self.id, + msg: self.msg.to_owned(), + click: self + .click + .as_ref() + .map(|(msg, click)| (msg.to_owned(), click.boxed_clone())), + } + } +} + +#[derive(Clone, PartialEq)] +pub struct DismissToast { + id: usize, +} + +impl DismissToast { + pub fn new(id: usize) -> Self { + DismissToast { id } + } +} + pub type WorkspaceId = i64; impl_internal_actions!( @@ -178,6 +239,8 @@ impl_internal_actions!( SplitWithItem, SplitWithProjectEntry, OpenProjectEntryInPane, + Toast, + DismissToast ] ); impl_actions!(workspace, [ActivatePane]); @@ -353,6 +416,24 @@ pub fn init(app_state: Arc, cx: &mut MutableAppContext) { .detach(); }); + cx.add_action(|workspace: &mut Workspace, alert: &Toast, cx| { + workspace.dismiss_notification::(alert.id, cx); + workspace.show_notification(alert.id, cx, |cx| { + cx.add_view(|_cx| match &alert.click { + Some((click_msg, action)) => MessageNotification::new_boxed_action( + alert.msg.clone(), + action.boxed_clone(), + click_msg.clone(), + ), + None => MessageNotification::new_message(alert.msg.clone()), + }) + }) + }); + + cx.add_action(|workspace: &mut Workspace, alert: &DismissToast, cx| { + workspace.dismiss_notification::(alert.id, cx); + }); + let client = &app_state.client; client.add_view_request_handler(Workspace::handle_follow); client.add_view_message_handler(Workspace::handle_unfollow); diff --git a/styles/src/styleTree/copilot.ts b/styles/src/styleTree/copilot.ts index 106fed298f..f25bc08103 100644 --- a/styles/src/styleTree/copilot.ts +++ b/styles/src/styleTree/copilot.ts @@ -31,13 +31,13 @@ export default function copilot(colorScheme: ColorScheme) { return { outLinkIcon: { - icon: svg(foreground(layer, "variant"), "icons/maybe_link_out.svg", 12, 12), + icon: svg(foreground(layer, "variant"), "icons/link_out_12.svg", 12, 12), container: { cornerRadius: 6, - padding: { top: 6, bottom: 6, left: 6, right: 6 }, + padding: { left: 6 }, }, hover: { - icon: svg(foreground(layer, "hovered"), "icons/maybe_link_out.svg", 12, 12) + icon: svg(foreground(layer, "hovered"), "icons/link_out_12.svg", 12, 12) }, }, modal: { @@ -103,19 +103,7 @@ export default function copilot(colorScheme: ColorScheme) { right: 0 } }, - copilotIcon: svg(foreground(layer, "default"), "icons/github-copilot-dummy.svg", 32, 32), - plusIcon: { - icon: svg(foreground(layer, "default"), "icons/plus_12.svg", 12, 12), - container: { - padding: { - top: 12, - bottom: 12, - left: 12, - right: 12, - } - } - }, - zedIcon: svg(foreground(layer, "default"), "icons/logo_96.svg", 32, 32), + copilotPlusZedIcon: svg(foreground(layer, "default"), "icons/zed_plus_copilot_32.svg", 32, 92), enableText: text(layer, "sans", { size: "md" }), enableGroup: { margin: { diff --git a/styles/src/styleTree/simpleMessageNotification.ts b/styles/src/styleTree/simpleMessageNotification.ts index 36b295c640..dde689e9bd 100644 --- a/styles/src/styleTree/simpleMessageNotification.ts +++ b/styles/src/styleTree/simpleMessageNotification.ts @@ -1,5 +1,5 @@ import { ColorScheme } from "../themes/common/colorScheme" -import { foreground, text } from "./components" +import { background, border, foreground, text } from "./components" const headerPadding = 8 @@ -14,9 +14,21 @@ export default function simpleMessageNotification( }, actionMessage: { ...text(layer, "sans", { size: "xs" }), + border: border(layer, "active"), + cornerRadius: 4, + padding: { + top: 3, + bottom: 3, + left: 7, + right: 7, + }, + + margin: { left: headerPadding, top: 6, bottom: 6 }, hover: { - color: foreground(layer, "hovered"), + ...text(layer, "sans", "default", { size: "xs" }), + background: background(layer, "hovered"), + border: border(layer, "active"), }, }, dismissButton: {