agent: Improve terminal tool card design (#29712)

To-dos:

- [x] Expose the command to defend against cases where that's just super
long
- [x] Tackle the vertical scroll conflict with panel scroll
- [x] Reduce default font-size

Release Notes:

- N/A

---------

Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
Co-authored-by: Agus Zubiaga <hi@aguz.me>

crates/assistant_tools/src/edit_file_tool.rs

@@ -708,7 +708,6 @@ impl ToolCard for EditFileToolCard {
                                 .cursor_pointer()
                                 .h_5()
                                 .justify_center()
-                                .rounded_b_md()
                                 .border_t_1()
                                 .border_color(border_color)
                                 .bg(cx.theme().colors().editor_background)

crates/assistant_tools/src/terminal_tool.rs

@@ -1,10 +1,7 @@
 use crate::schema::json_schema_for;
 use anyhow::{Context as _, Result, anyhow, bail};
 use assistant_tool::{ActionLog, Tool, ToolCard, ToolResult, ToolUseStatus};
-use gpui::{
+use gpui::{AnyWindowHandle, App, AppContext, Empty, Entity, EntityId, Task, WeakEntity, Window};
-    Animation, AnimationExt, AnyWindowHandle, App, AppContext, Empty, Entity, EntityId, Task,
-    Transformation, WeakEntity, Window, percentage,
-};
 use language::LineEnding;
 use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat};
 use portable_pty::{CommandBuilder, PtySize, native_pty_system};
@@ -19,7 +16,7 @@ use std::{
     time::{Duration, Instant},
 };
 use terminal_view::TerminalView;
-use ui::{Disclosure, IconName, Tooltip, prelude::*};
+use ui::{Disclosure, Tooltip, prelude::*};
 use util::{
     get_system_shell, markdown::MarkdownInlineCode, size::format_file_size,
     time::duration_alt_display,
@@ -175,11 +172,13 @@ impl Tool for TerminalTool {
                             workspace.downgrade(),
                             None,
                             project.downgrade(),
+                            true,
                             window,
                             cx,
                         )
                     })
                 })?;
