From caf222a71d29b932fe4f1c315acc37cf808104f5 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Wed, 14 Aug 2024 17:18:39 +0200 Subject: [PATCH] assistant: Show errors without mouse interaction (#16226) https://github.com/user-attachments/assets/a8e5e0ce-349d-4836-afe6-fc960a307c9f Release Notes: - N/A --------- Co-authored-by: Thorsten Co-authored-by: Danilo --- crates/assistant/src/assistant_panel.rs | 152 +++++++++++++----------- crates/assistant/src/context.rs | 10 +- 2 files changed, 87 insertions(+), 75 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 7ad125fb31..bfb95b9e5a 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -1973,6 +1973,8 @@ impl ContextEditor { } fn cancel(&mut self, _: &editor::actions::Cancel, cx: &mut ViewContext) { + self.error_message = None; + if self .context .update(cx, |context, cx| context.cancel_last_assist(cx)) @@ -2348,8 +2350,8 @@ impl ContextEditor { } } ContextEvent::Operation(_) => {} - ContextEvent::AssistError(error_message) => { - self.error_message = Some(SharedString::from(error_message.clone())); + ContextEvent::ShowAssistError(error_message) => { + self.error_message = Some(error_message.clone()); } } } @@ -3045,40 +3047,36 @@ impl ContextEditor { .gap_1() .child(sender) .children(match &message.status { - MessageStatus::Error(error) => { - let error_popover_trigger = - Button::new("show-error", "Error") - .color(Color::Error) - .selected_label_color(Color::Error) - .selected_icon_color(Color::Error) - .icon(IconName::XCircle) - .icon_color(Color::Error) - .icon_size(IconSize::Small) - .icon_position(IconPosition::Start) - .tooltip(move |cx| { - Tooltip::with_meta( - "Error interacting with language model", - None, - "Click for more details", - cx, - ) - }); - - Some( - PopoverMenu::new("show-error-popover") - .menu({ - let error = error.clone(); - move |cx| { - Some(cx.new_view(|cx| ErrorPopover { - error: error.clone(), - focus_handle: cx.focus_handle(), - })) - } - }) - .trigger(error_popover_trigger) - .into_any_element(), - ) - } + MessageStatus::Error(error) => Some( + Button::new("show-error", "Error") + .color(Color::Error) + .selected_label_color(Color::Error) + .selected_icon_color(Color::Error) + .icon(IconName::XCircle) + .icon_color(Color::Error) + .icon_size(IconSize::Small) + .icon_position(IconPosition::Start) + .tooltip(move |cx| { + Tooltip::with_meta( + "Error interacting with language model", + None, + "Click for more details", + cx, + ) + }) + .on_click({ + let context = context.clone(); + let error = error.clone(); + move |_, cx| { + context.update(cx, |_, cx| { + cx.emit(ContextEvent::ShowAssistError( + error.clone(), + )); + }); + } + }) + .into_any_element(), + ), MessageStatus::Canceled => Some( ButtonLike::new("canceled") .child( @@ -3579,10 +3577,13 @@ impl ContextEditor { }; let provider = LanguageModelRegistry::read_global(cx).active_provider(); - let disabled = self.show_accept_terms + + let needs_to_accept_terms = self.show_accept_terms && provider .as_ref() .map_or(false, |provider| provider.must_accept_terms(cx)); + let has_active_error = self.error_message.is_some(); + let disabled = needs_to_accept_terms || has_active_error; ButtonLike::new("send_button") .disabled(disabled) @@ -3677,6 +3678,48 @@ impl Render for ContextEditor { .child(element), ) }) + .when_some(self.error_message.clone(), |this, error_message| { + this.child( + div() + .absolute() + .right_4() + .bottom_10() + .max_w_96() + .py_2() + .px_3() + .elevation_2(cx) + .occlude() + .child( + v_flex() + .gap_0p5() + .child( + h_flex() + .gap_1() + .items_center() + .child(Icon::new(IconName::XCircle).color(Color::Error)) + .child( + Label::new("Error interacting with language model") + .weight(FontWeight::SEMIBOLD), + ), + ) + .child( + div() + .id("error-message") + .max_h_24() + .overflow_y_scroll() + .child(Label::new(error_message)), + ) + .child(h_flex().justify_end().mt_1().child( + Button::new("dismiss", "Dismiss").on_click(cx.listener( + |this, _, cx| { + this.error_message = None; + cx.notify(); + }, + )), + )), + ), + ) + }) .child( h_flex().flex_none().relative().child( h_flex() @@ -3691,41 +3734,6 @@ impl Render for ContextEditor { } } -struct ErrorPopover { - error: SharedString, - focus_handle: FocusHandle, -} - -impl EventEmitter for ErrorPopover {} - -impl FocusableView for ErrorPopover { - fn focus_handle(&self, _: &AppContext) -> FocusHandle { - self.focus_handle.clone() - } -} - -impl Render for ErrorPopover { - fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { - v_flex() - .mt_2() - .max_w_96() - .py_2() - .px_3() - .gap_0p5() - .elevation_2(cx) - .bg(cx.theme().colors().surface_background) - .occlude() - .child(Label::new("Error interacting with language model").weight(FontWeight::SEMIBOLD)) - .child(Label::new(self.error.clone())) - .child( - h_flex().justify_end().mt_1().child( - Button::new("dismiss", "Dismiss") - .on_click(cx.listener(|_, _, cx| cx.emit(DismissEvent))), - ), - ) - } -} - impl FocusableView for ContextEditor { fn focus_handle(&self, cx: &AppContext) -> FocusHandle { self.editor.focus_handle(cx) diff --git a/crates/assistant/src/context.rs b/crates/assistant/src/context.rs index b0d489924d..b98dc64d96 100644 --- a/crates/assistant/src/context.rs +++ b/crates/assistant/src/context.rs @@ -285,7 +285,7 @@ impl ContextOperation { #[derive(Debug, Clone)] pub enum ContextEvent { - AssistError(String), + ShowAssistError(SharedString), MessagesEdited, SummaryChanged, WorkflowStepsRemoved(Vec>), @@ -1790,7 +1790,9 @@ impl Context { .map(|error| error.to_string().trim().to_string()); if let Some(error_message) = error_message.as_ref() { - cx.emit(ContextEvent::AssistError(error_message.to_string())); + cx.emit(ContextEvent::ShowAssistError(SharedString::from( + error_message.clone(), + ))); } this.update_metadata(assistant_message_id, cx, |metadata| { @@ -1843,7 +1845,9 @@ impl Context { pub fn cancel_last_assist(&mut self, cx: &mut ModelContext) -> bool { if let Some(pending_completion) = self.pending_completions.pop() { self.update_metadata(pending_completion.assistant_message_id, cx, |metadata| { - metadata.status = MessageStatus::Canceled; + if metadata.status == MessageStatus::Pending { + metadata.status = MessageStatus::Canceled; + } }); true } else {