Fix copy button not working for REPL error output (#40669)

h-michaelson20 created 2 months ago

## Description

Fixes the copy button functionality in REPL interactive mode error
output sections.

When executing Python code that produces errors in the REPL (e.g.,
`NameError`), the copy button in the error output section was
unresponsive. The stdout/stderr copy button worked correctly, but the
error traceback section copy button had no effect when clicked.

Fixes #40207

## Changes

Modified the following:
src/outputs.rs: Fixed context issues in render_output_controls by
replacing cx.listener() with simple closures, and added custom button
implementation for ErrorOutput that copies/opens the complete error
(name + message + traceback)
src/outputs/plain.rs: Made full_text() method public to allow access
from button handlers
src/outputs/user_error.rs: Added Clone derive to ErrorView struct and
removed a couple pieces of commented code

## Why This Matters

The copy button was clearly broken and it is useful to have for REPL
workflows. Users could potentially need to copy error messages for a
variety of reasons.

## Testing

See attached demo for proof that the fix is working as intended. (this
is my first ever commit, if there are additional test cases I need to
write or run, please let me know!)


https://github.com/user-attachments/assets/da158205-4119-47eb-a271-196ef8d196e4

Release Notes:

- Fixed copy button not working for REPL error output

Change summary

crates/repl/src/outputs.rs            | 96 ++++++++++++++++++++++++++--
crates/repl/src/outputs/plain.rs      |  2 
crates/repl/src/outputs/user_error.rs |  8 -
3 files changed, 91 insertions(+), 15 deletions(-)

Detailed changes

crates/repl/src/outputs.rs

@@ -38,7 +38,8 @@ use gpui::{AnyElement, ClipboardItem, Entity, Render, WeakEntity};
 use language::Buffer;
 use runtimelib::{ExecutionState, JupyterMessageContent, MimeBundle, MimeType};
 use ui::{
-    CommonAnimationExt, Context, IntoElement, Styled, Tooltip, Window, div, prelude::*, v_flex,
+    ButtonStyle, CommonAnimationExt, Context, IconButton, IconName, IntoElement, Styled, Tooltip,
+    Window, div, h_flex, prelude::*, v_flex,
 };
 
 mod image;
@@ -146,13 +147,13 @@ impl Output {
                         IconButton::new(ElementId::Name("copy-output".into()), IconName::Copy)
                             .style(ButtonStyle::Transparent)
                             .tooltip(Tooltip::text("Copy Output"))
-                            .on_click(cx.listener(move |_, _, window, cx| {
+                            .on_click(move |_, window, cx| {
                                 let clipboard_content = v.clipboard_content(window, cx);
 
                                 if let Some(clipboard_content) = clipboard_content.as_ref() {
                                     cx.write_to_clipboard(clipboard_content.clone());
                                 }
-                            })),
+                            }),
                     )
                 })
                 .when(v.has_buffer_content(window, cx), |el| {
@@ -164,10 +165,9 @@ impl Output {
                         )
                         .style(ButtonStyle::Transparent)
                         .tooltip(Tooltip::text("Open in Buffer"))
-                        .on_click(cx.listener({
+                        .on_click({
                             let workspace = workspace.clone();
-
-                            move |_, _, window, cx| {
+                            move |_, window, cx| {
                                 let buffer_content =
                                     v.update(cx, |item, cx| item.buffer_content(window, cx));
 
@@ -193,7 +193,7 @@ impl Output {
                                         .ok();
                                 }
                             }
-                        })),
+                        }),
                     )
                 })
                 .into_any_element(),