+
                 let _ = card.update(cx, |card, _| {
                     card.terminal = Some(terminal_view.clone());
                     card.start_instant = Instant::now();
@@ -378,155 +377,118 @@ impl ToolCard for TerminalToolCard {
         };
 
         let tool_failed = matches!(status, ToolUseStatus::Error(_));
+
         let command_failed =
             self.command_finished && self.exit_status.is_none_or(|code| !code.success());
+
         if (tool_failed || command_failed) && self.elapsed_time.is_none() {
             self.elapsed_time = Some(self.start_instant.elapsed());
         }
         let time_elapsed = self
             .elapsed_time
             .unwrap_or_else(|| self.start_instant.elapsed());
-        let should_hide_terminal =
+        let should_hide_terminal = tool_failed || self.finished_with_empty_output;
-            tool_failed || self.finished_with_empty_output || !self.preview_expanded;
 
-        let border_color = cx.theme().colors().border.opacity(0.6);
         let header_bg = cx
             .theme()
             .colors()
             .element_background
             .blend(cx.theme().colors().editor_foreground.opacity(0.025));
 
-        let header_label = h_flex()
+        let border_color = cx.theme().colors().border.opacity(0.6);
-            .w_full()
+
-            .max_w_full()
+        let path = self
-            .px_1()
+            .working_dir
-            .gap_0p5()
+            .as_ref()
-            .opacity(0.8)
+            .cloned()
-            .child(
+            .or_else(|| env::current_dir().ok())
-                h_flex()
+            .map(|path| format!("{}", path.display()))
-                    .child(
+            .unwrap_or_else(|| "current directory".to_string());
-                        Icon::new(IconName::Terminal)
-                            .size(IconSize::XSmall)
-                            .color(Color::Muted),
-                    )
-                    .child(
-                        div()
-                            .id(("terminal-tool-header-input-command", self.entity_id))
-                            .text_size(rems(0.8125))
-                            .font_buffer(cx)
-                            .child(self.input_command.clone())
-                            .ml_1p5()
-                            .mr_0p5()
-                            .tooltip({
-                                let path = self
-                                    .working_dir
-                                    .as_ref()
-                                    .cloned()
-                                    .or_else(|| env::current_dir().ok())
-                                    .map(|path| format!("\"{}\"", path.display()))
-                                    .unwrap_or_else(|| "current directory".to_string());
-                                Tooltip::text(if self.command_finished {
-                                    format!("Ran in {path}")
-                                } else {
-                                    format!("Running in {path}")
-                                })
-                            }),
-                    ),
-            )
-            .into_any_element();
 
         let header = h_flex()
             .flex_none()
-            .p_1()
             .gap_1()
             .justify_between()
             .rounded_t_md()
-            .bg(header_bg)
+            .child(
-            .child(header_label)
+                div()
-            .map(|header| {
+                    .id(("command-target-path", self.entity_id))
-                let header = header
+                    .w_full()
-                    .when(self.was_content_truncated, |header| {
+                    .max_w_full()
-                        let tooltip =
+                    .overflow_x_scroll()
-                            if self.content_line_count + 10 > terminal::MAX_SCROLL_HISTORY_LINES {
+                    .child(
-                                "Output exceeded terminal max lines and was \
+                        Label::new(path)
-                                truncated, the model received the first 16 KB."
+                            .buffer_font(cx)
-                                    .to_string()
+                            .size(LabelSize::XSmall)
-                            } else {
+                            .color(Color::Muted),
-                                format!(
+                    ),
-                                    "Output is {} long, to avoid unexpected token usage, \
+            )
-                                    only 16 KB was sent back to the model.",
+            .when(self.was_content_truncated, |header| {
-                                    format_file_size(self.original_content_len as u64, true),
+                let tooltip = if self.content_line_count + 10 > terminal::MAX_SCROLL_HISTORY_LINES {
-                                )
+                    "Output exceeded terminal max lines and was \
-                            };
+                        truncated, the model received the first 16 KB."
-                        header.child(
+                        .to_string()
-                            div()
-                                .id(("terminal-tool-truncated-label", self.entity_id))
-                                .tooltip(Tooltip::text(tooltip))
-                                .child(
-                                    Label::new("(truncated)")
-                                        .color(Color::Disabled)
-                                        .size(LabelSize::Small),
-                                ),
-                        )
-                    })
-                    .when(time_elapsed > Duration::from_secs(10), |header| {
-                        header.child(
-                            Label::new(format!("({})", duration_alt_display(time_elapsed)))
-                                .buffer_font(cx)
-                                .color(Color::Disabled)
-                                .size(LabelSize::Small),
-                        )
-                    });
-
-                if tool_failed || command_failed {
-                    header.child(
-                        div()
-                            .id(("terminal-tool-error-code-indicator", self.entity_id))
-                            .child(
-                                Icon::new(IconName::Close)
-                                    .size(IconSize::Small)
-                                    .color(Color::Error),
-                            )
-                            .when(command_failed && self.exit_status.is_some(), |this| {
-                                this.tooltip(Tooltip::text(format!(
-                                    "Exited with code {}",
-                                    self.exit_status
-                                        .and_then(|status| status.code())
-                                        .unwrap_or(-1),
-                                )))
-                            })
-                            .when(
-                                !command_failed && tool_failed && status.error().is_some(),
-                                |this| {
-                                    this.tooltip(Tooltip::text(format!(
-                                        "Error: {}",
-                                        status.error().unwrap(),
-                                    )))
-                                },
-                            ),
-                    )
-                } else if self.command_finished {
-                    header.child(
-                        Icon::new(IconName::Check)
-                            .size(IconSize::Small)
-                            .color(Color::Success),
-                    )
                 } else {
-                    header.child(
+                    format!(
-                        Icon::new(IconName::ArrowCircle)
+                        "Output is {} long, to avoid unexpected token usage, \
-                            .size(IconSize::Small)
+                            only 16 KB was sent back to the model.",
-                            .color(Color::Info)
+                        format_file_size(self.original_content_len as u64, true),
-                            .with_animation(
-                                "arrow-circle",
-                                Animation::new(Duration::from_secs(2)).repeat(),
-                                |icon, delta| {
-                                    icon.transform(Transformation::rotate(percentage(delta)))
-                                },
-                            ),
                     )
-                }
+                };
+                header.child(
+                    h_flex()
+                        .id(("terminal-tool-truncated-label", self.entity_id))
+                        .tooltip(Tooltip::text(tooltip))
+                        .gap_1()
+                        .child(
+                            Icon::new(IconName::Info)
+                                .size(IconSize::XSmall)
+                                .color(Color::Ignored),
+                        )
+                        .child(
+                            Label::new("Truncated")
+                                .color(Color::Muted)
+                                .size(LabelSize::Small),
+                        ),
+                )
             })
-            .when(!tool_failed && !self.finished_with_empty_output, |header| {
+            .when(time_elapsed > Duration::from_secs(10), |header| {
+                header.child(
+                    Label::new(format!("({})", duration_alt_display(time_elapsed)))
+                        .buffer_font(cx)
+                        .color(Color::Muted)
+                        .size(LabelSize::Small),
+                )
+            })
+            .when(tool_failed || command_failed, |header| {
+                header.child(
+                    div()
+                        .id(("terminal-tool-error-code-indicator", self.entity_id))
+                        .child(
+                            Icon::new(IconName::Close)
+                                .size(IconSize::Small)
+                                .color(Color::Error),
+                        )
+                        .when(command_failed && self.exit_status.is_some(), |this| {
+                            this.tooltip(Tooltip::text(format!(
+                                "Exited with code {}",
+                                self.exit_status
+                                    .and_then(|status| status.code())
+                                    .unwrap_or(-1),
+                            )))
+                        })
+                        .when(
+                            !command_failed && tool_failed && status.error().is_some(),
+                            |this| {
+                                this.tooltip(Tooltip::text(format!(
+                                    "Error: {}",
+                                    status.error().unwrap(),
+                                )))
+                            },
+                        ),
+                )
+            })
+            .when(!should_hide_terminal, |header| {
                 header.child(
                     Disclosure::new(
                         ("terminal-tool-disclosure", self.entity_id),
@@ -549,9 +511,25 @@ impl ToolCard for TerminalToolCard {
             .border_color(border_color)
             .rounded_lg()
             .overflow_hidden()
-            .child(header)
+            .child(
-            .when(!should_hide_terminal, |this| {
+                v_flex().p_2().gap_0p5().bg(header_bg).child(header).child(
-                this.child(div().child(terminal.clone()).min_h(px(250.0)))
+                    Label::new(self.input_command.clone())
+                        .buffer_font(cx)
+                        .size(LabelSize::Small),
+                ),
+            )
+            .when(self.preview_expanded && !should_hide_terminal, |this| {
+                this.child(
+                    div()
+                        .pt_2()
+                        .min_h_72()
+                        .border_t_1()
+                        .border_color(border_color)
+                        .bg(cx.theme().colors().editor_background)
+                        .rounded_b_md()
+                        .text_ui_sm(cx)
+                        .child(terminal.clone()),
+                )
             })
             .into_any()
     }

crates/debugger_ui/src/session/running.rs

@@ -749,7 +749,15 @@ impl RunningState {
             let terminal = terminal_task.await?;
 
             let terminal_view = cx.new_window_entity(|window, cx| {
-                TerminalView::new(terminal.clone(), workspace, None, weak_project, window, cx)
+                TerminalView::new(
+                    terminal.clone(),
+                    workspace,
+                    None,
+                    weak_project,
+                    false,
+                    window,
+                    cx,
+                )
             })?;
 
             running.update_in(cx, |running, window, cx| {

crates/terminal/src/terminal.rs

@@ -601,6 +601,8 @@ pub struct TerminalContent {
     pub cursor_char: char,
     pub terminal_bounds: TerminalBounds,
     pub last_hovered_word: Option<HoveredWord>,
+    pub scrolled_to_top: bool,
+    pub scrolled_to_bottom: bool,
 }
 
 #[derive(Clone)]
@@ -625,6 +627,8 @@ impl Default for TerminalContent {
             cursor_char: Default::default(),
             terminal_bounds: Default::default(),
             last_hovered_word: None,
+            scrolled_to_top: false,
+            scrolled_to_bottom: false,
         }
     }
 }
@@ -1208,6 +1212,14 @@ impl Terminal {
             .push_back(InternalEvent::Scroll(AlacScroll::Bottom));
     }
 
+    pub fn scrolled_to_top(&self) -> bool {
+        self.last_content.scrolled_to_top
+    }
+
+    pub fn scrolled_to_bottom(&self) -> bool {
+        self.last_content.scrolled_to_bottom
+    }
+
     ///Resize the terminal and the PTY.
     pub fn set_size(&mut self, new_bounds: TerminalBounds) {
         if self.last_content.terminal_bounds != new_bounds {
@@ -1405,6 +1417,8 @@ impl Terminal {
             cursor_char: term.grid()[content.cursor.point].c,
             terminal_bounds: last_content.terminal_bounds,
             last_hovered_word: last_content.last_hovered_word.clone(),
+            scrolled_to_top: content.display_offset == term.history_size(),
+            scrolled_to_bottom: content.display_offset == 0,
         }
     }
 

crates/terminal_view/src/persistence.rs

@@ -261,6 +261,7 @@ async fn deserialize_pane_group(
                             workspace.clone(),
                             Some(workspace_id),
                             project.downgrade(),
+                            false,
                             window,
                             cx,
                         )

crates/terminal_view/src/terminal_element.rs

@@ -1,9 +1,9 @@
 use editor::{CursorLayout, HighlightedRange, HighlightedRangeLine};
 use gpui::{
     AnyElement, App, AvailableSpace, Bounds, ContentMask, Context, DispatchPhase, Element,
-    ElementId, Entity, FocusHandle, Font, FontStyle, FontWeight, GlobalElementId, HighlightStyle,
+    ElementId, Entity, FocusHandle, Focusable, Font, FontStyle, FontWeight, GlobalElementId,
-    Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement, LayoutId,
+    HighlightStyle, Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement,
-    ModifiersChangedEvent, MouseButton, MouseMoveEvent, Pixels, Point, ShapedLine,
+    LayoutId, ModifiersChangedEvent, MouseButton, MouseMoveEvent, Pixels, Point, ShapedLine,
     StatefulInteractiveElement, StrikethroughStyle, Styled, TextRun, TextStyle, UTF16Selection,
     UnderlineStyle, WeakEntity, WhiteSpace, Window, WindowTextSystem, div, fill, point, px,
     relative, size,
@@ -158,6 +158,7 @@ pub struct TerminalElement {
     focused: bool,
     cursor_visible: bool,
     interactivity: Interactivity,
+    embedded: bool,
     block_below_cursor: Option<Rc<BlockProperties>>,
 }
 
@@ -178,6 +179,7 @@ impl TerminalElement {
         focused: bool,
         cursor_visible: bool,
         block_below_cursor: Option<Rc<BlockProperties>>,
+        embedded: bool,
     ) -> TerminalElement {
         TerminalElement {
             terminal,
@@ -187,6 +189,7 @@ impl TerminalElement {
             focus: focus.clone(),
             cursor_visible,
             block_below_cursor,
+            embedded,
             interactivity: Default::default(),
         }
         .track_focus(&focus)
@@ -503,11 +506,15 @@ impl TerminalElement {
         );
         self.interactivity.on_scroll_wheel({
             let terminal_view = self.terminal_view.downgrade();
-            move |e, _, cx| {
+            move |e, window, cx| {
                 terminal_view
                     .update(cx, |terminal_view, cx| {
-                        terminal_view.scroll_wheel(e, cx);
+                        if !terminal_view.embedded
-                        cx.notify();
+                            || terminal_view.focus_handle(cx).is_focused(window)
+                        {
+                            terminal_view.scroll_wheel(e, cx);
+                            cx.notify();
+                        }
                     })
                     .ok();
             }
@@ -580,6 +587,16 @@ impl Element for TerminalElement {
         window: &mut Window,
         cx: &mut App,
     ) -> (LayoutId, Self::RequestLayoutState) {
+        if self.embedded {
+            let scrollable = {
+                let term = self.terminal.read(cx);
+                !term.scrolled_to_top() && !term.scrolled_to_bottom() && self.focused
+            };
+            if scrollable {
+                self.interactivity.occlude_mouse();
+            }
+        }
+
         let layout_id =
             self.interactivity
                 .request_layout(global_id, window, cx, |mut style, window, cx| {
@@ -636,10 +653,14 @@ impl Element for TerminalElement {
                 let font_weight = terminal_settings.font_weight.unwrap_or_default();
 
                 let line_height = terminal_settings.line_height.value();
-                let font_size = terminal_settings.font_size;
 
-                let font_size =
+                let font_size = if self.embedded {
-                    font_size.map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx));
+                    window.text_style().font_size.to_pixels(window.rem_size())
+                } else {
+                    terminal_settings
+                        .font_size
+                        .map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx))
+                };
 
                 let theme = cx.theme().clone();
 

crates/terminal_view/src/terminal_panel.rs

@@ -437,6 +437,7 @@ impl TerminalPanel {
                 weak_workspace.clone(),
                 database_id,
                 project.downgrade(),
+                false,
                 window,
                 cx,
             )
@@ -674,6 +675,7 @@ impl TerminalPanel {
                         workspace.weak_handle(),
                         workspace.database_id(),
                         workspace.project().downgrade(),
+                        false,
                         window,
                         cx,
                     )
@@ -714,6 +716,7 @@ impl TerminalPanel {
                         workspace.weak_handle(),
                         workspace.database_id(),
                         workspace.project().downgrade(),
+                        false,
                         window,
                         cx,
                     )

crates/terminal_view/src/terminal_view.rs

@@ -111,6 +111,7 @@ pub struct TerminalView {
     context_menu: Option<(Entity<ContextMenu>, gpui::Point<Pixels>, Subscription)>,
     cursor_shape: CursorShape,
     blink_state: bool,
+    embedded: bool,
     blinking_terminal_enabled: bool,
     cwd_serialized: bool,
     blinking_paused: bool,
@@ -162,6 +163,7 @@ impl TerminalView {
         workspace: WeakEntity<Workspace>,
         workspace_id: Option<WorkspaceId>,
         project: WeakEntity<Project>,
+        embedded: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Self {
@@ -200,6 +202,7 @@ impl TerminalView {
             blink_epoch: 0,
             hover_target_tooltip: None,
             hover_tooltip_update: Task::ready(()),
+            embedded,
             workspace_id,
             show_breadcrumbs: TerminalSettings::get_global(cx).toolbar.breadcrumbs,
             block_below_cursor: None,
@@ -381,7 +384,6 @@ impl TerminalView {
                 return;
             }
         }
-
         self.terminal.update(cx, |term, _| term.scroll_wheel(event));
     }
 
@@ -1377,6 +1379,7 @@ impl Render for TerminalView {
                         focused,
                         self.should_show_cursor(focused, cx),
                         self.block_below_cursor.clone(),
+                        self.embedded,
                     ))
                     .when_some(self.render_scrollbar(cx), |div, scrollbar| {
                         div.child(scrollbar)
@@ -1502,6 +1505,7 @@ impl Item for TerminalView {
                 self.workspace.clone(),
                 workspace_id,
                 self.project.clone(),
+                false,
                 window,
                 cx,
             )
@@ -1659,6 +1663,7 @@ impl SerializableItem for TerminalView {
                         workspace,
                         Some(workspace_id),
                         project.downgrade(),
+                        false,
                         window,
                         cx,
                     )