Polish diff for the edit_file tool (#29911)

I added some padding to the editor, and removed the border around each
hunk as it would overlap in weird ways with the card container.

## Before

<img width="1148" alt="image"
src="https://github.com/user-attachments/assets/2018feaa-c847-4609-bc82-522660714b9a"
/>

## After

One Light:

<img width="1148" alt="image"
src="https://github.com/user-attachments/assets/4da1a4b6-0af2-4479-afcc-02da50178fd6"
/>

One Dark:

<img width="1148" alt="image"
src="https://github.com/user-attachments/assets/0168631d-7b76-4582-8174-c6e9c1297dc8"
/>


Release Notes:

- Improved displaying of diffs when the agent edits files.
This commit is contained in:
Antonio Scandurra 2025-05-05 13:17:15 +02:00 committed by GitHub
parent 0048e67832
commit 1adb4ecc95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 61 additions and 23 deletions

View file

@ -50,13 +50,13 @@ strsim.workspace = true
task.workspace = true task.workspace = true
terminal.workspace = true terminal.workspace = true
terminal_view.workspace = true terminal_view.workspace = true
theme.workspace = true
ui.workspace = true ui.workspace = true
util.workspace = true util.workspace = true
web_search.workspace = true web_search.workspace = true
workspace-hack.workspace = true workspace-hack.workspace = true
workspace.workspace = true workspace.workspace = true
zed_llm_client.workspace = true zed_llm_client.workspace = true
theme.workspace = true
[dev-dependencies] [dev-dependencies]
client = { workspace = true, features = ["test-support"] } client = { workspace = true, features = ["test-support"] }

View file

@ -5,10 +5,10 @@ use crate::{
use anyhow::{Context as _, Result, anyhow}; use anyhow::{Context as _, Result, anyhow};
use assistant_tool::{ActionLog, AnyToolCard, Tool, ToolCard, ToolResult, ToolUseStatus}; use assistant_tool::{ActionLog, AnyToolCard, Tool, ToolCard, ToolResult, ToolUseStatus};
use buffer_diff::{BufferDiff, BufferDiffSnapshot}; use buffer_diff::{BufferDiff, BufferDiffSnapshot};
use editor::{Editor, EditorMode, MultiBuffer, PathKey}; use editor::{Editor, EditorElement, EditorMode, EditorStyle, MultiBuffer, PathKey};
use gpui::{ use gpui::{
Animation, AnimationExt, AnyWindowHandle, App, AppContext, AsyncApp, Context, Entity, EntityId, Animation, AnimationExt, AnyWindowHandle, App, AppContext, AsyncApp, Context, Entity, EntityId,
Task, TextStyleRefinement, WeakEntity, pulsating_between, Task, TextStyle, TextStyleRefinement, WeakEntity, pulsating_between,
}; };
use language::{ use language::{
Anchor, Buffer, Capability, LanguageRegistry, LineEnding, OffsetRangeExt, Rope, TextBuffer, Anchor, Buffer, Capability, LanguageRegistry, LineEnding, OffsetRangeExt, Rope, TextBuffer,
@ -382,14 +382,13 @@ impl EditFileToolCard {
.map(|diff_hunk| diff_hunk.buffer_range.to_point(&snapshot)) .map(|diff_hunk| diff_hunk.buffer_range.to_point(&snapshot))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
multibuffer.clear(cx); multibuffer.clear(cx);
let (_, is_newly_added) = multibuffer.set_excerpts_for_path( multibuffer.set_excerpts_for_path(
PathKey::for_buffer(&buffer, cx), PathKey::for_buffer(&buffer, cx),
buffer, buffer,
diff_hunk_ranges, diff_hunk_ranges,
editor::DEFAULT_MULTIBUFFER_CONTEXT, editor::DEFAULT_MULTIBUFFER_CONTEXT,
cx, cx,
); );
debug_assert!(is_newly_added);
multibuffer.add_diff(buffer_diff, cx); multibuffer.add_diff(buffer_diff, cx);
let end = multibuffer.len(cx); let end = multibuffer.len(cx);
Some(multibuffer.snapshot(cx).offset_to_point(end).row + 1) Some(multibuffer.snapshot(cx).offset_to_point(end).row + 1)
@ -554,7 +553,30 @@ impl ToolCard for EditFileToolCard {
.map(|style| style.text.line_height_in_pixels(window.rem_size())) .map(|style| style.text.line_height_in_pixels(window.rem_size()))
.unwrap_or_default(); .unwrap_or_default();
let element = editor.render(window, cx); let settings = ThemeSettings::get_global(cx);
let element = EditorElement::new(
&cx.entity(),
EditorStyle {
background: cx.theme().colors().editor_background,
horizontal_padding: rems(0.25).to_pixels(window.rem_size()),
local_player: cx.theme().players().local(),
text: TextStyle {
color: cx.theme().colors().editor_foreground,
font_family: settings.buffer_font.family.clone(),
font_features: settings.buffer_font.features.clone(),
font_fallbacks: settings.buffer_font.fallbacks.clone(),
font_size: settings.buffer_font_size(cx).into(),
font_weight: settings.buffer_font.weight,
line_height: relative(settings.buffer_line_height.value()),
..Default::default()
},
scrollbar_width: EditorElement::SCROLLBAR_WIDTH,
syntax: cx.theme().syntax().clone(),
status: cx.theme().status().clone(),
..Default::default()
},
);
(element.into_any_element(), line_height) (element.into_any_element(), line_height)
}); });
@ -775,9 +797,16 @@ async fn build_buffer_diff(
})? })?
.await; .await;
let secondary_diff = cx.new(|cx| {
let mut diff = BufferDiff::new(&buffer, cx);
diff.set_snapshot(diff_snapshot.clone(), &buffer, cx);
diff
})?;
cx.new(|cx| { cx.new(|cx| {
let mut diff = BufferDiff::new(&buffer.text, cx); let mut diff = BufferDiff::new(&buffer.text, cx);
diff.set_snapshot(diff_snapshot, &buffer.text, cx); diff.set_snapshot(diff_snapshot, &buffer, cx);
diff.set_secondary_diff(secondary_diff);
diff diff
}) })
} }

