From 550c3fb506bd6dae6673f278354baa701bdef8ec Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Fri, 2 May 2025 17:57:16 -0300 Subject: [PATCH] agent: Review edits in single-file editors (#29820) Enables reviewing agent edits from single-file editors in addition to the multibuffer experience we already had. https://github.com/user-attachments/assets/a2c287f0-51d6-43a1-8537-821498b91983 This feature can be turned off by setting `assistant.single_file_review: false`. Release Notes: - agent: Review edits in single-file editors --- assets/keymaps/default-linux.json | 10 + assets/keymaps/default-macos.json | 11 + crates/agent/src/active_thread.rs | 4 +- crates/agent/src/agent_diff.rs | 1479 ++++++++++++++--- crates/agent/src/assistant.rs | 2 +- crates/agent/src/assistant_panel.rs | 14 +- crates/agent/src/message_editor.rs | 12 +- crates/agent/src/thread.rs | 20 +- .../src/assistant_settings.rs | 37 +- crates/editor/src/editor.rs | 49 +- crates/eval/src/example.rs | 6 +- crates/multi_buffer/src/multi_buffer.rs | 5 + crates/workspace/src/toolbar.rs | 1 + 13 files changed, 1396 insertions(+), 254 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 3b87f6ee20..eac125e1ef 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -194,6 +194,16 @@ "alt-shift-y": "git::UnstageAndNext" } }, + { + "context": "Editor && editor_agent_diff", + "bindings": { + "ctrl-y": "agent::Keep", + "ctrl-n": "agent::Reject", + "ctrl-shift-y": "agent::KeepAll", + "ctrl-shift-n": "agent::RejectAll", + "shift-ctrl-r": "agent::OpenAgentDiff" + } + }, { "context": "AgentDiff", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 71222ffcfa..7a87b5687b 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -247,6 +247,17 @@ "cmd-shift-n": "agent::RejectAll" } }, + { + "context": "Editor && editor_agent_diff", + "use_key_equivalents": true, + "bindings": { + "cmd-y": "agent::Keep", + "cmd-n": "agent::Reject", + "cmd-shift-y": "agent::KeepAll", + "cmd-shift-n": "agent::RejectAll", + "shift-ctrl-r": "agent::OpenAgentDiff" + } + }, { "context": "AssistantPanel", "use_key_equivalents": true, diff --git a/crates/agent/src/active_thread.rs b/crates/agent/src/active_thread.rs index af62fedc5c..cbec9d44c3 100644 --- a/crates/agent/src/active_thread.rs +++ b/crates/agent/src/active_thread.rs @@ -765,7 +765,6 @@ impl ActiveThread { .unwrap() } }); - let mut this = Self { language_registry, thread_store, @@ -954,6 +953,9 @@ impl ActiveThread { ThreadEvent::UsageUpdated(usage) => { self.last_usage = Some(*usage); } + ThreadEvent::NewRequest | ThreadEvent::CompletionCanceled => { + cx.notify(); + } ThreadEvent::StreamedCompletion | ThreadEvent::SummaryGenerated | ThreadEvent::SummaryChanged => { diff --git a/crates/agent/src/agent_diff.rs b/crates/agent/src/agent_diff.rs index 2d7ab8df4a..c772e6d122 100644 --- a/crates/agent/src/agent_diff.rs +++ b/crates/agent/src/agent_diff.rs @@ -1,25 +1,33 @@ -use crate::{Keep, KeepAll, Reject, RejectAll, Thread, ThreadEvent, ui::AnimatedLabel}; +use crate::{ + Keep, KeepAll, OpenAgentDiff, Reject, RejectAll, Thread, ThreadEvent, ui::AnimatedLabel, +}; use anyhow::Result; +use assistant_settings::AssistantSettings; use buffer_diff::DiffHunkStatus; use collections::{HashMap, HashSet}; use editor::{ - Direction, Editor, EditorEvent, MultiBuffer, ToPoint, + Direction, Editor, EditorEvent, MultiBuffer, MultiBufferSnapshot, ToPoint, actions::{GoToHunk, GoToPreviousHunk}, scroll::Autoscroll, }; use gpui::{ - Action, AnyElement, AnyView, App, Empty, Entity, EventEmitter, FocusHandle, Focusable, - SharedString, Subscription, Task, WeakEntity, Window, prelude::*, + Action, AnyElement, AnyView, App, AppContext, Empty, Entity, EventEmitter, FocusHandle, + Focusable, Global, SharedString, Subscription, Task, WeakEntity, Window, prelude::*, }; -use language::{Capability, DiskState, OffsetRangeExt, Point}; + +use language::{Buffer, Capability, DiskState, OffsetRangeExt, Point}; +use language_model::StopReason; use multi_buffer::PathKey; -use project::{Project, ProjectPath}; +use project::{Project, ProjectItem, ProjectPath}; +use settings::{Settings, SettingsStore}; use std::{ any::{Any, TypeId}, + collections::hash_map::Entry, ops::Range, sync::Arc, }; -use ui::{IconButtonShape, KeyBinding, Tooltip, prelude::*}; +use ui::{IconButtonShape, KeyBinding, Tooltip, prelude::*, vertical_divider}; +use util::ResultExt; use workspace::{ Item, ItemHandle, ItemNavHistory, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace, @@ -28,7 +36,7 @@ use workspace::{ }; use zed_actions::assistant::ToggleFocus; -pub struct AgentDiff { +pub struct AgentDiffPane { multibuffer: Entity, editor: Entity, thread: Entity, @@ -38,7 +46,7 @@ pub struct AgentDiff { _subscriptions: Vec, } -impl AgentDiff { +impl AgentDiffPane { pub fn deploy( thread: Entity, workspace: WeakEntity, @@ -57,14 +65,14 @@ impl AgentDiff { cx: &mut Context, ) -> Entity { let existing_diff = workspace - .items_of_type::(cx) + .items_of_type::(cx) .find(|diff| diff.read(cx).thread == thread); if let Some(existing_diff) = existing_diff { workspace.activate_item(&existing_diff, true, true, window, cx); existing_diff } else { - let agent_diff = - cx.new(|cx| AgentDiff::new(thread.clone(), workspace.weak_handle(), window, cx)); + let agent_diff = cx + .new(|cx| AgentDiffPane::new(thread.clone(), workspace.weak_handle(), window, cx)); workspace.add_item_to_center(Box::new(agent_diff.clone()), window, cx); agent_diff } @@ -80,35 +88,12 @@ impl AgentDiff { let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); let project = thread.read(cx).project().clone(); - let render_diff_hunk_controls = Arc::new({ - let agent_diff = cx.entity(); - move |row, - status: &DiffHunkStatus, - hunk_range, - is_created_file, - line_height, - editor: &Entity, - window: &mut Window, - cx: &mut App| { - render_diff_hunk_controls( - row, - status, - hunk_range, - is_created_file, - line_height, - &agent_diff, - editor, - window, - cx, - ) - } - }); let editor = cx.new(|cx| { let mut editor = Editor::for_multibuffer(multibuffer.clone(), Some(project.clone()), window, cx); editor.disable_inline_diagnostics(); editor.set_expand_all_diff_hunks(cx); - editor.set_render_diff_hunk_controls(render_diff_hunk_controls, cx); + editor.set_render_diff_hunk_controls(diff_hunk_controls(&thread), cx); editor.register_addon(AgentDiffAddon); editor }); @@ -248,7 +233,7 @@ impl AgentDiff { } } - pub fn move_to_path(&mut self, path_key: PathKey, window: &mut Window, cx: &mut Context) { + pub fn move_to_path(&self, path_key: PathKey, window: &mut Window, cx: &mut App) { if let Some(position) = self.multibuffer.read(cx).location_for_path(&path_key, cx) { self.editor.update(cx, |editor, cx| { let first_hunk = editor @@ -268,164 +253,190 @@ impl AgentDiff { } } - fn keep(&mut self, _: &crate::Keep, window: &mut Window, cx: &mut Context) { - let ranges = self - .editor - .read(cx) - .selections - .disjoint_anchor_ranges() - .collect::>(); - self.keep_edits_in_ranges(ranges, window, cx); + fn keep(&mut self, _: &Keep, window: &mut Window, cx: &mut Context) { + self.editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + keep_edits_in_selection(editor, &snapshot, &self.thread, window, cx); + }); } - fn reject(&mut self, _: &crate::Reject, window: &mut Window, cx: &mut Context) { - let ranges = self - .editor - .read(cx) - .selections - .disjoint_anchor_ranges() - .collect::>(); - self.reject_edits_in_ranges(ranges, window, cx); + fn reject(&mut self, _: &Reject, window: &mut Window, cx: &mut Context) { + self.editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + reject_edits_in_selection(editor, &snapshot, &self.thread, window, cx); + }); } - fn reject_all(&mut self, _: &crate::RejectAll, window: &mut Window, cx: &mut Context) { - self.reject_edits_in_ranges( - vec![editor::Anchor::min()..editor::Anchor::max()], - window, - cx, - ); + fn reject_all(&mut self, _: &RejectAll, window: &mut Window, cx: &mut Context) { + self.editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + reject_edits_in_ranges( + editor, + &snapshot, + &self.thread, + vec![editor::Anchor::min()..editor::Anchor::max()], + window, + cx, + ); + }); } - fn keep_all(&mut self, _: &crate::KeepAll, _window: &mut Window, cx: &mut Context) { + fn keep_all(&mut self, _: &KeepAll, _window: &mut Window, cx: &mut Context) { self.thread .update(cx, |thread, cx| thread.keep_all_edits(cx)); } +} - fn keep_edits_in_ranges( - &mut self, - ranges: Vec>, - window: &mut Window, - cx: &mut Context, - ) { - if self.thread.read(cx).is_generating() { - return; +fn keep_edits_in_selection( + editor: &mut Editor, + buffer_snapshot: &MultiBufferSnapshot, + thread: &Entity, + window: &mut Window, + cx: &mut Context, +) { + let ranges = editor + .selections + .disjoint_anchor_ranges() + .collect::>(); + + keep_edits_in_ranges(editor, buffer_snapshot, &thread, ranges, window, cx) +} + +fn reject_edits_in_selection( + editor: &mut Editor, + buffer_snapshot: &MultiBufferSnapshot, + thread: &Entity, + window: &mut Window, + cx: &mut Context, +) { + let ranges = editor + .selections + .disjoint_anchor_ranges() + .collect::>(); + reject_edits_in_ranges(editor, buffer_snapshot, &thread, ranges, window, cx) +} + +fn keep_edits_in_ranges( + editor: &mut Editor, + buffer_snapshot: &MultiBufferSnapshot, + thread: &Entity, + ranges: Vec>, + window: &mut Window, + cx: &mut Context, +) { + if thread.read(cx).is_generating() { + return; + } + + let diff_hunks_in_ranges = editor + .diff_hunks_in_ranges(&ranges, buffer_snapshot) + .collect::>(); + + update_editor_selection(editor, buffer_snapshot, &diff_hunks_in_ranges, window, cx); + + let multibuffer = editor.buffer().clone(); + for hunk in &diff_hunks_in_ranges { + let buffer = multibuffer.read(cx).buffer(hunk.buffer_id); + if let Some(buffer) = buffer { + thread.update(cx, |thread, cx| { + thread.keep_edits_in_range(buffer, hunk.buffer_range.clone(), cx) + }); } + } +} - let snapshot = self.multibuffer.read(cx).snapshot(cx); - let diff_hunks_in_ranges = self - .editor - .read(cx) - .diff_hunks_in_ranges(&ranges, &snapshot) - .collect::>(); - let newest_cursor = self.editor.update(cx, |editor, cx| { - editor.selections.newest::(cx).head() - }); - if diff_hunks_in_ranges.iter().any(|hunk| { - hunk.row_range - .contains(&multi_buffer::MultiBufferRow(newest_cursor.row)) - }) { - self.update_selection(&diff_hunks_in_ranges, window, cx); - } +fn reject_edits_in_ranges( + editor: &mut Editor, + buffer_snapshot: &MultiBufferSnapshot, + thread: &Entity, + ranges: Vec>, + window: &mut Window, + cx: &mut Context, +) { + if thread.read(cx).is_generating() { + return; + } - for hunk in &diff_hunks_in_ranges { - let buffer = self.multibuffer.read(cx).buffer(hunk.buffer_id); - if let Some(buffer) = buffer { - self.thread.update(cx, |thread, cx| { - thread.keep_edits_in_range(buffer, hunk.buffer_range.clone(), cx) - }); - } + let diff_hunks_in_ranges = editor + .diff_hunks_in_ranges(&ranges, buffer_snapshot) + .collect::>(); + + update_editor_selection(editor, buffer_snapshot, &diff_hunks_in_ranges, window, cx); + + let multibuffer = editor.buffer().clone(); + + let mut ranges_by_buffer = HashMap::default(); + for hunk in &diff_hunks_in_ranges { + let buffer = multibuffer.read(cx).buffer(hunk.buffer_id); + if let Some(buffer) = buffer { + ranges_by_buffer + .entry(buffer.clone()) + .or_insert_with(Vec::new) + .push(hunk.buffer_range.clone()); } } - fn reject_edits_in_ranges( - &mut self, - ranges: Vec>, - window: &mut Window, - cx: &mut Context, - ) { - if self.thread.read(cx).is_generating() { - return; - } + for (buffer, ranges) in ranges_by_buffer { + thread + .update(cx, |thread, cx| { + thread.reject_edits_in_ranges(buffer, ranges, cx) + }) + .detach_and_log_err(cx); + } +} - let snapshot = self.multibuffer.read(cx).snapshot(cx); - let diff_hunks_in_ranges = self - .editor - .read(cx) - .diff_hunks_in_ranges(&ranges, &snapshot) - .collect::>(); - let newest_cursor = self.editor.update(cx, |editor, cx| { - editor.selections.newest::(cx).head() - }); - if diff_hunks_in_ranges.iter().any(|hunk| { - hunk.row_range - .contains(&multi_buffer::MultiBufferRow(newest_cursor.row)) - }) { - self.update_selection(&diff_hunks_in_ranges, window, cx); - } +fn update_editor_selection( + editor: &mut Editor, + buffer_snapshot: &MultiBufferSnapshot, + diff_hunks: &[multi_buffer::MultiBufferDiffHunk], + window: &mut Window, + cx: &mut Context, +) { + let newest_cursor = editor.selections.newest::(cx).head(); - let mut ranges_by_buffer = HashMap::default(); - for hunk in &diff_hunks_in_ranges { - let buffer = self.multibuffer.read(cx).buffer(hunk.buffer_id); - if let Some(buffer) = buffer { - ranges_by_buffer - .entry(buffer.clone()) - .or_insert_with(Vec::new) - .push(hunk.buffer_range.clone()); - } - } - - for (buffer, ranges) in ranges_by_buffer { - self.thread - .update(cx, |thread, cx| { - thread.reject_edits_in_ranges(buffer, ranges, cx) - }) - .detach_and_log_err(cx); - } + if !diff_hunks.iter().any(|hunk| { + hunk.row_range + .contains(&multi_buffer::MultiBufferRow(newest_cursor.row)) + }) { + return; } - fn update_selection( - &mut self, - diff_hunks: &[multi_buffer::MultiBufferDiffHunk], - window: &mut Window, - cx: &mut Context, - ) { - let snapshot = self.multibuffer.read(cx).snapshot(cx); - let target_hunk = diff_hunks + let target_hunk = { + diff_hunks .last() .and_then(|last_kept_hunk| { let last_kept_hunk_end = last_kept_hunk.multi_buffer_range().end; - self.editor - .read(cx) - .diff_hunks_in_ranges(&[last_kept_hunk_end..editor::Anchor::max()], &snapshot) + editor + .diff_hunks_in_ranges( + &[last_kept_hunk_end..editor::Anchor::max()], + buffer_snapshot, + ) .skip(1) .next() }) .or_else(|| { let first_kept_hunk = diff_hunks.first()?; let first_kept_hunk_start = first_kept_hunk.multi_buffer_range().start; - self.editor - .read(cx) + editor .diff_hunks_in_ranges( &[editor::Anchor::min()..first_kept_hunk_start], - &snapshot, + buffer_snapshot, ) .next() - }); + }) + }; - if let Some(target_hunk) = target_hunk { - self.editor.update(cx, |editor, cx| { - editor.change_selections(Some(Autoscroll::fit()), window, cx, |selections| { - let next_hunk_start = target_hunk.multi_buffer_range().start; - selections.select_anchor_ranges([next_hunk_start..next_hunk_start]); - }) - }); - } + if let Some(target_hunk) = target_hunk { + editor.change_selections(Some(Autoscroll::fit()), window, cx, |selections| { + let next_hunk_start = target_hunk.multi_buffer_range().start; + selections.select_anchor_ranges([next_hunk_start..next_hunk_start]); + }) } } -impl EventEmitter for AgentDiff {} +impl EventEmitter for AgentDiffPane {} -impl Focusable for AgentDiff { +impl Focusable for AgentDiffPane { fn focus_handle(&self, cx: &App) -> FocusHandle { if self.multibuffer.read(cx).is_empty() { self.focus_handle.clone() @@ -435,7 +446,7 @@ impl Focusable for AgentDiff { } } -impl Item for AgentDiff { +impl Item for AgentDiffPane { type Event = EditorEvent; fn tab_icon(&self, _window: &Window, _cx: &App) -> Option { @@ -603,7 +614,7 @@ impl Item for AgentDiff { } } -impl Render for AgentDiff { +impl Render for AgentDiffPane { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let is_empty = self.multibuffer.read(cx).is_empty(); let focus_handle = &self.focus_handle; @@ -650,20 +661,49 @@ impl Render for AgentDiff { } } +fn diff_hunk_controls(thread: &Entity) -> editor::RenderDiffHunkControlsFn { + let thread = thread.clone(); + + Arc::new( + move |row, + status: &DiffHunkStatus, + hunk_range, + is_created_file, + line_height, + editor: &Entity, + window: &mut Window, + cx: &mut App| { + { + render_diff_hunk_controls( + row, + status, + hunk_range, + is_created_file, + line_height, + &thread, + editor, + window, + cx, + ) + } + }, + ) +} + fn render_diff_hunk_controls( row: u32, _status: &DiffHunkStatus, hunk_range: Range, is_created_file: bool, line_height: Pixels, - agent_diff: &Entity, + thread: &Entity, editor: &Entity, window: &mut Window, cx: &mut App, ) -> AnyElement { let editor = editor.clone(); - if agent_diff.read(cx).thread.read(cx).is_generating() { + if thread.read(cx).is_generating() { return Empty.into_any(); } @@ -694,15 +734,20 @@ fn render_diff_hunk_controls( .map(|kb| kb.size(rems_from_px(12.))), ) .on_click({ - let agent_diff = agent_diff.clone(); + let editor = editor.clone(); + let thread = thread.clone(); move |_event, window, cx| { - agent_diff.update(cx, |diff, cx| { - diff.reject_edits_in_ranges( + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + reject_edits_in_ranges( + editor, + &snapshot, + &thread, vec![hunk_range.start..hunk_range.start], window, cx, ); - }); + }) } }), Button::new(("keep", row as u64), "Keep") @@ -711,10 +756,15 @@ fn render_diff_hunk_controls( .map(|kb| kb.size(rems_from_px(12.))), ) .on_click({ - let agent_diff = agent_diff.clone(); + let editor = editor.clone(); + let thread = thread.clone(); move |_event, window, cx| { - agent_diff.update(cx, |diff, cx| { - diff.keep_edits_in_ranges( + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + keep_edits_in_ranges( + editor, + &snapshot, + &thread, vec![hunk_range.start..hunk_range.start], window, cx, @@ -816,22 +866,40 @@ impl editor::Addon for AgentDiffAddon { } pub struct AgentDiffToolbar { - agent_diff: Option>, + active_item: Option, +} + +pub enum AgentDiffToolbarItem { + Pane(WeakEntity), + Editor { + editor: WeakEntity, + _diff_subscription: Subscription, + }, } impl AgentDiffToolbar { pub fn new() -> Self { - Self { agent_diff: None } - } - - fn agent_diff(&self, _: &App) -> Option> { - self.agent_diff.as_ref()?.upgrade() + Self { active_item: None } } fn dispatch_action(&self, action: &dyn Action, window: &mut Window, cx: &mut Context) { - if let Some(agent_diff) = self.agent_diff(cx) { - agent_diff.focus_handle(cx).focus(window); + let Some(active_item) = self.active_item.as_ref() else { + return; + }; + + match active_item { + AgentDiffToolbarItem::Pane(agent_diff) => { + if let Some(agent_diff) = agent_diff.upgrade() { + agent_diff.focus_handle(cx).focus(window); + } + } + AgentDiffToolbarItem::Editor { editor, .. } => { + if let Some(editor) = editor.upgrade() { + editor.read(cx).focus_handle(cx).focus(window); + } + } } + let action = action.boxed_clone(); cx.defer(move |cx| { cx.dispatch_action(action.as_ref()); @@ -848,14 +916,30 @@ impl ToolbarItemView for AgentDiffToolbar { _: &mut Window, cx: &mut Context, ) -> ToolbarItemLocation { - self.agent_diff = active_pane_item - .and_then(|item| item.act_as::(cx)) - .map(|entity| entity.downgrade()); - if self.agent_diff.is_some() { - ToolbarItemLocation::PrimaryRight - } else { - ToolbarItemLocation::Hidden + if let Some(item) = active_pane_item { + if let Some(pane) = item.act_as::(cx) { + self.active_item = Some(AgentDiffToolbarItem::Pane(pane.downgrade())); + return ToolbarItemLocation::PrimaryRight; + } + + if let Some(editor) = item.act_as::(cx) { + if editor.read(cx).mode().is_full() { + let agent_diff = AgentDiff::global(cx); + + self.active_item = Some(AgentDiffToolbarItem::Editor { + editor: editor.downgrade(), + _diff_subscription: cx.observe(&agent_diff, |_, _, cx| { + cx.notify(); + }), + }); + + return ToolbarItemLocation::PrimaryLeft; + } + } } + + self.active_item = None; + ToolbarItemLocation::Hidden } fn pane_focus_update( @@ -869,61 +953,766 @@ impl ToolbarItemView for AgentDiffToolbar { impl Render for AgentDiffToolbar { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - let agent_diff = match self.agent_diff(cx) { - Some(ad) => ad, - None => return div(), + let Some(active_item) = self.active_item.as_ref() else { + return Empty.into_any(); }; - let is_generating = agent_diff.read(cx).thread.read(cx).is_generating(); - if is_generating { - return div() - .w(rems(6.5625)) // Arbitrary 105px size—so the label doesn't dance around - .child(AnimatedLabel::new("Generating")); - } + match active_item { + AgentDiffToolbarItem::Editor { editor, .. } => { + let Some(editor) = editor.upgrade() else { + return Empty.into_any(); + }; - let is_empty = agent_diff.read(cx).multibuffer.read(cx).is_empty(); - if is_empty { - return div(); - } + let agent_diff = AgentDiff::global(cx); + let editor_focus_handle = editor.read(cx).focus_handle(cx); - let focus_handle = agent_diff.focus_handle(cx); + let content = match agent_diff.read(cx).editor_state(&editor) { + EditorState::Idle => return Empty.into_any(), + EditorState::Generating => vec![ + h_flex() + .ml_1() + .gap_1p5() + .w_32() + .child(Icon::new(IconName::ZedAssistant)) + .child( + div() + .w(rems(6.5625)) + .child(AnimatedLabel::new("Generating")), + ) + .into_any(), + ], + EditorState::Reviewing => vec![ + h_flex() + .child( + IconButton::new("hunk-up", IconName::ArrowUp) + .tooltip(ui::Tooltip::for_action_title_in( + "Previous Hunk", + &GoToPreviousHunk, + &editor_focus_handle, + )) + .on_click({ + let editor_focus_handle = editor_focus_handle.clone(); + move |_, window, cx| { + editor_focus_handle.dispatch_action( + &GoToPreviousHunk, + window, + cx, + ); + } + }), + ) + .child( + IconButton::new("hunk-down", IconName::ArrowDown) + .tooltip(ui::Tooltip::for_action_title_in( + "Next Hunk", + &GoToHunk, + &editor_focus_handle, + )) + .on_click({ + let editor_focus_handle = editor_focus_handle.clone(); + move |_, window, cx| { + editor_focus_handle + .dispatch_action(&GoToHunk, window, cx); + } + }), + ) + .into_any(), + vertical_divider().into_any_element(), + h_flex() + .child( + Button::new("reject-all", "Reject All") + .key_binding({ + KeyBinding::for_action_in( + &RejectAll, + &editor_focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))) + }) + .on_click(cx.listener(|this, _, window, cx| { + this.dispatch_action(&RejectAll, window, cx) + })), + ) + .child( + Button::new("keep-all", "Keep All") + .key_binding({ + KeyBinding::for_action_in( + &KeepAll, + &editor_focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))) + }) + .on_click(cx.listener(|this, _, window, cx| { + this.dispatch_action(&KeepAll, window, cx) + })), + ) + .into_any(), + ], + }; - h_group_xl() - .my_neg_1() - .items_center() - .p_1() - .flex_wrap() - .justify_between() - .child( - h_group_sm() + h_flex() + .bg(cx.theme().colors().surface_background) + .rounded_md() + .p_1() + .mx_2() + .gap_1() + .children(content) + .child(vertical_divider()) + .track_focus(&editor_focus_handle) + .on_action({ + let agent_diff = agent_diff.clone(); + let editor = editor.clone(); + move |_action: &OpenAgentDiff, window, cx| { + agent_diff.update(cx, |agent_diff, cx| { + agent_diff.deploy_pane_from_editor(&editor, window, cx); + }); + } + }) + .when_some(editor.read(cx).workspace(), |this, _workspace| { + this.child( + IconButton::new("review", IconName::ListTree) + .tooltip(ui::Tooltip::for_action_title_in( + "Review All Files", + &OpenAgentDiff, + &editor_focus_handle, + )) + .on_click({ + cx.listener(move |this, _, window, cx| { + this.dispatch_action(&OpenAgentDiff, window, cx); + }) + }), + ) + }) + .into_any() + } + AgentDiffToolbarItem::Pane(agent_diff) => { + let Some(agent_diff) = agent_diff.upgrade() else { + return Empty.into_any(); + }; + + let is_generating = agent_diff.read(cx).thread.read(cx).is_generating(); + if is_generating { + return div() + .w(rems(6.5625)) // Arbitrary 105px size—so the label doesn't dance around + .child(AnimatedLabel::new("Generating")) + .into_any(); + } + + let is_empty = agent_diff.read(cx).multibuffer.read(cx).is_empty(); + if is_empty { + return Empty.into_any(); + } + + let focus_handle = agent_diff.focus_handle(cx); + + h_group_xl() + .my_neg_1() + .items_center() + .p_1() + .flex_wrap() + .justify_between() .child( - Button::new("reject-all", "Reject All") - .key_binding({ - KeyBinding::for_action_in(&RejectAll, &focus_handle, window, cx) - .map(|kb| kb.size(rems_from_px(12.))) - }) - .on_click(cx.listener(|this, _, window, cx| { - this.dispatch_action(&RejectAll, window, cx) - })), + h_group_sm() + .child( + Button::new("reject-all", "Reject All") + .key_binding({ + KeyBinding::for_action_in( + &RejectAll, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))) + }) + .on_click(cx.listener(|this, _, window, cx| { + this.dispatch_action(&RejectAll, window, cx) + })), + ) + .child( + Button::new("keep-all", "Keep All") + .key_binding({ + KeyBinding::for_action_in( + &KeepAll, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))) + }) + .on_click(cx.listener(|this, _, window, cx| { + this.dispatch_action(&KeepAll, window, cx) + })), + ), ) - .child( - Button::new("keep-all", "Keep All") - .key_binding({ - KeyBinding::for_action_in(&KeepAll, &focus_handle, window, cx) - .map(|kb| kb.size(rems_from_px(12.))) + .into_any() + } + } + } +} + +pub struct AgentDiff { + reviewing_editors: HashMap, EditorState>, + workspace_threads: HashMap, WorkspaceThread>, + _settings_subscription: Subscription, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +enum EditorState { + Idle, + Reviewing, + Generating, +} + +struct WorkspaceThread { + thread: WeakEntity, + _thread_subscriptions: [Subscription; 2], + singleton_editors: HashMap, HashMap, Subscription>>, + _workspace_subscription: Option, +} + +struct AgentDiffGlobal(Entity); + +impl Global for AgentDiffGlobal {} + +impl AgentDiff { + fn global(cx: &mut App) -> Entity { + cx.try_global::() + .map(|global| global.0.clone()) + .unwrap_or_else(|| { + let entity = cx.new(Self::new); + let global = AgentDiffGlobal(entity.clone()); + cx.set_global(global); + entity.clone() + }) + } + + fn new(cx: &mut Context) -> Self { + let mut was_active = AssistantSettings::get_global(cx).single_file_review; + let settings_subscription = cx.observe_global::(move |this, cx| { + let is_active = AssistantSettings::get_global(cx).single_file_review; + + if was_active != is_active { + let workspaces = this.workspace_threads.keys().cloned().collect::>(); + for workspace in workspaces { + this.update_reviewing_editors(&workspace, cx); + } + } + + was_active = is_active; + }); + + Self { + reviewing_editors: HashMap::default(), + workspace_threads: HashMap::default(), + _settings_subscription: settings_subscription, + } + } + + pub fn set_active_thread( + workspace: &WeakEntity, + thread: &Entity, + cx: &mut App, + ) { + Self::global(cx).update(cx, |this, cx| { + this.register_active_thread_impl(workspace, thread, cx); + }); + } + + fn register_active_thread_impl( + &mut self, + workspace: &WeakEntity, + thread: &Entity, + cx: &mut Context, + ) { + let agent_diff = cx.entity(); + let action_log = thread.read(cx).action_log().clone(); + + let action_log_subscription = cx.observe(&action_log, { + let workspace = workspace.clone(); + move |this, _action_log, cx| { + this.update_reviewing_editors(&workspace, cx); + } + }); + + let thread_subscription = cx.subscribe(&thread, { + let workspace = workspace.clone(); + move |this, _thread, event, cx| this.handle_thread_event(&workspace, event, cx) + }); + + if let Some(workspace_thread) = self.workspace_threads.get_mut(&workspace) { + // replace thread and action log subscription, but keep editors + workspace_thread.thread = thread.downgrade(); + workspace_thread._thread_subscriptions = [action_log_subscription, thread_subscription]; + self.update_reviewing_editors(&workspace, cx); + return; + } + + let workspace_subscription = workspace + .upgrade() + .map(|workspace| cx.subscribe(&workspace, Self::handle_workspace_event)); + + self.workspace_threads.insert( + workspace.clone(), + WorkspaceThread { + thread: thread.downgrade(), + _thread_subscriptions: [action_log_subscription, thread_subscription], + singleton_editors: HashMap::default(), + _workspace_subscription: workspace_subscription, + }, + ); + + let workspace = workspace.clone(); + cx.defer(move |cx| { + if let Some(strong_workspace) = workspace.upgrade() { + agent_diff.update(cx, |this, cx| { + this.register_workspace(strong_workspace, cx); + }) + } + }); + } + + fn register_workspace(&mut self, workspace: Entity, cx: &mut Context) { + let agent_diff = cx.entity(); + + let editors = workspace.update(cx, |workspace, cx| { + let agent_diff = agent_diff.clone(); + + Self::register_review_action::(workspace, Self::keep, &agent_diff); + Self::register_review_action::(workspace, Self::reject, &agent_diff); + Self::register_review_action::(workspace, Self::keep_all, &agent_diff); + Self::register_review_action::(workspace, Self::reject_all, &agent_diff); + + workspace.items_of_type(cx).collect::>() + }); + + let weak_workspace = workspace.downgrade(); + + for editor in editors { + if let Some(buffer) = Self::full_editor_buffer(editor.read(cx), cx) { + self.register_editor(weak_workspace.clone(), buffer, editor, cx); + }; + } + + self.update_reviewing_editors(&weak_workspace, cx); + } + + fn register_review_action( + workspace: &mut Workspace, + review: impl Fn(&Entity, &Entity, &mut Window, &mut App) -> PostReviewState + + 'static, + this: &Entity, + ) { + let this = this.clone(); + workspace.register_action(move |workspace, _: &T, window, cx| { + let review = &review; + let task = this.update(cx, |this, cx| { + this.review_in_active_editor(workspace, review, window, cx) + }); + + if let Some(task) = task { + task.detach_and_log_err(cx); + } else { + cx.propagate(); + } + }); + } + + fn handle_thread_event( + &mut self, + workspace: &WeakEntity, + event: &ThreadEvent, + cx: &mut Context, + ) { + match event { + ThreadEvent::NewRequest + | ThreadEvent::Stopped(Ok(StopReason::EndTurn)) + | ThreadEvent::Stopped(Ok(StopReason::MaxTokens)) + | ThreadEvent::Stopped(Err(_)) + | ThreadEvent::ShowError(_) + | ThreadEvent::CompletionCanceled => { + self.update_reviewing_editors(workspace, cx); + } + // intentionally being exhaustive in case we add a variant we should handle + ThreadEvent::Stopped(Ok(StopReason::ToolUse)) + | ThreadEvent::UsageUpdated(_) + | ThreadEvent::StreamedCompletion + | ThreadEvent::ReceivedTextChunk + | ThreadEvent::StreamedAssistantText(_, _) + | ThreadEvent::StreamedAssistantThinking(_, _) + | ThreadEvent::StreamedToolUse { .. } + | ThreadEvent::InvalidToolInput { .. } + | ThreadEvent::MessageAdded(_) + | ThreadEvent::MessageEdited(_) + | ThreadEvent::MessageDeleted(_) + | ThreadEvent::SummaryGenerated + | ThreadEvent::SummaryChanged + | ThreadEvent::UsePendingTools { .. } + | ThreadEvent::ToolFinished { .. } + | ThreadEvent::CheckpointChanged + | ThreadEvent::ToolConfirmationNeeded + | ThreadEvent::CancelEditing => {} + } + } + + fn handle_workspace_event( + &mut self, + workspace: Entity, + event: &workspace::Event, + cx: &mut Context, + ) { + match event { + workspace::Event::ItemAdded { item } => { + if let Some(editor) = item.downcast::() { + if let Some(buffer) = Self::full_editor_buffer(editor.read(cx), cx) { + self.register_editor(workspace.downgrade(), buffer.clone(), editor, cx); + } + } + } + _ => {} + } + } + + fn full_editor_buffer(editor: &Editor, cx: &App) -> Option> { + if editor.mode().is_full() { + editor + .buffer() + .read(cx) + .as_singleton() + .map(|buffer| buffer.downgrade()) + } else { + None + } + } + + fn register_editor( + &mut self, + workspace: WeakEntity, + buffer: WeakEntity, + editor: Entity, + cx: &mut Context, + ) { + let Some(workspace_thread) = self.workspace_threads.get_mut(&workspace) else { + return; + }; + + let weak_editor = editor.downgrade(); + + workspace_thread + .singleton_editors + .entry(buffer.clone()) + .or_default() + .entry(weak_editor.clone()) + .or_insert_with(|| { + let workspace = workspace.clone(); + cx.observe_release(&editor, move |this, _, _cx| { + let Some(active_thread) = this.workspace_threads.get_mut(&workspace) else { + return; + }; + + if let Entry::Occupied(mut entry) = + active_thread.singleton_editors.entry(buffer) + { + let set = entry.get_mut(); + set.remove(&weak_editor); + + if set.is_empty() { + entry.remove(); + } + } + }) + }); + + self.update_reviewing_editors(&workspace, cx); + } + + fn update_reviewing_editors( + &mut self, + workspace: &WeakEntity, + cx: &mut Context, + ) { + if !AssistantSettings::get_global(cx).single_file_review { + for (editor, _) in self.reviewing_editors.drain() { + editor + .update(cx, |editor, cx| editor.end_temporary_diff_override(cx)) + .ok(); + } + return; + } + + let Some(workspace_thread) = self.workspace_threads.get_mut(workspace) else { + return; + }; + + let Some(thread) = workspace_thread.thread.upgrade() else { + return; + }; + + let action_log = thread.read(cx).action_log(); + let changed_buffers = action_log.read(cx).changed_buffers(cx); + + let mut unaffected = self.reviewing_editors.clone(); + + for (buffer, diff_handle) in changed_buffers { + if buffer.read(cx).file().is_none() { + continue; + } + + let Some(buffer_editors) = workspace_thread.singleton_editors.get(&buffer.downgrade()) + else { + continue; + }; + + for (weak_editor, _) in buffer_editors { + let Some(editor) = weak_editor.upgrade() else { + continue; + }; + + let multibuffer = editor.read(cx).buffer().clone(); + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.add_diff(diff_handle.clone(), cx); + }); + + let new_state = if thread.read(cx).is_generating() { + EditorState::Generating + } else { + EditorState::Reviewing + }; + + let previous_state = self + .reviewing_editors + .insert(weak_editor.clone(), new_state.clone()); + + if previous_state.is_none() { + editor.update(cx, |editor, cx| { + editor.start_temporary_diff_override(); + editor.set_render_diff_hunk_controls(diff_hunk_controls(&thread), cx); + editor.set_expand_all_diff_hunks(cx); + editor.register_addon(EditorAgentDiffAddon); + }); + } else { + unaffected.remove(&weak_editor); + } + + if new_state == EditorState::Reviewing && previous_state != Some(new_state) { + if let Some(workspace) = workspace.upgrade() { + let workspace_id = workspace.entity_id(); + let workspace_window = cx.windows().iter().find_map(|w| { + w.downcast::().and_then(|window_workspace| { + if window_workspace + .entity(cx) + .map_or(false, |entity| entity.entity_id() == workspace_id) + { + Some(window_workspace) + } else { + None + } }) - .on_click(cx.listener(|this, _, window, cx| { - this.dispatch_action(&KeepAll, window, cx) - })), - ), - ) + }); + + if let Some(workspace_window) = workspace_window { + workspace_window + .update(cx, |_, window, cx| { + editor.update(cx, |editor, cx| { + editor.go_to_next_hunk(&GoToHunk, window, cx); + }); + }) + .log_err(); + } + } + } + } + } + + // Remove editors from this workspace that are no longer under review + for (editor, _) in unaffected { + // Note: We could avoid this check by storing `reviewing_editors` by Workspace, + // but that would add another lookup in `AgentDiff::editor_state` + // which gets called much more frequently. + let in_workspace = editor + .read_with(cx, |editor, _cx| editor.workspace()) + .ok() + .flatten() + .map_or(false, |editor_workspace| { + editor_workspace.entity_id() == workspace.entity_id() + }); + + if in_workspace { + editor + .update(cx, |editor, cx| editor.end_temporary_diff_override(cx)) + .ok(); + self.reviewing_editors.remove(&editor); + } + } + + cx.notify(); + } + + fn editor_state(&self, editor: &Entity) -> &EditorState { + self.reviewing_editors + .get(&editor.downgrade()) + .unwrap_or(&EditorState::Idle) + } + + fn deploy_pane_from_editor(&self, editor: &Entity, window: &mut Window, cx: &mut App) { + let Some(workspace) = editor.read(cx).workspace() else { + return; + }; + + let Some(WorkspaceThread { thread, .. }) = + self.workspace_threads.get(&workspace.downgrade()) + else { + return; + }; + + let Some(thread) = thread.upgrade() else { + return; + }; + + AgentDiffPane::deploy(thread, workspace.downgrade(), window, cx).log_err(); + } + + fn keep_all( + editor: &Entity, + thread: &Entity, + window: &mut Window, + cx: &mut App, + ) -> PostReviewState { + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + keep_edits_in_ranges( + editor, + &snapshot, + thread, + vec![editor::Anchor::min()..editor::Anchor::max()], + window, + cx, + ); + }); + PostReviewState::AllReviewed + } + + fn reject_all( + editor: &Entity, + thread: &Entity, + window: &mut Window, + cx: &mut App, + ) -> PostReviewState { + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + reject_edits_in_ranges( + editor, + &snapshot, + thread, + vec![editor::Anchor::min()..editor::Anchor::max()], + window, + cx, + ); + }); + PostReviewState::AllReviewed + } + + fn keep( + editor: &Entity, + thread: &Entity, + window: &mut Window, + cx: &mut App, + ) -> PostReviewState { + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + keep_edits_in_selection(editor, &snapshot, thread, window, cx); + Self::post_review_state(&snapshot) + }) + } + + fn reject( + editor: &Entity, + thread: &Entity, + window: &mut Window, + cx: &mut App, + ) -> PostReviewState { + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + reject_edits_in_selection(editor, &snapshot, thread, window, cx); + Self::post_review_state(&snapshot) + }) + } + + fn post_review_state(snapshot: &MultiBufferSnapshot) -> PostReviewState { + for (i, _) in snapshot.diff_hunks().enumerate() { + if i > 0 { + return PostReviewState::Pending; + } + } + PostReviewState::AllReviewed + } + + fn review_in_active_editor( + &mut self, + workspace: &mut Workspace, + review: impl Fn(&Entity, &Entity, &mut Window, &mut App) -> PostReviewState, + window: &mut Window, + cx: &mut Context, + ) -> Option>> { + let active_item = workspace.active_item(cx)?; + let editor = active_item.act_as::(cx)?; + + if !matches!(self.editor_state(&editor), EditorState::Reviewing) { + return None; + } + + let WorkspaceThread { thread, .. } = + self.workspace_threads.get(&workspace.weak_handle())?; + + let thread = thread.upgrade()?; + + if let PostReviewState::AllReviewed = review(&editor, &thread, window, cx) { + if let Some(curr_buffer) = editor.read(cx).buffer().read(cx).as_singleton() { + let changed_buffers = thread.read(cx).action_log().read(cx).changed_buffers(cx); + + let mut keys = changed_buffers.keys().cycle(); + keys.find(|k| *k == &curr_buffer); + let next_project_path = keys + .next() + .filter(|k| *k != &curr_buffer) + .and_then(|after| after.read(cx).project_path(cx)); + + if let Some(path) = next_project_path { + let task = workspace.open_path(path, None, true, window, cx); + let task = cx.spawn(async move |_, _cx| task.await.map(|_| ())); + return Some(task); + } + } + } + + return Some(Task::ready(Ok(()))); + } +} + +enum PostReviewState { + AllReviewed, + Pending, +} + +pub struct EditorAgentDiffAddon; + +impl editor::Addon for EditorAgentDiffAddon { + fn to_any(&self) -> &dyn std::any::Any { + self + } + + fn extend_key_context(&self, key_context: &mut gpui::KeyContext, _: &App) { + key_context.add("agent_diff"); + key_context.add("editor_agent_diff"); } } #[cfg(test)] mod tests { use super::*; - use crate::{ThreadStore, thread_store}; + use crate::{Keep, ThreadStore, thread_store}; use assistant_settings::AssistantSettings; use assistant_tool::ToolWorkingSet; use context_server::ContextServerSettings; @@ -938,7 +1727,7 @@ mod tests { use util::path; #[gpui::test] - async fn test_agent_diff(cx: &mut TestAppContext) { + async fn test_multibuffer_agent_diff(cx: &mut TestAppContext) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); @@ -986,7 +1775,7 @@ mod tests { let (workspace, cx) = cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); let agent_diff = cx.new_window_entity(|window, cx| { - AgentDiff::new(thread.clone(), workspace.downgrade(), window, cx) + AgentDiffPane::new(thread.clone(), workspace.downgrade(), window, cx) }); let editor = agent_diff.read_with(cx, |diff, _cx| diff.editor.clone()); @@ -1027,7 +1816,7 @@ mod tests { ); // After keeping a hunk, the cursor should be positioned on the second hunk. - agent_diff.update_in(cx, |diff, window, cx| diff.keep(&crate::Keep, window, cx)); + agent_diff.update_in(cx, |diff, window, cx| diff.keep(&Keep, window, cx)); cx.run_until_parked(); assert_eq!( editor.read_with(cx, |editor, cx| editor.text(cx)), @@ -1062,14 +1851,24 @@ mod tests { ); // Keeping a range that doesn't intersect the current selection doesn't move it. - agent_diff.update_in(cx, |diff, window, cx| { + agent_diff.update_in(cx, |_diff, window, cx| { let position = editor .read(cx) .buffer() .read(cx) .read(cx) .anchor_before(Point::new(7, 0)); - diff.keep_edits_in_ranges(vec![position..position], window, cx) + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + keep_edits_in_ranges( + editor, + &snapshot, + &thread, + vec![position..position], + window, + cx, + ) + }); }); cx.run_until_parked(); assert_eq!( @@ -1083,4 +1882,226 @@ mod tests { Point::new(3, 0)..Point::new(3, 0) ); } + + #[gpui::test] + async fn test_singleton_agent_diff(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + language::init(cx); + Project::init_settings(cx); + AssistantSettings::register(cx); + prompt_store::init(cx); + thread_store::init(cx); + workspace::init_settings(cx); + ThemeSettings::register(cx); + ContextServerSettings::register(cx); + EditorSettings::register(cx); + language_model::init_settings(cx); + workspace::register_project_item::(cx); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/test"), + json!({"file1": "abc\ndef\nghi\njkl\nmno\npqr\nstu\nvwx\nyz"}), + ) + .await; + fs.insert_tree(path!("/test"), json!({"file2": "abc\ndef\nghi"})) + .await; + + let project = Project::test(fs, [path!("/test").as_ref()], cx).await; + let buffer_path1 = project + .read_with(cx, |project, cx| { + project.find_project_path("test/file1", cx) + }) + .unwrap(); + let buffer_path2 = project + .read_with(cx, |project, cx| { + project.find_project_path("test/file2", cx) + }) + .unwrap(); + + let prompt_store = None; + let thread_store = cx + .update(|cx| { + ThreadStore::load( + project.clone(), + cx.new(|_| ToolWorkingSet::default()), + prompt_store, + Arc::new(PromptBuilder::new(None).unwrap()), + cx, + ) + }) + .await + .unwrap(); + let thread = thread_store.update(cx, |store, cx| store.create_thread(cx)); + let action_log = thread.read_with(cx, |thread, _| thread.action_log().clone()); + + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + + // Set the active thread + cx.update(|_, cx| AgentDiff::set_active_thread(&workspace.downgrade(), &thread, cx)); + + let buffer1 = project + .update(cx, |project, cx| { + project.open_buffer(buffer_path1.clone(), cx) + }) + .await + .unwrap(); + let buffer2 = project + .update(cx, |project, cx| { + project.open_buffer(buffer_path2.clone(), cx) + }) + .await + .unwrap(); + + // Open an editor for buffer1 + let editor1 = cx.new_window_entity(|window, cx| { + Editor::for_buffer(buffer1.clone(), Some(project.clone()), window, cx) + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(editor1.clone()), None, true, window, cx); + }); + cx.run_until_parked(); + + // Make changes + cx.update(|_, cx| { + action_log.update(cx, |log, cx| log.track_buffer(buffer1.clone(), cx)); + buffer1.update(cx, |buffer, cx| { + buffer + .edit( + [ + (Point::new(1, 1)..Point::new(1, 2), "E"), + (Point::new(3, 2)..Point::new(3, 3), "L"), + (Point::new(5, 0)..Point::new(5, 1), "P"), + (Point::new(7, 1)..Point::new(7, 2), "W"), + ], + None, + cx, + ) + .unwrap() + }); + action_log.update(cx, |log, cx| log.buffer_edited(buffer1.clone(), cx)); + + action_log.update(cx, |log, cx| log.track_buffer(buffer2.clone(), cx)); + buffer2.update(cx, |buffer, cx| { + buffer + .edit([(Point::new(2, 1)..Point::new(2, 2), "H")], None, cx) + .unwrap(); + }); + action_log.update(cx, |log, cx| log.buffer_edited(buffer2.clone(), cx)); + }); + cx.run_until_parked(); + + // The already opened editor displays the diff and the cursor is at the first hunk + assert_eq!( + editor1.read_with(cx, |editor, cx| editor.text(cx)), + "abc\ndef\ndEf\nghi\njkl\njkL\nmno\npqr\nPqr\nstu\nvwx\nvWx\nyz" + ); + assert_eq!( + editor1 + .update(cx, |editor, cx| editor.selections.newest::(cx)) + .range(), + Point::new(1, 0)..Point::new(1, 0) + ); + + // After keeping a hunk, the cursor should be positioned on the second hunk. + workspace.update(cx, |_, cx| { + cx.dispatch_action(&Keep); + }); + cx.run_until_parked(); + assert_eq!( + editor1.read_with(cx, |editor, cx| editor.text(cx)), + "abc\ndEf\nghi\njkl\njkL\nmno\npqr\nPqr\nstu\nvwx\nvWx\nyz" + ); + assert_eq!( + editor1 + .update(cx, |editor, cx| editor.selections.newest::(cx)) + .range(), + Point::new(3, 0)..Point::new(3, 0) + ); + + // Rejecting a hunk also moves the cursor to the next hunk, possibly cycling if it's at the end. + editor1.update_in(cx, |editor, window, cx| { + editor.change_selections(None, window, cx, |selections| { + selections.select_ranges([Point::new(10, 0)..Point::new(10, 0)]) + }); + }); + workspace.update(cx, |_, cx| { + cx.dispatch_action(&Reject); + }); + cx.run_until_parked(); + assert_eq!( + editor1.read_with(cx, |editor, cx| editor.text(cx)), + "abc\ndEf\nghi\njkl\njkL\nmno\npqr\nPqr\nstu\nvwx\nyz" + ); + assert_eq!( + editor1 + .update(cx, |editor, cx| editor.selections.newest::(cx)) + .range(), + Point::new(3, 0)..Point::new(3, 0) + ); + + // Keeping a range that doesn't intersect the current selection doesn't move it. + editor1.update_in(cx, |editor, window, cx| { + let buffer = editor.buffer().read(cx); + let position = buffer.read(cx).anchor_before(Point::new(7, 0)); + let snapshot = buffer.snapshot(cx); + keep_edits_in_ranges( + editor, + &snapshot, + &thread, + vec![position..position], + window, + cx, + ) + }); + cx.run_until_parked(); + assert_eq!( + editor1.read_with(cx, |editor, cx| editor.text(cx)), + "abc\ndEf\nghi\njkl\njkL\nmno\nPqr\nstu\nvwx\nyz" + ); + assert_eq!( + editor1 + .update(cx, |editor, cx| editor.selections.newest::(cx)) + .range(), + Point::new(3, 0)..Point::new(3, 0) + ); + + // Reviewing the last change opens the next changed buffer + workspace + .update_in(cx, |workspace, window, cx| { + AgentDiff::global(cx).update(cx, |agent_diff, cx| { + agent_diff.review_in_active_editor(workspace, AgentDiff::keep, window, cx) + }) + }) + .unwrap() + .await + .unwrap(); + + cx.run_until_parked(); + + let editor2 = workspace.update(cx, |workspace, cx| { + workspace.active_item_as::(cx).unwrap() + }); + + let editor2_path = editor2 + .read_with(cx, |editor, cx| editor.project_path(cx)) + .unwrap(); + assert_eq!(editor2_path, buffer_path2); + + assert_eq!( + editor2.read_with(cx, |editor, cx| editor.text(cx)), + "abc\ndef\nghi\ngHi" + ); + assert_eq!( + editor2 + .update(cx, |editor, cx| editor.selections.newest::(cx)) + .range(), + Point::new(2, 0)..Point::new(2, 0) + ); + } } diff --git a/crates/agent/src/assistant.rs b/crates/agent/src/assistant.rs index aa919cc561..8f45ca042b 100644 --- a/crates/agent/src/assistant.rs +++ b/crates/agent/src/assistant.rs @@ -45,7 +45,7 @@ pub use crate::context::{ContextLoadResult, LoadedContext}; pub use crate::inline_assistant::InlineAssistant; pub use crate::thread::{Message, MessageSegment, Thread, ThreadEvent}; pub use crate::thread_store::ThreadStore; -pub use agent_diff::{AgentDiff, AgentDiffToolbar}; +pub use agent_diff::{AgentDiffPane, AgentDiffToolbar}; pub use context_store::ContextStore; pub use ui::{all_agent_previews, get_agent_preview}; diff --git a/crates/agent/src/assistant_panel.rs b/crates/agent/src/assistant_panel.rs index 0b5d99867f..28d449b249 100644 --- a/crates/agent/src/assistant_panel.rs +++ b/crates/agent/src/assistant_panel.rs @@ -43,6 +43,7 @@ use zed_actions::agent::OpenConfiguration; use zed_actions::assistant::{OpenRulesLibrary, ToggleFocus}; use crate::active_thread::{ActiveThread, ActiveThreadEvent}; +use crate::agent_diff::AgentDiff; use crate::assistant_configuration::{AssistantConfiguration, AssistantConfigurationEvent}; use crate::history_store::{HistoryEntry, HistoryStore, RecentEntry}; use crate::message_editor::{MessageEditor, MessageEditorEvent}; @@ -51,9 +52,9 @@ use crate::thread_history::{PastContext, PastThread, ThreadHistory}; use crate::thread_store::ThreadStore; use crate::ui::UsageBanner; use crate::{ - AddContextServer, AgentDiff, DeleteRecentlyOpenThread, ExpandMessageEditor, InlineAssistant, - NewTextThread, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff, OpenHistory, ThreadEvent, - ToggleContextPicker, ToggleNavigationMenu, ToggleOptionsMenu, + AddContextServer, AgentDiffPane, DeleteRecentlyOpenThread, ExpandMessageEditor, + InlineAssistant, NewTextThread, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff, + OpenHistory, ThreadEvent, ToggleContextPicker, ToggleNavigationMenu, ToggleOptionsMenu, }; const AGENT_PANEL_KEY: &str = "agent_panel"; @@ -103,7 +104,7 @@ pub fn init(cx: &mut App) { if let Some(panel) = workspace.panel::(cx) { workspace.focus_panel::(window, cx); let thread = panel.read(cx).thread.read(cx).thread().clone(); - AgentDiff::deploy_in_workspace(thread, workspace, window, cx); + AgentDiffPane::deploy_in_workspace(thread, workspace, window, cx); } }) .register_action(|workspace, _: &ExpandMessageEditor, window, cx| { @@ -474,6 +475,7 @@ impl AssistantPanel { cx, ) }); + AgentDiff::set_active_thread(&workspace, &thread, cx); let active_thread_subscription = cx.subscribe(&active_thread, |_, _, event, cx| match &event { @@ -717,6 +719,7 @@ impl AssistantPanel { cx, ) }); + AgentDiff::set_active_thread(&self.workspace, &thread, cx); let active_thread_subscription = cx.subscribe(&self.thread, |_, _, event, cx| match &event { @@ -914,6 +917,7 @@ impl AssistantPanel { cx, ) }); + AgentDiff::set_active_thread(&self.workspace, &thread, cx); let active_thread_subscription = cx.subscribe(&self.thread, |_, _, event, cx| match &event { @@ -989,7 +993,7 @@ impl AssistantPanel { let thread = self.thread.read(cx).thread().clone(); self.workspace .update(cx, |workspace, cx| { - AgentDiff::deploy_in_workspace(thread, workspace, window, cx) + AgentDiffPane::deploy_in_workspace(thread, workspace, window, cx) }) .log_err(); } diff --git a/crates/agent/src/message_editor.rs b/crates/agent/src/message_editor.rs index 7299e01f64..ebb041c8fd 100644 --- a/crates/agent/src/message_editor.rs +++ b/crates/agent/src/message_editor.rs @@ -42,8 +42,8 @@ use crate::profile_selector::ProfileSelector; use crate::thread::{MessageCrease, Thread, TokenUsageRatio}; use crate::thread_store::ThreadStore; use crate::{ - ActiveThread, AgentDiff, Chat, ExpandMessageEditor, NewThread, OpenAgentDiff, RemoveAllContext, - ToggleContextPicker, ToggleProfileSelector, register_agent_preview, + ActiveThread, AgentDiffPane, Chat, ExpandMessageEditor, NewThread, OpenAgentDiff, + RemoveAllContext, ToggleContextPicker, ToggleProfileSelector, register_agent_preview, }; #[derive(RegisterComponent)] @@ -168,6 +168,9 @@ impl MessageEditor { // When context changes, reload it for token counting. let _ = this.reload_context(cx); }), + cx.observe(&thread.read(cx).action_log().clone(), |_, _, cx| { + cx.notify() + }), ]; let model_selector = cx.new(|cx| { @@ -404,7 +407,7 @@ impl MessageEditor { fn handle_review_click(&mut self, window: &mut Window, cx: &mut Context) { self.edits_expanded = true; - AgentDiff::deploy(self.thread.clone(), self.workspace.clone(), window, cx).log_err(); + AgentDiffPane::deploy(self.thread.clone(), self.workspace.clone(), window, cx).log_err(); cx.notify(); } @@ -414,7 +417,8 @@ impl MessageEditor { window: &mut Window, cx: &mut Context, ) { - if let Ok(diff) = AgentDiff::deploy(self.thread.clone(), self.workspace.clone(), window, cx) + if let Ok(diff) = + AgentDiffPane::deploy(self.thread.clone(), self.workspace.clone(), window, cx) { let path_key = multi_buffer::PathKey::for_buffer(&buffer, cx); diff.update(cx, |diff, cx| diff.move_to_path(path_key, window, cx)); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index fe9295341f..5942f363ed 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -1325,13 +1325,14 @@ impl Thread { let mut stop_reason = StopReason::EndTurn; let mut current_token_usage = TokenUsage::default(); - if let Some(usage) = usage { - thread - .update(cx, |_thread, cx| { + thread + .update(cx, |_thread, cx| { + if let Some(usage) = usage { cx.emit(ThreadEvent::UsageUpdated(usage)); - }) - .ok(); - } + } + cx.emit(ThreadEvent::NewRequest); + }) + .ok(); let mut request_assistant_message_id = None; @@ -1962,6 +1963,11 @@ impl Thread { } self.finalize_pending_checkpoint(cx); + + if canceled { + cx.emit(ThreadEvent::CompletionCanceled); + } + canceled } @@ -2463,6 +2469,7 @@ pub enum ThreadEvent { UsageUpdated(RequestUsage), StreamedCompletion, ReceivedTextChunk, + NewRequest, StreamedAssistantText(MessageId, String), StreamedAssistantThinking(MessageId, String), StreamedToolUse { @@ -2493,6 +2500,7 @@ pub enum ThreadEvent { CheckpointChanged, ToolConfirmationNeeded, CancelEditing, + CompletionCanceled, } impl EventEmitter for Thread {} diff --git a/crates/assistant_settings/src/assistant_settings.rs b/crates/assistant_settings/src/assistant_settings.rs index 80f1f1ef34..5d805aaa5a 100644 --- a/crates/assistant_settings/src/assistant_settings.rs +++ b/crates/assistant_settings/src/assistant_settings.rs @@ -69,7 +69,7 @@ pub enum AssistantProviderContentV1 { }, } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct AssistantSettings { pub enabled: bool, pub button: bool, @@ -88,6 +88,32 @@ pub struct AssistantSettings { pub always_allow_tool_actions: bool, pub notify_when_agent_waiting: NotifyWhenAgentWaiting, pub stream_edits: bool, + pub single_file_review: bool, +} + +impl Default for AssistantSettings { + fn default() -> Self { + Self { + enabled: Default::default(), + button: Default::default(), + dock: Default::default(), + default_width: Default::default(), + default_height: Default::default(), + default_model: Default::default(), + inline_assistant_model: Default::default(), + commit_message_model: Default::default(), + thread_summary_model: Default::default(), + inline_alternatives: Default::default(), + using_outdated_settings_version: Default::default(), + enable_experimental_live_diffs: Default::default(), + default_profile: Default::default(), + profiles: Default::default(), + always_allow_tool_actions: Default::default(), + notify_when_agent_waiting: Default::default(), + stream_edits: Default::default(), + single_file_review: true, + } + } } impl AssistantSettings { @@ -224,6 +250,7 @@ impl AssistantSettingsContent { always_allow_tool_actions: None, notify_when_agent_waiting: None, stream_edits: None, + single_file_review: None, }, VersionedAssistantSettingsContent::V2(ref settings) => settings.clone(), }, @@ -252,6 +279,7 @@ impl AssistantSettingsContent { always_allow_tool_actions: None, notify_when_agent_waiting: None, stream_edits: None, + single_file_review: None, }, None => AssistantSettingsContentV2::default(), } @@ -503,6 +531,7 @@ impl Default for VersionedAssistantSettingsContent { always_allow_tool_actions: None, notify_when_agent_waiting: None, stream_edits: None, + single_file_review: None, }) } } @@ -562,6 +591,10 @@ pub struct AssistantSettingsContentV2 { /// /// Default: false stream_edits: Option, + /// Whether to display agent edits in single-file editors in addition to the review multibuffer pane. + /// + /// Default: true + single_file_review: Option, } #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)] @@ -725,6 +758,7 @@ impl Settings for AssistantSettings { value.notify_when_agent_waiting, ); merge(&mut settings.stream_edits, value.stream_edits); + merge(&mut settings.single_file_review, value.single_file_review); merge(&mut settings.default_profile, value.default_profile); if let Some(profiles) = value.profiles { @@ -857,6 +891,7 @@ mod tests { always_allow_tool_actions: None, notify_when_agent_waiting: None, stream_edits: None, + single_file_review: None, }, )), } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0aa85c05c1..778d2b60ca 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -981,6 +981,8 @@ pub struct Editor { addons: HashMap>, registered_buffers: HashMap, load_diff_task: Option>>, + /// Whether we are temporarily displaying a diff other than git's + temporary_diff_override: bool, selection_mark_mode: bool, toggle_fold_multiple_buffers: Task<()>, _scroll_cursor_center_top_bottom_task: Task<()>, @@ -1626,7 +1628,8 @@ impl Editor { let mut load_uncommitted_diff = None; if let Some(project) = project.clone() { load_uncommitted_diff = Some( - get_uncommitted_diff_for_buffer( + update_uncommitted_diff_for_buffer( + cx.entity(), &project, buffer.read(cx).all_buffers(), buffer.clone(), @@ -1802,6 +1805,7 @@ impl Editor { serialize_folds: Task::ready(()), text_style_refinement: None, load_diff_task: load_uncommitted_diff, + temporary_diff_override: false, mouse_cursor_hidden: false, hide_mouse_mode: EditorSettings::get_global(cx) .hide_mouse @@ -1941,7 +1945,7 @@ impl Editor { .is_some_and(|menu| menu.context_menu.focus_handle(cx).is_focused(window)) } - fn key_context(&self, window: &Window, cx: &App) -> KeyContext { + pub fn key_context(&self, window: &Window, cx: &App) -> KeyContext { self.key_context_internal(self.has_active_inline_completion(), window, cx) } @@ -13649,7 +13653,7 @@ impl Editor { self.refresh_inline_completion(false, true, window, cx); } - fn go_to_next_hunk(&mut self, _: &GoToHunk, window: &mut Window, cx: &mut Context) { + pub fn go_to_next_hunk(&mut self, _: &GoToHunk, window: &mut Window, cx: &mut Context) { self.hide_mouse_cursor(&HideMouseCursorOrigin::MovementAction); let snapshot = self.snapshot(window, cx); let selection = self.selections.newest::(cx); @@ -17820,7 +17824,8 @@ impl Editor { let buffer_id = buffer.read(cx).remote_id(); if self.buffer.read(cx).diff_for(buffer_id).is_none() { if let Some(project) = &self.project { - get_uncommitted_diff_for_buffer( + update_uncommitted_diff_for_buffer( + cx.entity(), project, [buffer.clone()], self.buffer.clone(), @@ -17896,6 +17901,32 @@ impl Editor { }; } + pub fn start_temporary_diff_override(&mut self) { + self.load_diff_task.take(); + self.temporary_diff_override = true; + } + + pub fn end_temporary_diff_override(&mut self, cx: &mut Context) { + self.temporary_diff_override = false; + self.set_render_diff_hunk_controls(Arc::new(render_diff_hunk_controls), cx); + self.buffer.update(cx, |buffer, cx| { + buffer.set_all_diff_hunks_collapsed(cx); + }); + + if let Some(project) = self.project.clone() { + self.load_diff_task = Some( + update_uncommitted_diff_for_buffer( + cx.entity(), + &project, + self.buffer.read(cx).all_buffers(), + self.buffer.clone(), + cx, + ) + .shared(), + ); + } + } + fn on_display_map_changed( &mut self, _: Entity, @@ -18875,7 +18906,8 @@ fn insert_extra_newline_tree_sitter(buffer: &MultiBufferSnapshot, range: Range, project: &Entity, buffers: impl IntoIterator>, buffer: Entity, @@ -18891,6 +18923,13 @@ fn get_uncommitted_diff_for_buffer( }); cx.spawn(async move |cx| { let diffs = future::join_all(tasks).await; + if editor + .read_with(cx, |editor, _cx| editor.temporary_diff_override) + .unwrap_or(false) + { + return; + } + buffer .update(cx, |buffer, cx| { for diff in diffs.into_iter().flatten() { diff --git a/crates/eval/src/example.rs b/crates/eval/src/example.rs index 098964b2b7..9ca2bb10bb 100644 --- a/crates/eval/src/example.rs +++ b/crates/eval/src/example.rs @@ -234,9 +234,11 @@ impl ExampleContext { tx.try_send(Err(anyhow!(err.clone()))).ok(); } }, - ThreadEvent::StreamedAssistantText(_, _) + ThreadEvent::NewRequest + | ThreadEvent::StreamedAssistantText(_, _) | ThreadEvent::StreamedAssistantThinking(_, _) - | ThreadEvent::UsePendingTools { .. } => {} + | ThreadEvent::UsePendingTools { .. } + | ThreadEvent::CompletionCanceled => {} ThreadEvent::ToolFinished { tool_use_id, pending_tool_use, diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 06a461bb5b..e5ee1a7844 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -2633,6 +2633,11 @@ impl MultiBuffer { self.snapshot.borrow().all_diff_hunks_expanded } + pub fn set_all_diff_hunks_collapsed(&mut self, cx: &mut Context) { + self.snapshot.borrow_mut().all_diff_hunks_expanded = false; + self.expand_or_collapse_diff_hunks(vec![Anchor::min()..Anchor::max()], false, cx); + } + pub fn has_multiple_hunks(&self, cx: &App) -> bool { self.read(cx) .diff_hunks_in_range(Anchor::min()..Anchor::max()) diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 9818e1a42b..6eccf68693 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -111,6 +111,7 @@ impl Render for Toolbar { }) .border_b_1() .border_color(cx.theme().colors().border_variant) + .relative() .bg(cx.theme().colors().toolbar_background) .when(has_left_items || has_right_items, |this| { this.child(