From b7abc9d4932e0021910cfb79cd80f07a9e0f1e8b Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Tue, 3 Jun 2025 10:54:25 -0700 Subject: [PATCH] agent: Display full terminal output without scrolling (#31922) The terminal tool card used a fixed height and scrolling, but this meant that it was too tall for commands that only outputted a few lines, and the nested scrolling was undesirable. This PR makes the card be as too as needed to fit the entire output (no scrolling), and allows the user to collapse it to fewer lines when applicable. Making it work the same way as the edit tool card. In fact, both tools now use a shared UI component. https://github.com/user-attachments/assets/1127e21d-1d41-4a4b-a99f-7cd70fccbb56 Release Notes: - Agent: Display full terminal output - Agent: Allow collapsing terminal output --- crates/assistant_tools/src/edit_file_tool.rs | 91 +++++--------- crates/assistant_tools/src/terminal_tool.rs | 77 ++++++++---- crates/assistant_tools/src/ui.rs | 2 + .../src/ui/tool_output_preview.rs | 115 ++++++++++++++++++ crates/debugger_ui/src/session/running.rs | 11 +- crates/terminal_view/src/persistence.rs | 1 - crates/terminal_view/src/terminal_element.rs | 85 +++++++------ crates/terminal_view/src/terminal_panel.rs | 3 - crates/terminal_view/src/terminal_view.rs | 34 +++++- 9 files changed, 275 insertions(+), 144 deletions(-) create mode 100644 crates/assistant_tools/src/ui/tool_output_preview.rs diff --git a/crates/assistant_tools/src/edit_file_tool.rs b/crates/assistant_tools/src/edit_file_tool.rs index 150fdc5eca00654070aa14c747ffcda93c30098c..c4768934db21aaf4b257efd17a152778062203d4 100644 --- a/crates/assistant_tools/src/edit_file_tool.rs +++ b/crates/assistant_tools/src/edit_file_tool.rs @@ -2,6 +2,7 @@ use crate::{ Templates, edit_agent::{EditAgent, EditAgentOutput, EditAgentOutputEvent}, schema::json_schema_for, + ui::{COLLAPSED_LINES, ToolOutputPreview}, }; use anyhow::{Context as _, Result, anyhow}; use assistant_tool::{ @@ -13,7 +14,7 @@ use editor::{Editor, EditorMode, MinimapVisibility, MultiBuffer, PathKey}; use futures::StreamExt; use gpui::{ Animation, AnimationExt, AnyWindowHandle, App, AppContext, AsyncApp, Entity, Task, - TextStyleRefinement, WeakEntity, pulsating_between, + TextStyleRefinement, WeakEntity, pulsating_between, px, }; use indoc::formatdoc; use language::{ @@ -884,30 +885,8 @@ impl ToolCard for EditFileToolCard { (element.into_any_element(), line_height) }); - let (full_height_icon, full_height_tooltip_label) = if self.full_height_expanded { - (IconName::ChevronUp, "Collapse Code Block") - } else { - (IconName::ChevronDown, "Expand Code Block") - }; - - let gradient_overlay = - div() - .absolute() - .bottom_0() - .left_0() - .w_full() - .h_2_5() - .bg(gpui::linear_gradient( - 0., - gpui::linear_color_stop(cx.theme().colors().editor_background, 0.), - gpui::linear_color_stop(cx.theme().colors().editor_background.opacity(0.), 1.), - )); - let border_color = cx.theme().colors().border.opacity(0.6); - const DEFAULT_COLLAPSED_LINES: u32 = 10; - let is_collapsible = self.total_lines.unwrap_or(0) > DEFAULT_COLLAPSED_LINES; - let waiting_for_diff = { let styles = [ ("w_4_5", (0.1, 0.85), 2000), @@ -992,48 +971,34 @@ impl ToolCard for EditFileToolCard { card.child(waiting_for_diff) }) .when(self.preview_expanded && !self.is_loading(), |card| { + let editor_view = v_flex() + .relative() + .h_full() + .when(!self.full_height_expanded, |editor_container| { + editor_container.max_h(px(COLLAPSED_LINES as f32 * editor_line_height.0)) + }) + .overflow_hidden() + .border_t_1() + .border_color(border_color) + .bg(cx.theme().colors().editor_background) + .child(editor); + card.child( - v_flex() - .relative() - .h_full() - .when(!self.full_height_expanded, |editor_container| { - editor_container - .max_h(DEFAULT_COLLAPSED_LINES as f32 * editor_line_height) - }) - .overflow_hidden() - .border_t_1() - .border_color(border_color) - .bg(cx.theme().colors().editor_background) - .child(editor) - .when( - !self.full_height_expanded && is_collapsible, - |editor_container| editor_container.child(gradient_overlay), - ), + ToolOutputPreview::new(editor_view.into_any_element(), self.editor.entity_id()) + .with_total_lines(self.total_lines.unwrap_or(0) as usize) + .toggle_state(self.full_height_expanded) + .with_collapsed_fade() + .on_toggle({ + let this = cx.entity().downgrade(); + move |is_expanded, _window, cx| { + if let Some(this) = this.upgrade() { + this.update(cx, |this, _cx| { + this.full_height_expanded = is_expanded; + }); + } + } + }), ) - .when(is_collapsible, |card| { - card.child( - h_flex() - .id(("expand-button", self.editor.entity_id())) - .flex_none() - .cursor_pointer() - .h_5() - .justify_center() - .border_t_1() - .rounded_b_md() - .border_color(border_color) - .bg(cx.theme().colors().editor_background) - .hover(|style| style.bg(cx.theme().colors().element_hover.opacity(0.1))) - .child( - Icon::new(full_height_icon) - .size(IconSize::Small) - .color(Color::Muted), - ) - .tooltip(Tooltip::text(full_height_tooltip_label)) - .on_click(cx.listener(move |this, _event, _window, _cx| { - this.full_height_expanded = !this.full_height_expanded; - })), - ) - }) }) } } diff --git a/crates/assistant_tools/src/terminal_tool.rs b/crates/assistant_tools/src/terminal_tool.rs index 2a8eff8c60e1c5c18cd87996c7fa786a32e35206..91a2d994eda499f7e5970201acf583eb711792d3 100644 --- a/crates/assistant_tools/src/terminal_tool.rs +++ b/crates/assistant_tools/src/terminal_tool.rs @@ -1,4 +1,7 @@ -use crate::schema::json_schema_for; +use crate::{ + schema::json_schema_for, + ui::{COLLAPSED_LINES, ToolOutputPreview}, +}; use anyhow::{Context as _, Result, anyhow}; use assistant_tool::{ActionLog, Tool, ToolCard, ToolResult, ToolUseStatus}; use futures::{FutureExt as _, future::Shared}; @@ -25,7 +28,7 @@ use terminal_view::TerminalView; use theme::ThemeSettings; use ui::{Disclosure, Tooltip, prelude::*}; use util::{ - get_system_shell, markdown::MarkdownInlineCode, size::format_file_size, + ResultExt, get_system_shell, markdown::MarkdownInlineCode, size::format_file_size, time::duration_alt_display, }; use workspace::Workspace; @@ -254,22 +257,24 @@ impl Tool for TerminalTool { let terminal_view = window.update(cx, |_, window, cx| { cx.new(|cx| { - TerminalView::new( + let mut view = TerminalView::new( terminal.clone(), workspace.downgrade(), None, project.downgrade(), - true, window, cx, - ) + ); + view.set_embedded_mode(None, cx); + view }) })?; - let _ = card.update(cx, |card, _| { + card.update(cx, |card, _| { card.terminal = Some(terminal_view.clone()); card.start_instant = Instant::now(); - }); + }) + .log_err(); let exit_status = terminal .update(cx, |terminal, cx| terminal.wait_for_completed_task(cx))? @@ -285,7 +290,7 @@ impl Tool for TerminalTool { exit_status.map(portable_pty::ExitStatus::from), ); - let _ = card.update(cx, |card, _| { + card.update(cx, |card, _| { card.command_finished = true; card.exit_status = exit_status; card.was_content_truncated = processed_content.len() < previous_len; @@ -293,7 +298,8 @@ impl Tool for TerminalTool { card.content_line_count = content_line_count; card.finished_with_empty_output = finished_with_empty_output; card.elapsed_time = Some(card.start_instant.elapsed()); - }); + }) + .log_err(); Ok(processed_content.into()) } @@ -473,7 +479,6 @@ impl ToolCard for TerminalToolCard { let time_elapsed = self .elapsed_time .unwrap_or_else(|| self.start_instant.elapsed()); - let should_hide_terminal = tool_failed || self.finished_with_empty_output; let header_bg = cx .theme() @@ -574,7 +579,7 @@ impl ToolCard for TerminalToolCard { ), ) }) - .when(!should_hide_terminal, |header| { + .when(!self.finished_with_empty_output, |header| { header.child( Disclosure::new( ("terminal-tool-disclosure", self.entity_id), @@ -618,19 +623,43 @@ impl ToolCard for TerminalToolCard { ), ), ) - .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()), - ) - }) + .when( + self.preview_expanded && !self.finished_with_empty_output, + |this| { + this.child( + div() + .pt_2() + .border_t_1() + .border_color(border_color) + .bg(cx.theme().colors().editor_background) + .rounded_b_md() + .text_ui_sm(cx) + .child( + ToolOutputPreview::new( + terminal.clone().into_any_element(), + terminal.entity_id(), + ) + .with_total_lines(self.content_line_count) + .toggle_state(!terminal.read(cx).is_content_limited(window)) + .on_toggle({ + let terminal = terminal.clone(); + move |is_expanded, _, cx| { + terminal.update(cx, |terminal, cx| { + terminal.set_embedded_mode( + if is_expanded { + None + } else { + Some(COLLAPSED_LINES) + }, + cx, + ); + }); + } + }), + ), + ) + }, + ) .into_any() } } diff --git a/crates/assistant_tools/src/ui.rs b/crates/assistant_tools/src/ui.rs index a8ff923ef5b7c4a206cebe89dae3373947fc79e2..793427385456939eb1a7070fff5bba928a6c2643 100644 --- a/crates/assistant_tools/src/ui.rs +++ b/crates/assistant_tools/src/ui.rs @@ -1,3 +1,5 @@ mod tool_call_card_header; +mod tool_output_preview; pub use tool_call_card_header::*; +pub use tool_output_preview::*; diff --git a/crates/assistant_tools/src/ui/tool_output_preview.rs b/crates/assistant_tools/src/ui/tool_output_preview.rs new file mode 100644 index 0000000000000000000000000000000000000000..a672bb8b99daa1fd776f59c4e8be789b8e25240c --- /dev/null +++ b/crates/assistant_tools/src/ui/tool_output_preview.rs @@ -0,0 +1,115 @@ +use gpui::{AnyElement, EntityId, prelude::*}; +use ui::{Tooltip, prelude::*}; + +#[derive(IntoElement)] +pub struct ToolOutputPreview +where + F: Fn(bool, &mut Window, &mut App) + 'static, +{ + content: AnyElement, + entity_id: EntityId, + full_height: bool, + total_lines: usize, + collapsed_fade: bool, + on_toggle: Option, +} + +pub const COLLAPSED_LINES: usize = 10; + +impl ToolOutputPreview +where + F: Fn(bool, &mut Window, &mut App) + 'static, +{ + pub fn new(content: AnyElement, entity_id: EntityId) -> Self { + Self { + content, + entity_id, + full_height: true, + total_lines: 0, + collapsed_fade: false, + on_toggle: None, + } + } + + pub fn with_total_lines(mut self, total_lines: usize) -> Self { + self.total_lines = total_lines; + self + } + + pub fn toggle_state(mut self, full_height: bool) -> Self { + self.full_height = full_height; + self + } + + pub fn with_collapsed_fade(mut self) -> Self { + self.collapsed_fade = true; + self + } + + pub fn on_toggle(mut self, listener: F) -> Self { + self.on_toggle = Some(listener); + self + } +} + +impl RenderOnce for ToolOutputPreview +where + F: Fn(bool, &mut Window, &mut App) + 'static, +{ + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + if self.total_lines <= COLLAPSED_LINES { + return self.content; + } + let border_color = cx.theme().colors().border.opacity(0.6); + + let (icon, tooltip_label) = if self.full_height { + (IconName::ChevronUp, "Collapse") + } else { + (IconName::ChevronDown, "Expand") + }; + + let gradient_overlay = + if self.collapsed_fade && !self.full_height { + Some(div().absolute().bottom_5().left_0().w_full().h_2_5().bg( + gpui::linear_gradient( + 0., + gpui::linear_color_stop(cx.theme().colors().editor_background, 0.), + gpui::linear_color_stop( + cx.theme().colors().editor_background.opacity(0.), + 1., + ), + ), + )) + } else { + None + }; + + v_flex() + .relative() + .child(self.content) + .children(gradient_overlay) + .child( + h_flex() + .id(("expand-button", self.entity_id)) + .flex_none() + .cursor_pointer() + .h_5() + .justify_center() + .border_t_1() + .rounded_b_md() + .border_color(border_color) + .bg(cx.theme().colors().editor_background) + .hover(|style| style.bg(cx.theme().colors().element_hover.opacity(0.1))) + .child(Icon::new(icon).size(IconSize::Small).color(Color::Muted)) + .tooltip(Tooltip::text(tooltip_label)) + .when_some(self.on_toggle, |this, on_toggle| { + this.on_click({ + move |_, window, cx| { + on_toggle(!self.full_height, window, cx); + } + }) + }), + ) + .into_any() + } +} diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 456f6d3d433268edc02149783641a49807323b27..3151feeba406c3c1e68bc5efa78591447aef052c 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -901,7 +901,6 @@ impl RunningState { weak_workspace, None, weak_project, - false, window, cx, ) @@ -1055,15 +1054,7 @@ impl RunningState { let terminal = terminal_task.await?; let terminal_view = cx.new_window_entity(|window, cx| { - TerminalView::new( - terminal.clone(), - workspace, - None, - weak_project, - false, - window, - cx, - ) + TerminalView::new(terminal.clone(), workspace, None, weak_project, window, cx) })?; running.update_in(cx, |running, window, cx| { diff --git a/crates/terminal_view/src/persistence.rs b/crates/terminal_view/src/persistence.rs index 55259c143fa8b912600c065676b2a17799585d55..056365ab8cd98d020fc154bce97ca6714aa57bf4 100644 --- a/crates/terminal_view/src/persistence.rs +++ b/crates/terminal_view/src/persistence.rs @@ -264,7 +264,6 @@ async fn deserialize_pane_group( workspace.clone(), Some(workspace_id), project.downgrade(), - false, window, cx, ) diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 730e1e743f77b565d8189b49e30882b99781f84b..2ea27fe5bb383cf2fdbbc1ac164e3df1f4639b5d 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/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, Focusable, Font, FontStyle, FontWeight, GlobalElementId, - HighlightStyle, Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement, - LayoutId, ModifiersChangedEvent, MouseButton, MouseMoveEvent, Pixels, Point, ShapedLine, + ElementId, Entity, FocusHandle, Font, FontStyle, FontWeight, GlobalElementId, HighlightStyle, + Hitbox, Hsla, InputHandler, InteractiveElement, Interactivity, IntoElement, LayoutId, + ModifiersChangedEvent, MouseButton, MouseMoveEvent, Pixels, Point, ShapedLine, StatefulInteractiveElement, StrikethroughStyle, Styled, TextRun, TextStyle, UTF16Selection, UnderlineStyle, WeakEntity, WhiteSpace, Window, WindowTextSystem, div, fill, point, px, relative, size, @@ -32,7 +32,7 @@ use workspace::Workspace; use std::mem; use std::{fmt::Debug, ops::RangeInclusive, rc::Rc}; -use crate::{BlockContext, BlockProperties, TerminalView}; +use crate::{BlockContext, BlockProperties, TerminalMode, TerminalView}; /// The information generated during layout that is necessary for painting. pub struct LayoutState { @@ -160,7 +160,7 @@ pub struct TerminalElement { focused: bool, cursor_visible: bool, interactivity: Interactivity, - embedded: bool, + mode: TerminalMode, block_below_cursor: Option>, } @@ -181,7 +181,7 @@ impl TerminalElement { focused: bool, cursor_visible: bool, block_below_cursor: Option>, - embedded: bool, + mode: TerminalMode, ) -> TerminalElement { TerminalElement { terminal, @@ -191,7 +191,7 @@ impl TerminalElement { focus: focus.clone(), cursor_visible, block_below_cursor, - embedded, + mode, interactivity: Default::default(), } .track_focus(&focus) @@ -511,21 +511,20 @@ impl TerminalElement { }, ), ); - self.interactivity.on_scroll_wheel({ - let terminal_view = self.terminal_view.downgrade(); - move |e, window, cx| { - terminal_view - .update(cx, |terminal_view, cx| { - if !terminal_view.embedded - || terminal_view.focus_handle(cx).is_focused(window) - { + + if !matches!(self.mode, TerminalMode::Embedded { .. }) { + self.interactivity.on_scroll_wheel({ + let terminal_view = self.terminal_view.downgrade(); + move |e, _window, cx| { + terminal_view + .update(cx, |terminal_view, cx| { terminal_view.scroll_wheel(e, cx); cx.notify(); - } - }) - .ok(); - } - }); + }) + .ok(); + } + }); + } // Mouse mode handlers: // All mouse modes need the extra click handlers @@ -606,16 +605,6 @@ 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, inspector_id, @@ -623,8 +612,29 @@ impl Element for TerminalElement { cx, |mut style, window, cx| { style.size.width = relative(1.).into(); - style.size.height = relative(1.).into(); - // style.overflow = point(Overflow::Hidden, Overflow::Hidden); + + match &self.mode { + TerminalMode::Scrollable => { + style.size.height = relative(1.).into(); + } + TerminalMode::Embedded { max_lines } => { + let rem_size = window.rem_size(); + let line_height = window.text_style().font_size.to_pixels(rem_size) + * TerminalSettings::get_global(cx) + .line_height + .value() + .to_pixels(rem_size) + .0; + + let mut line_count = self.terminal.read(cx).total_lines(); + if !self.focused { + if let Some(max_lines) = max_lines { + line_count = line_count.min(*max_lines); + } + } + style.size.height = (line_count * line_height).into(); + } + } window.request_layout(style, None, cx) }, @@ -679,12 +689,13 @@ impl Element for TerminalElement { let line_height = terminal_settings.line_height.value(); - let font_size = if self.embedded { - window.text_style().font_size.to_pixels(window.rem_size()) - } else { - terminal_settings + let font_size = match &self.mode { + TerminalMode::Embedded { .. } => { + window.text_style().font_size.to_pixels(window.rem_size()) + } + TerminalMode::Scrollable => terminal_settings .font_size - .map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx)) + .map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx)), }; let theme = cx.theme().clone(); diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index f7b9a31275e4931a241ae0b91f7e3f668256b5a8..355e3328cfb55df332ce924313fe8a92796bd870 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -439,7 +439,6 @@ impl TerminalPanel { weak_workspace.clone(), database_id, project.downgrade(), - false, window, cx, ) @@ -677,7 +676,6 @@ impl TerminalPanel { workspace.weak_handle(), workspace.database_id(), workspace.project().downgrade(), - false, window, cx, ) @@ -718,7 +716,6 @@ impl TerminalPanel { workspace.weak_handle(), workspace.database_id(), workspace.project().downgrade(), - false, window, cx, ) diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index e7b2138677fb266fe4d7e4fc22f04994b11fed97..ebc84aad4281a5c9bafa47f6e2b91c8a26c6f436 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -116,7 +116,7 @@ pub struct TerminalView { context_menu: Option<(Entity, gpui::Point, Subscription)>, cursor_shape: CursorShape, blink_state: bool, - embedded: bool, + mode: TerminalMode, blinking_terminal_enabled: bool, cwd_serialized: bool, blinking_paused: bool, @@ -137,6 +137,15 @@ pub struct TerminalView { _terminal_subscriptions: Vec, } +#[derive(Default, Clone)] +pub enum TerminalMode { + #[default] + Scrollable, + Embedded { + max_lines: Option, + }, +} + #[derive(Debug)] struct HoverTarget { tooltip: String, @@ -176,7 +185,6 @@ impl TerminalView { workspace: WeakEntity, workspace_id: Option, project: WeakEntity, - embedded: bool, window: &mut Window, cx: &mut Context, ) -> Self { @@ -215,7 +223,7 @@ impl TerminalView { blink_epoch: 0, hover: None, hover_tooltip_update: Task::ready(()), - embedded, + mode: TerminalMode::Scrollable, workspace_id, show_breadcrumbs: TerminalSettings::get_global(cx).toolbar.breadcrumbs, block_below_cursor: None, @@ -236,6 +244,21 @@ impl TerminalView { } } + /// Enable 'embedded' mode where the terminal displays the full content with an optional limit of lines. + pub fn set_embedded_mode(&mut self, max_lines: Option, cx: &mut Context) { + self.mode = TerminalMode::Embedded { max_lines }; + cx.notify(); + } + + pub fn is_content_limited(&self, window: &Window) -> bool { + match &self.mode { + TerminalMode::Scrollable => false, + TerminalMode::Embedded { max_lines } => { + !self.focus_handle.is_focused(window) && max_lines.is_some() + } + } + } + /// Sets the marked (pre-edit) text from the IME. pub(crate) fn set_marked_text( &mut self, @@ -820,6 +843,7 @@ impl TerminalView { fn render_scrollbar(&self, cx: &mut Context) -> Option> { if !Self::should_show_scrollbar(cx) || !(self.show_scrollbar || self.scrollbar_state.is_dragging()) + || matches!(self.mode, TerminalMode::Embedded { .. }) { return None; } @@ -1467,7 +1491,7 @@ impl Render for TerminalView { focused, self.should_show_cursor(focused, cx), self.block_below_cursor.clone(), - self.embedded, + self.mode.clone(), )) .when_some(self.render_scrollbar(cx), |div, scrollbar| { div.child(scrollbar) @@ -1593,7 +1617,6 @@ impl Item for TerminalView { self.workspace.clone(), workspace_id, self.project.clone(), - false, window, cx, ) @@ -1751,7 +1774,6 @@ impl SerializableItem for TerminalView { workspace, Some(workspace_id), project.downgrade(), - false, window, cx, )