View file

@ -517,6 +517,7 @@ pub enum SoftWrap {
#[derive(Clone)] #[derive(Clone)]
pub struct EditorStyle { pub struct EditorStyle {
pub background: Hsla, pub background: Hsla,
pub horizontal_padding: Pixels,
pub local_player: PlayerColor, pub local_player: PlayerColor,
pub text: TextStyle, pub text: TextStyle,
pub scrollbar_width: Pixels, pub scrollbar_width: Pixels,
@ -531,6 +532,7 @@ impl Default for EditorStyle {
fn default() -> Self { fn default() -> Self {
Self { Self {
background: Hsla::default(), background: Hsla::default(),
horizontal_padding: Pixels::default(),
local_player: PlayerColor::default(), local_player: PlayerColor::default(),
text: TextStyle::default(), text: TextStyle::default(),
scrollbar_width: Pixels::default(), scrollbar_width: Pixels::default(),
@ -20352,6 +20354,7 @@ impl Render for Editor {
&cx.entity(), &cx.entity(),
EditorStyle { EditorStyle {
background, background,
horizontal_padding: Pixels::default(),
local_player: cx.theme().players().local(), local_player: cx.theme().players().local(),
text: text_style, text: text_style,
scrollbar_width: EditorElement::SCROLLBAR_WIDTH, scrollbar_width: EditorElement::SCROLLBAR_WIDTH,

View file

@ -170,7 +170,7 @@ pub struct EditorElement {
type DisplayRowDelta = u32; type DisplayRowDelta = u32;
impl EditorElement { impl EditorElement {
pub(crate) const SCROLLBAR_WIDTH: Pixels = px(15.); pub const SCROLLBAR_WIDTH: Pixels = px(15.);
pub fn new(editor: &Entity<Editor>, style: EditorStyle) -> Self { pub fn new(editor: &Entity<Editor>, style: EditorStyle) -> Self {
Self { Self {
@ -6810,10 +6810,27 @@ impl Element for EditorElement {
cx, cx,
) )
.unwrap_or_default(); .unwrap_or_default();
let text_width = bounds.size.width - gutter_dimensions.width; let hitbox = window.insert_hitbox(bounds, false);
let gutter_hitbox =
window.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false);
let text_hitbox = window.insert_hitbox(
Bounds {
origin: gutter_hitbox.top_right()
+ point(style.horizontal_padding, Pixels::default()),
size: size(
bounds.size.width
- gutter_dimensions.width
- 2. * style.horizontal_padding,
bounds.size.height,
),
},
false,
);
let editor_width = let editor_width = text_hitbox.size.width
text_width - gutter_dimensions.margin - em_width - style.scrollbar_width; - gutter_dimensions.margin
- em_width
- style.scrollbar_width;
snapshot = self.editor.update(cx, |editor, cx| { snapshot = self.editor.update(cx, |editor, cx| {
editor.last_bounds = Some(bounds); editor.last_bounds = Some(bounds);
@ -6849,24 +6866,13 @@ impl Element for EditorElement {
.map(|(guide, active)| (self.column_pixels(*guide, window, cx), *active)) .map(|(guide, active)| (self.column_pixels(*guide, window, cx), *active))
.collect::<SmallVec<[_; 2]>>(); .collect::<SmallVec<[_; 2]>>();
let hitbox = window.insert_hitbox(bounds, false);
let gutter_hitbox =
window.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false);
let text_hitbox = window.insert_hitbox(
Bounds {
origin: gutter_hitbox.top_right(),
size: size(text_width, bounds.size.height),
},
false,
);
// Offset the content_bounds from the text_bounds by the gutter margin (which // Offset the content_bounds from the text_bounds by the gutter margin (which
// is roughly half a character wide) to make hit testing work more like how we want. // is roughly half a character wide) to make hit testing work more like how we want.
let content_offset = point(gutter_dimensions.margin, Pixels::ZERO); let content_offset = point(gutter_dimensions.margin, Pixels::ZERO);
let content_origin = text_hitbox.origin + content_offset; let content_origin = text_hitbox.origin + content_offset;
let editor_text_bounds = let editor_text_bounds =
Bounds::from_corners(content_origin, bounds.bottom_right()); Bounds::from_corners(content_origin, text_hitbox.bounds.bottom_right());
let height_in_lines = editor_text_bounds.size.height / line_height; let height_in_lines = editor_text_bounds.size.height / line_height;