From 128b7d224533fb48c1d8de6c2d027ac04c35f147 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Wed, 30 Apr 2025 11:21:29 -0300 Subject: [PATCH] agent: Improve edit file tool card (#29448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⚠️ Work in progress until all of the to-dos are knocked out: - [x] Disable soft-wrapping - [x] Make it foldable only after a certain number of lines - [x] Display tool status errors - [x] Fix horizontal scroll now that we've disabled soft-wrap - [ ] Don't render unnecessary extra lines (will be added later, on a follow-up PR) Release Notes: - N/A --------- Co-authored-by: Agus Zubiaga Co-authored-by: Michael Sloan --- crates/assistant_tools/src/edit_file_tool.rs | 148 ++++++++++++++----- crates/editor/src/editor.rs | 7 - crates/editor/src/element.rs | 4 +- crates/editor/src/scroll.rs | 37 +++-- 4 files changed, 134 insertions(+), 62 deletions(-) diff --git a/crates/assistant_tools/src/edit_file_tool.rs b/crates/assistant_tools/src/edit_file_tool.rs index fc88db2351..a11398a167 100644 --- a/crates/assistant_tools/src/edit_file_tool.rs +++ b/crates/assistant_tools/src/edit_file_tool.rs @@ -11,6 +11,7 @@ use gpui::{ }; use language::{ Anchor, Buffer, Capability, LanguageRegistry, LineEnding, OffsetRangeExt, Rope, TextBuffer, + language_settings::SoftWrap, }; use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}; use project::Project; @@ -274,7 +275,9 @@ pub struct EditFileToolCard { project: Entity, diff_task: Option>>, preview_expanded: bool, + error_expanded: bool, full_height_expanded: bool, + total_lines: Option, editor_unique_id: EntityId, } @@ -293,11 +296,13 @@ impl EditFileToolCard { window, cx, ); - editor.set_show_scrollbars(false, cx); editor.set_show_gutter(false, cx); editor.disable_inline_diagnostics(); - editor.disable_scrolling(cx); editor.disable_expand_excerpt_buttons(cx); + editor.set_soft_wrap_mode(SoftWrap::None, cx); + editor.scroll_manager.set_forbid_vertical_scroll(true); + editor.set_show_scrollbars(false, cx); + editor.set_read_only(true); editor.set_show_breakpoints(false, cx); editor.set_show_code_actions(false, cx); editor.set_show_git_diff_gutter(false, cx); @@ -312,7 +317,9 @@ impl EditFileToolCard { multibuffer, diff_task: None, preview_expanded: true, + error_expanded: false, full_height_expanded: false, + total_lines: None, } } @@ -329,7 +336,7 @@ impl EditFileToolCard { let buffer_diff = build_buffer_diff(old_text, &buffer, &language_registry, cx).await?; this.update(cx, |this, cx| { - this.multibuffer.update(cx, |multibuffer, cx| { + this.total_lines = this.multibuffer.update(cx, |multibuffer, cx| { let snapshot = buffer.read(cx).snapshot(); let diff = buffer_diff.read(cx); let diff_hunk_ranges = diff @@ -345,7 +352,10 @@ impl EditFileToolCard { ); debug_assert!(is_newly_added); multibuffer.add_diff(buffer_diff, cx); + let end = multibuffer.len(cx); + Some(multibuffer.snapshot(cx).offset_to_point(end).row + 1) }); + cx.notify(); }) })); @@ -360,7 +370,10 @@ impl ToolCard for EditFileToolCard { workspace: WeakEntity, cx: &mut Context, ) -> impl IntoElement { - let failed = matches!(status, ToolUseStatus::Error(_)); + let (failed, error_message) = match status { + ToolUseStatus::Error(err) => (true, Some(err.to_string())), + _ => (false, None), + }; let path_label_button = h_flex() .id(("edit-tool-path-label-button", self.editor_unique_id)) @@ -452,9 +465,26 @@ impl ToolCard for EditFileToolCard { .map(|container| { if failed { container.child( - Icon::new(IconName::Close) - .size(IconSize::Small) - .color(Color::Error), + h_flex() + .gap_1() + .child( + Icon::new(IconName::Close) + .size(IconSize::Small) + .color(Color::Error), + ) + .child( + Disclosure::new( + ("edit-file-error-disclosure", self.editor_unique_id), + self.error_expanded, + ) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronDown) + .on_click(cx.listener( + move |this, _event, _window, _cx| { + this.error_expanded = !this.error_expanded; + }, + )), + ), ) } else { container.child( @@ -473,8 +503,14 @@ impl ToolCard for EditFileToolCard { } }); - let editor = self.editor.update(cx, |editor, cx| { - editor.render(window, cx).into_any_element() + let (editor, editor_line_height) = self.editor.update(cx, |editor, cx| { + let line_height = editor + .style() + .map(|style| style.text.line_height_in_pixels(window.rem_size())) + .unwrap_or_default(); + + let element = editor.render(window, cx); + (element.into_any_element(), line_height) }); let (full_height_icon, full_height_tooltip_label) = if self.full_height_expanded { @@ -498,6 +534,9 @@ impl ToolCard for EditFileToolCard { 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; + v_flex() .mb_2() .border_1() @@ -506,50 +545,79 @@ impl ToolCard for EditFileToolCard { .rounded_lg() .overflow_hidden() .child(codeblock_header) - .when(!failed && self.preview_expanded, |card| { + .when(failed && self.error_expanded, |card| { card.child( v_flex() - .relative() - .overflow_hidden() + .p_2() + .gap_1() .border_t_1() + .border_dashed() .border_color(border_color) .bg(cx.theme().colors().editor_background) - .map(|editor_container| { - if self.full_height_expanded { - editor_container.h_full() - } else { - editor_container.max_h_64() - } - }) - .child(div().pl_1().child(editor)) - .when(!self.full_height_expanded, |editor_container| { - editor_container.child(gradient_overlay) - }), + .rounded_b_md() + .child( + Label::new("Error") + .size(LabelSize::XSmall) + .color(Color::Error), + ) + .child( + div() + .rounded_md() + .text_ui_sm(cx) + .bg(cx.theme().colors().editor_background) + .children( + error_message + .map(|error| div().child(error).into_any_element()), + ), + ), ) }) .when(!failed && self.preview_expanded, |card| { card.child( - h_flex() - .id(("edit-tool-card-inner-hflex", self.editor_unique_id)) - .flex_none() - .cursor_pointer() - .h_5() - .justify_center() - .rounded_b_md() + v_flex() + .relative() + .map(|editor_container| { + if self.full_height_expanded { + editor_container.h_full() + } else { + editor_container + .h(DEFAULT_COLLAPSED_LINES as f32 * editor_line_height) + } + }) + .overflow_hidden() .border_t_1() .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; - })), + .child(div().pl_1().child(editor)) + .when( + !self.full_height_expanded && is_collapsible, + |editor_container| editor_container.child(gradient_overlay), + ), ) + .when(is_collapsible, |editor_container| { + editor_container.child( + h_flex() + .id(("expand-button", self.editor_unique_id)) + .flex_none() + .cursor_pointer() + .h_5() + .justify_center() + .rounded_b_md() + .border_t_1() + .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/editor/src/editor.rs b/crates/editor/src/editor.rs index 9ef27f10fd..d3e89df3fc 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -871,7 +871,6 @@ pub struct Editor { show_breadcrumbs: bool, show_gutter: bool, show_scrollbars: bool, - disable_scrolling: bool, disable_expand_excerpt_buttons: bool, show_line_numbers: Option, use_relative_line_numbers: Option, @@ -1668,7 +1667,6 @@ impl Editor { blink_manager: blink_manager.clone(), show_local_selections: true, show_scrollbars: true, - disable_scrolling: false, mode, show_breadcrumbs: EditorSettings::get_global(cx).toolbar.breadcrumbs, show_gutter: mode.is_full(), @@ -16487,11 +16485,6 @@ impl Editor { cx.notify(); } - pub fn disable_scrolling(&mut self, cx: &mut Context) { - self.disable_scrolling = true; - cx.notify(); - } - pub fn set_show_line_numbers(&mut self, show_line_numbers: bool, cx: &mut Context) { self.show_line_numbers = Some(show_line_numbers); cx.notify(); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 78b5adff5f..c1a71d654b 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -5678,9 +5678,7 @@ impl EditorElement { } fn paint_mouse_listeners(&mut self, layout: &EditorLayout, window: &mut Window, cx: &mut App) { - if !self.editor.read(cx).disable_scrolling { - self.paint_scroll_wheel_listener(layout, window, cx); - } + self.paint_scroll_wheel_listener(layout, window, cx); window.on_mouse_event({ let position_map = layout.position_map.clone(); diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index 625fdaa6f2..8d4d8cce46 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -184,9 +184,6 @@ impl ScrollManager { window: &mut Window, cx: &mut Context, ) { - if self.forbid_vertical_scroll { - return; - } let (new_anchor, top_row) = if scroll_position.y <= 0. { ( ScrollAnchor { @@ -258,10 +255,16 @@ impl ScrollManager { window: &mut Window, cx: &mut Context, ) { - if self.forbid_vertical_scroll { - return; - } - self.anchor = anchor; + let adjusted_anchor = if self.forbid_vertical_scroll { + ScrollAnchor { + offset: gpui::Point::new(anchor.offset.x, self.anchor.offset.y), + anchor: self.anchor.anchor, + } + } else { + anchor + }; + + self.anchor = adjusted_anchor; cx.emit(EditorEvent::ScrollPositionChanged { local, autoscroll }); self.show_scrollbars(window, cx); self.autoscroll_request.take(); @@ -404,11 +407,12 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { + let mut delta = scroll_delta; if self.scroll_manager.forbid_vertical_scroll { - return; + delta.y = 0.0; } let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); - let position = self.scroll_manager.anchor.scroll_position(&display_map) + scroll_delta; + let position = self.scroll_manager.anchor.scroll_position(&display_map) + delta; self.set_scroll_position_taking_display_map(position, true, false, display_map, window, cx); } @@ -418,10 +422,12 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { + let mut position = scroll_position; if self.scroll_manager.forbid_vertical_scroll { - return; + let current_position = self.scroll_position(cx); + position.y = current_position.y; } - self.set_scroll_position_internal(scroll_position, true, false, window, cx); + self.set_scroll_position_internal(position, true, false, window, cx); } /// Scrolls so that `row` is at the top of the editor view. @@ -480,8 +486,15 @@ impl Editor { self.edit_prediction_preview .set_previous_scroll_position(None); + let adjusted_position = if self.scroll_manager.forbid_vertical_scroll { + let current_position = self.scroll_manager.anchor.scroll_position(&display_map); + gpui::Point::new(scroll_position.x, current_position.y) + } else { + scroll_position + }; + self.scroll_manager.set_scroll_position( - scroll_position, + adjusted_position, &display_map, local, autoscroll,