@@ -237,7 +237,87 @@ impl Output {
                     Self::render_output_controls(content.clone(), workspace, window, cx)
                 }
                 Self::ErrorOutput(err) => {
-                    Self::render_output_controls(err.traceback.clone(), workspace, window, cx)
+                    // Add buttons for the traceback section
+                    Some(
+                        h_flex()
+                            .pl_1()
+                            .child(
+                                IconButton::new(
+                                    ElementId::Name("copy-full-error-traceback".into()),
+                                    IconName::Copy,
+                                )
+                                .style(ButtonStyle::Transparent)
+                                .tooltip(Tooltip::text("Copy Full Error"))
+                                .on_click({
+                                    let ename = err.ename.clone();
+                                    let evalue = err.evalue.clone();
+                                    let traceback = err.traceback.clone();
+                                    move |_, _window, cx| {
+                                        let traceback_text = traceback.read(cx).full_text();
+                                        let full_error =
+                                            format!("{}: {}\n{}", ename, evalue, traceback_text);
+                                        let clipboard_content =
+                                            ClipboardItem::new_string(full_error);
+                                        cx.write_to_clipboard(clipboard_content);
+                                    }
+                                }),
+                            )
+                            .child(
+                                IconButton::new(
+                                    ElementId::Name("open-full-error-in-buffer-traceback".into()),
+                                    IconName::FileTextOutlined,
+                                )
+                                .style(ButtonStyle::Transparent)
+                                .tooltip(Tooltip::text("Open Full Error in Buffer"))
+                                .on_click({
+                                    let ename = err.ename.clone();
+                                    let evalue = err.evalue.clone();
+                                    let traceback = err.traceback.clone();
+                                    move |_, window, cx| {
+                                        if let Some(workspace) = workspace.upgrade() {
+                                            let traceback_text = traceback.read(cx).full_text();
+                                            let full_error = format!(
+                                                "{}: {}\n{}",
+                                                ename, evalue, traceback_text
+                                            );
+                                            let buffer = cx.new(|cx| {
+                                                let mut buffer = Buffer::local(full_error, cx)
+                                                    .with_language(
+                                                        language::PLAIN_TEXT.clone(),
+                                                        cx,
+                                                    );
+                                                buffer.set_capability(
+                                                    language::Capability::ReadOnly,
+                                                    cx,
+                                                );
+                                                buffer
+                                            });
+                                            let editor = Box::new(cx.new(|cx| {
+                                                let multibuffer = cx.new(|cx| {
+                                                    let mut multi_buffer =
+                                                        MultiBuffer::singleton(buffer.clone(), cx);
+                                                    multi_buffer
+                                                        .set_title("Full Error".to_string(), cx);
+                                                    multi_buffer
+                                                });
+                                                Editor::for_multibuffer(
+                                                    multibuffer,
+                                                    None,
+                                                    window,
+                                                    cx,
+                                                )
+                                            }));
+                                            workspace.update(cx, |workspace, cx| {
+                                                workspace.add_item_to_active_pane(
+                                                    editor, None, true, window, cx,
+                                                );
+                                            });
+                                        }
+                                    }
+                                }),
+                            )
+                            .into_any_element(),
+                    )
                 }
                 Self::Message(_) => None,
                 Self::Table { content, .. } => {

crates/repl/src/outputs/plain.rs

@@ -197,7 +197,7 @@ impl TerminalOutput {
         }
     }
 
-    fn full_text(&self) -> String {
+    pub fn full_text(&self) -> String {
         fn sanitize(mut line: String) -> Option<String> {
             line.retain(|ch| ch != '\u{0}' && ch != '\r');
             if line.trim().is_empty() {

crates/repl/src/outputs/user_error.rs

@@ -4,6 +4,7 @@ use ui::{Label, h_flex, prelude::*, v_flex};
 use crate::outputs::plain::TerminalOutput;
 
 /// Userspace error from the kernel
+#[derive(Clone)]
 pub struct ErrorView {
     pub ename: String,
     pub evalue: String,
@@ -24,15 +25,10 @@ impl ErrorView {
                         .font_buffer(cx)
                         .child(
                             Label::new(format!("{}: ", self.ename.clone()))
-                                // .size(LabelSize::Large)
                                 .color(Color::Error)
                                 .weight(FontWeight::BOLD),
                         )
-                        .child(
-                            Label::new(self.evalue.clone())
-                                // .size(LabelSize::Large)
-                                .weight(FontWeight::BOLD),
-                        ),
+                        .child(Label::new(self.evalue.clone()).weight(FontWeight::BOLD)),
                 )
                 .child(
                     div()