From 415f384ff3d4d7bb91f2eee89dede46d1324df0c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 14 Jan 2026 12:43:38 -0500 Subject: [PATCH] Diff review comments: store locally and batch submit to agent (#46669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn't support editing or deleting comments yet, but inputs work: Screenshot 2026-01-14 at 11 51 36 AM Screenshot 2026-01-14 at 11 47 51 AM Screenshot 2026-01-14 at 11 47 58 AM ### Features implemented: - **Local comment storage**: Comments are stored per-hunk in the Editor, with chronological ordering - **Expandable comments section**: Shows "N Comments" header that can expand/collapse - **Inline editing**: Click edit to transform a comment row into an editable text field with confirm/cancel buttons - **Delete functionality**: Remove individual comments - **Send Review to Agent**: Toolbar button with badge showing comment count, batch-submits all comments across all files/hunks to the Agent panel - **User avatars**: Shows the current user's avatar (from their Zed account) next to comments, falls back to Person icon when not logged in - **Visual tests**: Added tests for one comment, multiple comments expanded, and comments collapsed Note that this is behind a feature flag, so no release notes yet. Release Notes: - N/A --------- Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> Co-authored-by: David Baldwin --- crates/agent_ui/src/acp/message_editor.rs | 68 +- crates/agent_ui/src/acp/thread_view.rs | 12 + crates/agent_ui/src/agent_panel.rs | 2 +- crates/agent_ui/src/text_thread_editor.rs | 161 ++- crates/editor/src/actions.rs | 38 + crates/editor/src/editor.rs | 1209 ++++++++++++++++++++- crates/editor/src/editor_tests.rs | 739 +++++++++++++ crates/editor/src/element.rs | 33 +- crates/git_ui/src/project_diff.rs | 51 +- crates/zed/src/visual_test_runner.rs | 220 +++- 10 files changed, 2491 insertions(+), 42 deletions(-) diff --git a/crates/agent_ui/src/acp/message_editor.rs b/crates/agent_ui/src/acp/message_editor.rs index ee25811e6c543eb5c135d6b76655df2e7633270f..52c56035294a801272ef19f32426f3b0bdd2a350 100644 --- a/crates/agent_ui/src/acp/message_editor.rs +++ b/crates/agent_ui/src/acp/message_editor.rs @@ -33,7 +33,7 @@ use rope::Point; use settings::Settings; use std::{cell::RefCell, fmt::Write, rc::Rc, sync::Arc}; use theme::ThemeSettings; -use ui::{ContextMenu, prelude::*}; +use ui::{ButtonLike, ButtonStyle, ContextMenu, Disclosure, ElevationIndex, prelude::*}; use util::{ResultExt, debug_panic}; use workspace::{CollaboratorId, Workspace}; use zed_actions::agent::{Chat, PasteRaw}; @@ -803,6 +803,72 @@ impl MessageEditor { .detach(); } + /// Inserts code snippets as creases into the editor. + /// Each tuple contains (code_text, crease_title). + pub fn insert_code_creases( + &mut self, + creases: Vec<(String, String)>, + window: &mut Window, + cx: &mut Context, + ) { + use editor::display_map::{Crease, FoldPlaceholder}; + use multi_buffer::MultiBufferRow; + use rope::Point; + + self.editor.update(cx, |editor, cx| { + editor.insert("\n", window, cx); + for (text, crease_title) in creases { + let point = editor + .selections + .newest::(&editor.display_snapshot(cx)) + .head(); + let start_row = MultiBufferRow(point.row); + + editor.insert(&text, window, cx); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor_before = snapshot.anchor_after(point); + let anchor_after = editor + .selections + .newest_anchor() + .head() + .bias_left(&snapshot); + + editor.insert("\n", window, cx); + + let fold_placeholder = FoldPlaceholder { + render: Arc::new({ + let title = crease_title.clone(); + move |_fold_id, _fold_range, _cx| { + ButtonLike::new("code-crease") + .style(ButtonStyle::Filled) + .layer(ElevationIndex::ElevatedSurface) + .child(Icon::new(IconName::TextSnippet)) + .child(Label::new(title.clone()).single_line()) + .into_any_element() + } + }), + merge_adjacent: false, + ..Default::default() + }; + + let crease = Crease::inline( + anchor_before..anchor_after, + fold_placeholder, + |row, is_folded, fold, _window, _cx| { + Disclosure::new(("code-crease-toggle", row.0 as u64), !is_folded) + .toggle_state(is_folded) + .on_click(move |_e, window, cx| fold(!is_folded, window, cx)) + .into_any_element() + }, + |_, _, _, _| gpui::Empty.into_any(), + ); + editor.insert_creases(vec![crease], cx); + editor.fold_at(start_row, window, cx); + } + }); + } + pub fn insert_selections(&mut self, window: &mut Window, cx: &mut Context) { let editor = self.editor.read(cx); let editor_buffer = editor.buffer().read(cx); diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 43cb0d6390431ad80111124e83e2d51b15f8d437..2cb2ea4606437264b483c5d88ef317c805da75f2 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -6787,6 +6787,18 @@ impl AcpThreadView { }); } + /// Inserts code snippets as creases into the message editor. + pub(crate) fn insert_code_crease( + &self, + creases: Vec<(String, String)>, + window: &mut Window, + cx: &mut Context, + ) { + self.message_editor.update(cx, |message_editor, cx| { + message_editor.insert_code_creases(creases, window, cx); + }); + } + fn render_thread_retry_status_callout( &self, _window: &mut Window, diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 51aa88bf7029e06b82ba59e2f621db268d0b1738..19b7c7d9477ccc4894abfff611afdc1f3a9627d1 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -775,7 +775,7 @@ impl AgentPanel { .unwrap_or(true) } - fn active_thread_view(&self) -> Option<&Entity> { + pub(crate) fn active_thread_view(&self) -> Option<&Entity> { match &self.active_view { ActiveView::ExternalAgentThread { thread_view, .. } => Some(thread_view), ActiveView::TextThread { .. } diff --git a/crates/agent_ui/src/text_thread_editor.rs b/crates/agent_ui/src/text_thread_editor.rs index 3a790dd354afb9ae21cc49687da08256f167b19d..2fc1b4e35d4535b60d1404e74916ce41dc58b589 100644 --- a/crates/agent_ui/src/text_thread_editor.rs +++ b/crates/agent_ui/src/text_thread_editor.rs @@ -1,4 +1,5 @@ use crate::{ + agent_panel::AgentType, language_model_selector::{LanguageModelSelector, language_model_selector}, ui::{BurnModeTooltip, ModelSelectorTooltip}, }; @@ -10,8 +11,8 @@ use client::{proto, zed_urls}; use collections::{BTreeSet, HashMap, HashSet, hash_map}; use editor::{ Anchor, Editor, EditorEvent, MenuEditPredictionsPolicy, MultiBuffer, MultiBufferOffset, - MultiBufferSnapshot, RowExt, ToOffset as _, ToPoint, - actions::{MoveToEndOfLine, Newline, ShowCompletions}, + MultiBufferSnapshot, RowExt, ToOffset as _, ToPoint as _, + actions::{MoveToEndOfLine, Newline, SendReviewToAgent, ShowCompletions}, display_map::{ BlockPlacement, BlockProperties, BlockStyle, Crease, CreaseMetadata, CustomBlockId, FoldId, RenderBlock, ToDisplayPoint, @@ -220,7 +221,8 @@ impl TextThreadEditor { .register_action(TextThreadEditor::quote_selection) .register_action(TextThreadEditor::insert_selection) .register_action(TextThreadEditor::copy_code) - .register_action(TextThreadEditor::handle_insert_dragged_files); + .register_action(TextThreadEditor::handle_insert_dragged_files) + .register_action(TextThreadEditor::handle_send_review_to_agent); }, ) .detach(); @@ -1517,6 +1519,159 @@ impl TextThreadEditor { agent_panel_delegate.quote_selection(workspace, selections, buffer, window, cx); } + /// Handles the SendReviewToAgent action from the ProjectDiff toolbar. + /// Collects ALL stored review comments from ALL hunks and sends them + /// to the Agent panel as creases. + pub fn handle_send_review_to_agent( + workspace: &mut Workspace, + _: &SendReviewToAgent, + window: &mut Window, + cx: &mut Context, + ) { + use editor::{DiffHunkKey, StoredReviewComment}; + use git_ui::project_diff::ProjectDiff; + + // Find the ProjectDiff item + let Some(project_diff) = workspace.items_of_type::(cx).next() else { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "No Project Diff panel found. Open it first to add review comments.", + ), + cx, + ); + return; + }; + + // Get the buffer reference first (before taking comments) + let buffer = project_diff.update(cx, |project_diff, cx| { + project_diff + .editor() + .read(cx) + .primary_editor() + .read(cx) + .buffer() + .clone() + }); + + // Extract all stored comments from all hunks + let all_comments: Vec<(DiffHunkKey, Vec)> = + project_diff.update(cx, |project_diff, cx| { + let editor = project_diff.editor().read(cx).primary_editor().clone(); + editor.update(cx, |editor, cx| editor.take_all_review_comments(cx)) + }); + + // Flatten: we have Vec<(DiffHunkKey, Vec)> + // Convert to Vec for processing + let comments: Vec = all_comments + .into_iter() + .flat_map(|(_, comments)| comments) + .collect(); + + if comments.is_empty() { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "No review comments to send. Add comments using the + button in the diff view.", + ), + cx, + ); + return; + } + + // Get or create the agent panel + let Some(panel) = workspace.panel::(cx) else { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "Agent panel is not available.", + ), + cx, + ); + return; + }; + + // Create a new thread if there isn't an active one (synchronous call) + let has_active_thread = panel.read(cx).active_thread_view().is_some(); + if !has_active_thread { + panel.update(cx, |panel, cx| { + panel.new_agent_thread(AgentType::NativeAgent, window, cx); + }); + } + + // Focus the agent panel + workspace.focus_panel::(window, cx); + + // Defer inserting creases until after the current update cycle completes, + // allowing the newly created thread (if any) to fully initialize. + cx.defer_in(window, move |workspace, window, cx| { + let Some(panel) = workspace.panel::(cx) else { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "Agent panel closed unexpectedly.", + ), + cx, + ); + return; + }; + + let thread_view = panel.read(cx).active_thread_view().cloned(); + let Some(thread_view) = thread_view else { + workspace.show_toast( + Toast::new( + NotificationId::unique::(), + "No active thread view available after creating thread.", + ), + cx, + ); + return; + }; + + // Build creases for all comments, grouping by code snippet + // so each snippet appears once with all its comments + let snapshot = buffer.read(cx).snapshot(cx); + + // Group comments by their point range (code snippet) + let mut comments_by_range: std::collections::BTreeMap< + (rope::Point, rope::Point), + Vec, + > = std::collections::BTreeMap::new(); + + for comment in comments { + let start = comment.anchor_range.start.to_point(&snapshot); + let end = comment.anchor_range.end.to_point(&snapshot); + comments_by_range + .entry((start, end)) + .or_default() + .push(comment.comment); + } + + // Build one crease per unique code snippet with all its comments + let mut all_creases = Vec::new(); + for ((start, end), comment_texts) in comments_by_range { + let point_range = start..end; + + let mut creases = + selections_creases(vec![point_range.clone()], snapshot.clone(), cx); + + // Append all comments after the code snippet + for (code_text, crease_title) in &mut creases { + let comments_section = comment_texts.join("\n\n"); + *code_text = format!("{}\n\n{}", code_text, comments_section); + *crease_title = format!("Review: {}", crease_title); + } + + all_creases.extend(creases); + } + + // Insert all creases into the message editor + thread_view.update(cx, |thread_view, cx| { + thread_view.insert_code_crease(all_creases, window, cx); + }); + }); + } + pub fn quote_ranges( &mut self, ranges: Vec>, diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index d17c12ad729a4c07e2ca345870f0f93b3e008617..3c99feb18359750ef9445071f872efe49304cbfd 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -844,6 +844,12 @@ actions!( /// Toggles diff display for selected hunks. #[action(deprecated_aliases = ["editor::ToggleHunkDiff"])] ToggleSelectedDiffHunks, + /// Stores the diff review comment locally (for later batch submission). + SubmitDiffReviewComment, + /// Toggles the expanded state of the comments section in the overlay. + ToggleReviewCommentsExpanded, + /// Sends all stored review comments to the Agent panel. + SendReviewToAgent, /// Toggles the selection menu. ToggleSelectionMenu, /// Toggles soft wrap mode. @@ -890,3 +896,35 @@ impl Default for FindAllReferences { } } } + +/// Edits a stored review comment inline. +#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)] +#[action(namespace = editor)] +#[serde(deny_unknown_fields)] +pub struct EditReviewComment { + pub id: usize, +} + +/// Deletes a stored review comment. +#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)] +#[action(namespace = editor)] +#[serde(deny_unknown_fields)] +pub struct DeleteReviewComment { + pub id: usize, +} + +/// Confirms an inline edit of a review comment. +#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)] +#[action(namespace = editor)] +#[serde(deny_unknown_fields)] +pub struct ConfirmEditReviewComment { + pub id: usize, +} + +/// Cancels an inline edit of a review comment. +#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)] +#[action(namespace = editor)] +#[serde(deny_unknown_fields)] +pub struct CancelEditReviewComment { + pub id: usize, +} diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 19c6ca50b364531602b328e76fd00d7f76e1d75f..d076f1d2ae59762da886e97012105ced90f5a19e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -108,10 +108,10 @@ use gpui::{ DispatchPhase, Edges, Entity, EntityInputHandler, EventEmitter, FocusHandle, FocusOutEvent, Focusable, FontId, FontWeight, Global, HighlightStyle, Hsla, KeyContext, Modifiers, MouseButton, MouseDownEvent, MouseMoveEvent, PaintQuad, ParentElement, Pixels, PressureStage, - Render, ScrollHandle, SharedString, Size, Stateful, Styled, Subscription, Task, TextRun, - TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle, UniformListScrollHandle, - WeakEntity, WeakFocusHandle, Window, div, point, prelude::*, pulsating_between, px, relative, - size, + Render, ScrollHandle, SharedString, SharedUri, Size, Stateful, Styled, Subscription, Task, + TextRun, TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle, + UniformListScrollHandle, WeakEntity, WeakFocusHandle, Window, div, point, prelude::*, + pulsating_between, px, relative, size, }; use hover_links::{HoverLink, HoveredLinkState, find_file}; use hover_popover::{HoverState, hide_hover}; @@ -198,8 +198,8 @@ use theme::{ observe_buffer_font_size_adjustment, }; use ui::{ - ButtonSize, ButtonStyle, ContextMenu, Disclosure, IconButton, IconButtonShape, IconName, - IconSize, Indicator, Key, Tooltip, h_flex, prelude::*, scrollbars::ScrollbarAutoHide, + Avatar, ButtonSize, ButtonStyle, ContextMenu, Disclosure, IconButton, IconButtonShape, + IconName, IconSize, Indicator, Key, Tooltip, h_flex, prelude::*, scrollbars::ScrollbarAutoHide, }; use util::{RangeExt, ResultExt, TryFutureExt, maybe, post_inc}; use workspace::{ @@ -1031,6 +1031,75 @@ pub(crate) struct PhantomDiffReviewIndicator { pub is_active: bool, } +/// Identifies a specific hunk in the diff buffer. +/// Used as a key to group comments by their location. +#[derive(Clone, Debug)] +pub struct DiffHunkKey { + /// The file path (relative to worktree) this hunk belongs to. + pub file_path: Arc, + /// An anchor at the start of the hunk. This tracks position as the buffer changes. + pub hunk_start_anchor: Anchor, +} + +/// A review comment stored locally before being sent to the Agent panel. +#[derive(Clone)] +pub struct StoredReviewComment { + /// Unique identifier for this comment (for edit/delete operations). + pub id: usize, + /// The comment text entered by the user. + pub comment: String, + /// The display row where this comment was added (within the hunk). + pub display_row: DisplayRow, + /// Anchors for the code range being reviewed. + pub anchor_range: Range, + /// Timestamp when the comment was created (for chronological ordering). + pub created_at: Instant, + /// Whether this comment is currently being edited inline. + pub is_editing: bool, +} + +impl StoredReviewComment { + pub fn new( + id: usize, + comment: String, + display_row: DisplayRow, + anchor_range: Range, + ) -> Self { + Self { + id, + comment, + display_row, + anchor_range, + created_at: Instant::now(), + is_editing: false, + } + } +} + +/// Represents an active diff review overlay that appears when clicking the "Add Review" button. +pub(crate) struct DiffReviewOverlay { + /// The display row where the overlay is anchored. + pub display_row: DisplayRow, + /// The block ID for the overlay. + pub block_id: CustomBlockId, + /// The editor entity for the review input. + pub prompt_editor: Entity, + /// The hunk key this overlay belongs to. + pub hunk_key: DiffHunkKey, + /// Whether the comments section is expanded. + pub comments_expanded: bool, + /// Editors for comments currently being edited inline. + /// Key: comment ID, Value: Editor entity for inline editing. + pub inline_edit_editors: HashMap>, + /// Subscriptions for inline edit editors' action handlers. + /// Key: comment ID, Value: Subscription keeping the Newline action handler alive. + pub inline_edit_subscriptions: HashMap, + /// The current user's avatar URI for display in comment rows. + pub user_avatar_uri: Option, + /// Subscription to keep the action handler alive. + _subscription: Subscription, +} + /// Zed's primary implementation of text input, allowing users to edit a [`MultiBuffer`]. /// /// See the [module level documentation](self) for more information. @@ -1196,6 +1265,15 @@ pub struct Editor { breakpoint_store: Option>, gutter_breakpoint_indicator: (Option, Option>), pub(crate) gutter_diff_review_indicator: (Option, Option>), + /// Active diff review overlays. Multiple overlays can be open simultaneously + /// when hunks have comments stored. + pub(crate) diff_review_overlays: Vec, + /// Stored review comments grouped by hunk. + /// Uses a Vec instead of HashMap because DiffHunkKey contains an Anchor + /// which doesn't implement Hash/Eq in a way suitable for HashMap keys. + stored_review_comments: Vec<(DiffHunkKey, Vec)>, + /// Counter for generating unique comment IDs. + next_review_comment_id: usize, hovered_diff_hunk_row: Option, pull_diagnostics_task: Task<()>, pull_diagnostics_background_task: Task<()>, @@ -2370,6 +2448,9 @@ impl Editor { breakpoint_store, gutter_breakpoint_indicator: (None, None), gutter_diff_review_indicator: (None, None), + diff_review_overlays: Vec::new(), + stored_review_comments: Vec::new(), + next_review_comment_id: 0, hovered_diff_hunk_row: None, _subscriptions: (!is_minimap) .then(|| { @@ -4284,6 +4365,10 @@ impl Editor { dismissed |= self.mouse_context_menu.take().is_some(); dismissed |= is_user_requested && self.discard_edit_prediction(true, cx); dismissed |= self.snippet_stack.pop().is_some(); + if !self.diff_review_overlays.is_empty() { + self.dismiss_all_diff_review_overlays(cx); + dismissed = true; + } if self.mode.is_full() && matches!(self.active_diagnostics, ActiveDiagnostic::Group(_)) { self.dismiss_diagnostics(cx); @@ -20898,6 +20983,1110 @@ impl Editor { self.show_diff_review_button } + pub fn render_diff_review_button( + &self, + display_row: DisplayRow, + width: Pixels, + cx: &mut Context, + ) -> impl IntoElement { + let text_color = cx.theme().colors().text; + let icon_color = cx.theme().colors().icon_accent; + + h_flex() + .id("diff_review_button") + .cursor_pointer() + .w(width - px(1.)) + .h(relative(0.9)) + .justify_center() + .rounded_sm() + .border_1() + .border_color(text_color.opacity(0.1)) + .bg(text_color.opacity(0.15)) + .hover(|s| { + s.bg(icon_color.opacity(0.4)) + .border_color(icon_color.opacity(0.5)) + }) + .child(Icon::new(IconName::Plus).size(IconSize::Small)) + .tooltip(Tooltip::text("Add Review")) + .on_click(cx.listener(move |editor, _event: &ClickEvent, window, cx| { + editor.show_diff_review_overlay(display_row, window, cx); + })) + } + + /// Calculates the appropriate block height for the diff review overlay. + /// Height is in lines: 2 for input row, 1 for header when comments exist, + /// and 2 lines per comment when expanded. + fn calculate_overlay_height( + &self, + hunk_key: &DiffHunkKey, + comments_expanded: bool, + snapshot: &MultiBufferSnapshot, + ) -> u32 { + let comment_count = self.hunk_comment_count(hunk_key, snapshot); + let base_height: u32 = 2; // Input row with avatar and buttons + + if comment_count == 0 { + base_height + } else if comments_expanded { + // Header (1 line) + 2 lines per comment + base_height + 1 + (comment_count as u32 * 2) + } else { + // Just header when collapsed + base_height + 1 + } + } + + pub fn show_diff_review_overlay( + &mut self, + display_row: DisplayRow, + window: &mut Window, + cx: &mut Context, + ) { + // Check if there's already an overlay for the same hunk - if so, just focus it + let buffer_snapshot = self.buffer.read(cx).snapshot(cx); + let editor_snapshot = self.snapshot(window, cx); + let display_point = DisplayPoint::new(display_row, 0); + let buffer_point = editor_snapshot + .display_snapshot + .display_point_to_point(display_point, Bias::Left); + + // Compute the hunk key for this display row + let file_path = buffer_snapshot + .file_at(Point::new(buffer_point.row, 0)) + .map(|file: &Arc| file.path().clone()) + .unwrap_or_else(|| Arc::from(util::rel_path::RelPath::empty())); + let hunk_start_anchor = buffer_snapshot.anchor_before(Point::new(buffer_point.row, 0)); + let new_hunk_key = DiffHunkKey { + file_path, + hunk_start_anchor, + }; + + // Check if we already have an overlay for this hunk + if let Some(existing_overlay) = self.diff_review_overlays.iter().find(|overlay| { + Self::hunk_keys_match(&overlay.hunk_key, &new_hunk_key, &buffer_snapshot) + }) { + // Just focus the existing overlay's prompt editor + let focus_handle = existing_overlay.prompt_editor.focus_handle(cx); + window.focus(&focus_handle, cx); + return; + } + + // Dismiss overlays that have no comments for their hunks + self.dismiss_overlays_without_comments(cx); + + // Get the current user's avatar URI from the project's user_store + let user_avatar_uri = self.project.as_ref().and_then(|project| { + let user_store = project.read(cx).user_store(); + user_store + .read(cx) + .current_user() + .map(|user| user.avatar_uri.clone()) + }); + + // Create anchor at the end of the row so the block appears immediately below it + let line_len = buffer_snapshot.line_len(MultiBufferRow(buffer_point.row)); + let anchor = buffer_snapshot.anchor_after(Point::new(buffer_point.row, line_len)); + + // Use the hunk key we already computed + let hunk_key = new_hunk_key; + + // Create the prompt editor for the review input + let prompt_editor = cx.new(|cx| { + let mut editor = Editor::single_line(window, cx); + editor.set_placeholder_text("Add a review comment...", window, cx); + editor + }); + + // Register the Newline action on the prompt editor to submit the review + let parent_editor = cx.entity().downgrade(); + let subscription = prompt_editor.update(cx, |prompt_editor, _cx| { + prompt_editor.register_action({ + let parent_editor = parent_editor.clone(); + move |_: &crate::actions::Newline, window, cx| { + if let Some(editor) = parent_editor.upgrade() { + editor.update(cx, |editor, cx| { + editor.submit_diff_review_comment(window, cx); + }); + } + } + }) + }); + + // Calculate initial height based on existing comments for this hunk + let initial_height = self.calculate_overlay_height(&hunk_key, true, &buffer_snapshot); + + // Create the overlay block + let prompt_editor_for_render = prompt_editor.clone(); + let hunk_key_for_render = hunk_key.clone(); + let editor_handle = cx.entity().downgrade(); + let block = BlockProperties { + style: BlockStyle::Sticky, + placement: BlockPlacement::Below(anchor), + height: Some(initial_height), + render: Arc::new(move |cx| { + Self::render_diff_review_overlay( + &prompt_editor_for_render, + &hunk_key_for_render, + &editor_handle, + cx, + ) + }), + priority: 0, + }; + + let block_ids = self.insert_blocks([block], None, cx); + let Some(block_id) = block_ids.into_iter().next() else { + log::error!("Failed to insert diff review overlay block"); + return; + }; + + self.diff_review_overlays.push(DiffReviewOverlay { + display_row, + block_id, + prompt_editor: prompt_editor.clone(), + hunk_key, + comments_expanded: true, + inline_edit_editors: HashMap::default(), + inline_edit_subscriptions: HashMap::default(), + user_avatar_uri, + _subscription: subscription, + }); + + // Focus the prompt editor + let focus_handle = prompt_editor.focus_handle(cx); + window.focus(&focus_handle, cx); + + cx.notify(); + } + + /// Dismisses all diff review overlays. + pub fn dismiss_all_diff_review_overlays(&mut self, cx: &mut Context) { + if self.diff_review_overlays.is_empty() { + return; + } + let block_ids: HashSet<_> = self + .diff_review_overlays + .drain(..) + .map(|overlay| overlay.block_id) + .collect(); + self.remove_blocks(block_ids, None, cx); + cx.notify(); + } + + /// Dismisses overlays that have no comments stored for their hunks. + /// Keeps overlays that have at least one comment. + fn dismiss_overlays_without_comments(&mut self, cx: &mut Context) { + let snapshot = self.buffer.read(cx).snapshot(cx); + + // First, compute which overlays have comments (to avoid borrow issues with retain) + let overlays_with_comments: Vec = self + .diff_review_overlays + .iter() + .map(|overlay| self.hunk_comment_count(&overlay.hunk_key, &snapshot) > 0) + .collect(); + + // Now collect block IDs to remove and retain overlays + let mut block_ids_to_remove = HashSet::default(); + let mut index = 0; + self.diff_review_overlays.retain(|overlay| { + let has_comments = overlays_with_comments[index]; + index += 1; + if !has_comments { + block_ids_to_remove.insert(overlay.block_id); + } + has_comments + }); + + if !block_ids_to_remove.is_empty() { + self.remove_blocks(block_ids_to_remove, None, cx); + cx.notify(); + } + } + + /// Refreshes the diff review overlay block to update its height and render function. + /// Uses resize_blocks and replace_blocks to avoid visual flicker from remove+insert. + fn refresh_diff_review_overlay_height( + &mut self, + hunk_key: &DiffHunkKey, + _window: &mut Window, + cx: &mut Context, + ) { + // Extract all needed data from overlay first to avoid borrow conflicts + let snapshot = self.buffer.read(cx).snapshot(cx); + let (comments_expanded, block_id, prompt_editor) = { + let Some(overlay) = self + .diff_review_overlays + .iter() + .find(|overlay| Self::hunk_keys_match(&overlay.hunk_key, hunk_key, &snapshot)) + else { + return; + }; + + ( + overlay.comments_expanded, + overlay.block_id, + overlay.prompt_editor.clone(), + ) + }; + + // Calculate new height + let snapshot = self.buffer.read(cx).snapshot(cx); + let new_height = self.calculate_overlay_height(hunk_key, comments_expanded, &snapshot); + + // Update the block height using resize_blocks (avoids flicker) + let mut heights = HashMap::default(); + heights.insert(block_id, new_height); + self.resize_blocks(heights, None, cx); + + // Update the render function using replace_blocks (avoids flicker) + let hunk_key_for_render = hunk_key.clone(); + let editor_handle = cx.entity().downgrade(); + let render: Arc AnyElement + Send + Sync> = + Arc::new(move |cx| { + Self::render_diff_review_overlay( + &prompt_editor, + &hunk_key_for_render, + &editor_handle, + cx, + ) + }); + + let mut renderers = HashMap::default(); + renderers.insert(block_id, render); + self.replace_blocks(renderers, None, cx); + } + + /// Action handler for SubmitDiffReviewComment. + pub fn submit_diff_review_comment_action( + &mut self, + _: &SubmitDiffReviewComment, + window: &mut Window, + cx: &mut Context, + ) { + self.submit_diff_review_comment(window, cx); + } + + /// Stores the diff review comment locally. + /// Comments are stored per-hunk and can later be batch-submitted to the Agent panel. + pub fn submit_diff_review_comment(&mut self, window: &mut Window, cx: &mut Context) { + // Find the overlay that currently has focus + let overlay_index = self + .diff_review_overlays + .iter() + .position(|overlay| overlay.prompt_editor.focus_handle(cx).is_focused(window)); + let Some(overlay_index) = overlay_index else { + return; + }; + let overlay = &self.diff_review_overlays[overlay_index]; + + // Get the comment text from the prompt editor + let comment_text = overlay.prompt_editor.read(cx).text(cx).trim().to_string(); + + // Don't submit if the comment is empty + if comment_text.is_empty() { + return; + } + + // Get the display row and hunk key + let display_row = overlay.display_row; + let hunk_key = overlay.hunk_key.clone(); + + // Convert to buffer position for anchors + let snapshot = self.snapshot(window, cx); + let display_point = DisplayPoint::new(display_row, 0); + let buffer_point = snapshot + .display_snapshot + .display_point_to_point(display_point, Bias::Left); + + // Get the line range + let buffer_snapshot = self.buffer.read(cx).snapshot(cx); + let line_start = Point::new(buffer_point.row, 0); + let line_end = Point::new( + buffer_point.row, + buffer_snapshot.line_len(MultiBufferRow(buffer_point.row)), + ); + + // Create anchors for the selection + let anchor_start = buffer_snapshot.anchor_after(line_start); + let anchor_end = buffer_snapshot.anchor_before(line_end); + + // Store the comment locally + self.add_review_comment( + hunk_key.clone(), + comment_text, + display_row, + anchor_start..anchor_end, + cx, + ); + + // Clear the prompt editor but keep the overlay open + if let Some(overlay) = self.diff_review_overlays.get(overlay_index) { + overlay.prompt_editor.update(cx, |editor, cx| { + editor.clear(window, cx); + }); + } + + // Refresh the overlay to update the block height for the new comment + self.refresh_diff_review_overlay_height(&hunk_key, window, cx); + + cx.notify(); + } + + /// Returns the prompt editor for the diff review overlay, if one is active. + /// This is primarily used for testing. + pub fn diff_review_prompt_editor(&self) -> Option<&Entity> { + self.diff_review_overlays + .first() + .map(|overlay| &overlay.prompt_editor) + } + + /// Returns the display row for the first diff review overlay, if one is active. + pub fn diff_review_display_row(&self) -> Option { + self.diff_review_overlays + .first() + .map(|overlay| overlay.display_row) + } + + /// Sets whether the comments section is expanded in the diff review overlay. + /// This is primarily used for testing. + pub fn set_diff_review_comments_expanded(&mut self, expanded: bool, cx: &mut Context) { + for overlay in &mut self.diff_review_overlays { + overlay.comments_expanded = expanded; + } + cx.notify(); + } + + /// Compares two DiffHunkKeys for equality by resolving their anchors. + fn hunk_keys_match(a: &DiffHunkKey, b: &DiffHunkKey, snapshot: &MultiBufferSnapshot) -> bool { + a.file_path == b.file_path + && a.hunk_start_anchor.to_point(snapshot) == b.hunk_start_anchor.to_point(snapshot) + } + + /// Returns comments for a specific hunk, ordered by creation time. + pub fn comments_for_hunk<'a>( + &'a self, + key: &DiffHunkKey, + snapshot: &MultiBufferSnapshot, + ) -> &'a [StoredReviewComment] { + let key_point = key.hunk_start_anchor.to_point(snapshot); + self.stored_review_comments + .iter() + .find(|(k, _)| { + k.file_path == key.file_path && k.hunk_start_anchor.to_point(snapshot) == key_point + }) + .map(|(_, comments)| comments.as_slice()) + .unwrap_or(&[]) + } + + /// Returns the total count of stored review comments across all hunks. + pub fn total_review_comment_count(&self) -> usize { + self.stored_review_comments + .iter() + .map(|(_, v)| v.len()) + .sum() + } + + /// Returns the count of comments for a specific hunk. + pub fn hunk_comment_count(&self, key: &DiffHunkKey, snapshot: &MultiBufferSnapshot) -> usize { + let key_point = key.hunk_start_anchor.to_point(snapshot); + self.stored_review_comments + .iter() + .find(|(k, _)| { + k.file_path == key.file_path && k.hunk_start_anchor.to_point(snapshot) == key_point + }) + .map(|(_, v)| v.len()) + .unwrap_or(0) + } + + /// Adds a new review comment to a specific hunk. + pub fn add_review_comment( + &mut self, + hunk_key: DiffHunkKey, + comment: String, + display_row: DisplayRow, + anchor_range: Range, + cx: &mut Context, + ) -> usize { + let id = self.next_review_comment_id; + self.next_review_comment_id += 1; + + let stored_comment = StoredReviewComment::new(id, comment, display_row, anchor_range); + + let snapshot = self.buffer.read(cx).snapshot(cx); + let key_point = hunk_key.hunk_start_anchor.to_point(&snapshot); + + // Find existing entry for this hunk or add a new one + if let Some((_, comments)) = self.stored_review_comments.iter_mut().find(|(k, _)| { + k.file_path == hunk_key.file_path + && k.hunk_start_anchor.to_point(&snapshot) == key_point + }) { + comments.push(stored_comment); + } else { + self.stored_review_comments + .push((hunk_key, vec![stored_comment])); + } + + cx.emit(EditorEvent::ReviewCommentsChanged { + total_count: self.total_review_comment_count(), + }); + cx.notify(); + id + } + + /// Removes a review comment by ID from any hunk. + pub fn remove_review_comment(&mut self, id: usize, cx: &mut Context) -> bool { + for (_, comments) in self.stored_review_comments.iter_mut() { + if let Some(index) = comments.iter().position(|c| c.id == id) { + comments.remove(index); + cx.emit(EditorEvent::ReviewCommentsChanged { + total_count: self.total_review_comment_count(), + }); + cx.notify(); + return true; + } + } + false + } + + /// Updates a review comment's text by ID. + pub fn update_review_comment( + &mut self, + id: usize, + new_comment: String, + cx: &mut Context, + ) -> bool { + for (_, comments) in self.stored_review_comments.iter_mut() { + if let Some(comment) = comments.iter_mut().find(|c| c.id == id) { + comment.comment = new_comment; + comment.is_editing = false; + cx.emit(EditorEvent::ReviewCommentsChanged { + total_count: self.total_review_comment_count(), + }); + cx.notify(); + return true; + } + } + false + } + + /// Sets a comment's editing state. + pub fn set_comment_editing(&mut self, id: usize, is_editing: bool, cx: &mut Context) { + for (_, comments) in self.stored_review_comments.iter_mut() { + if let Some(comment) = comments.iter_mut().find(|c| c.id == id) { + comment.is_editing = is_editing; + cx.notify(); + return; + } + } + } + + /// Takes all stored comments from all hunks, clearing the storage. + /// Returns a Vec of (hunk_key, comments) pairs. + pub fn take_all_review_comments( + &mut self, + cx: &mut Context, + ) -> Vec<(DiffHunkKey, Vec)> { + // Dismiss all overlays when taking comments (e.g., when sending to agent) + self.dismiss_all_diff_review_overlays(cx); + let comments = std::mem::take(&mut self.stored_review_comments); + // Reset the ID counter since all comments have been taken + self.next_review_comment_id = 0; + cx.emit(EditorEvent::ReviewCommentsChanged { total_count: 0 }); + cx.notify(); + comments + } + + /// Removes review comments whose anchors are no longer valid or whose + /// associated diff hunks no longer exist. + /// + /// This should be called when the buffer changes to prevent orphaned comments + /// from accumulating. + pub fn cleanup_orphaned_review_comments(&mut self, cx: &mut Context) { + let snapshot = self.buffer.read(cx).snapshot(cx); + let original_count = self.total_review_comment_count(); + + // Remove comments with invalid hunk anchors + self.stored_review_comments + .retain(|(hunk_key, _)| hunk_key.hunk_start_anchor.is_valid(&snapshot)); + + // Also clean up individual comments with invalid anchor ranges + for (_, comments) in &mut self.stored_review_comments { + comments.retain(|comment| { + comment.anchor_range.start.is_valid(&snapshot) + && comment.anchor_range.end.is_valid(&snapshot) + }); + } + + // Remove empty hunk entries + self.stored_review_comments + .retain(|(_, comments)| !comments.is_empty()); + + let new_count = self.total_review_comment_count(); + if new_count != original_count { + cx.emit(EditorEvent::ReviewCommentsChanged { + total_count: new_count, + }); + cx.notify(); + } + } + + /// Toggles the expanded state of the comments section in the overlay. + pub fn toggle_review_comments_expanded( + &mut self, + _: &ToggleReviewCommentsExpanded, + window: &mut Window, + cx: &mut Context, + ) { + // Find the overlay that currently has focus, or use the first one + let overlay_info = self.diff_review_overlays.iter_mut().find_map(|overlay| { + if overlay.prompt_editor.focus_handle(cx).is_focused(window) { + overlay.comments_expanded = !overlay.comments_expanded; + Some(overlay.hunk_key.clone()) + } else { + None + } + }); + + // If no focused overlay found, toggle the first one + let hunk_key = overlay_info.or_else(|| { + self.diff_review_overlays.first_mut().map(|overlay| { + overlay.comments_expanded = !overlay.comments_expanded; + overlay.hunk_key.clone() + }) + }); + + if let Some(hunk_key) = hunk_key { + self.refresh_diff_review_overlay_height(&hunk_key, window, cx); + cx.notify(); + } + } + + /// Handles the EditReviewComment action - sets a comment into editing mode. + pub fn edit_review_comment( + &mut self, + action: &EditReviewComment, + window: &mut Window, + cx: &mut Context, + ) { + let comment_id = action.id; + + // Set the comment to editing mode + self.set_comment_editing(comment_id, true, cx); + + // Find the overlay that contains this comment and create an inline editor if needed + // First, find which hunk this comment belongs to + let hunk_key = self + .stored_review_comments + .iter() + .find_map(|(key, comments)| { + if comments.iter().any(|c| c.id == comment_id) { + Some(key.clone()) + } else { + None + } + }); + + let snapshot = self.buffer.read(cx).snapshot(cx); + if let Some(hunk_key) = hunk_key { + if let Some(overlay) = self + .diff_review_overlays + .iter_mut() + .find(|overlay| Self::hunk_keys_match(&overlay.hunk_key, &hunk_key, &snapshot)) + { + if let std::collections::hash_map::Entry::Vacant(entry) = + overlay.inline_edit_editors.entry(comment_id) + { + // Find the comment text + let comment_text = self + .stored_review_comments + .iter() + .flat_map(|(_, comments)| comments) + .find(|c| c.id == comment_id) + .map(|c| c.comment.clone()) + .unwrap_or_default(); + + // Create inline editor + let parent_editor = cx.entity().downgrade(); + let inline_editor = cx.new(|cx| { + let mut editor = Editor::single_line(window, cx); + editor.set_text(&*comment_text, window, cx); + // Select all text for easy replacement + editor.select_all(&crate::actions::SelectAll, window, cx); + editor + }); + + // Register the Newline action to confirm the edit + let subscription = inline_editor.update(cx, |inline_editor, _cx| { + inline_editor.register_action({ + let parent_editor = parent_editor.clone(); + move |_: &crate::actions::Newline, window, cx| { + if let Some(editor) = parent_editor.upgrade() { + editor.update(cx, |editor, cx| { + editor.confirm_edit_review_comment(comment_id, window, cx); + }); + } + } + }) + }); + + // Store the subscription to keep the action handler alive + overlay + .inline_edit_subscriptions + .insert(comment_id, subscription); + + // Focus the inline editor + let focus_handle = inline_editor.focus_handle(cx); + window.focus(&focus_handle, cx); + + entry.insert(inline_editor); + } + } + } + + cx.notify(); + } + + /// Confirms an inline edit of a review comment. + pub fn confirm_edit_review_comment( + &mut self, + comment_id: usize, + _window: &mut Window, + cx: &mut Context, + ) { + // Get the new text from the inline editor + // Find the overlay containing this comment's inline editor + let snapshot = self.buffer.read(cx).snapshot(cx); + let hunk_key = self + .stored_review_comments + .iter() + .find_map(|(key, comments)| { + if comments.iter().any(|c| c.id == comment_id) { + Some(key.clone()) + } else { + None + } + }); + + let new_text = hunk_key + .as_ref() + .and_then(|hunk_key| { + self.diff_review_overlays + .iter() + .find(|overlay| Self::hunk_keys_match(&overlay.hunk_key, hunk_key, &snapshot)) + }) + .as_ref() + .and_then(|overlay| overlay.inline_edit_editors.get(&comment_id)) + .map(|editor| editor.read(cx).text(cx).trim().to_string()); + + if let Some(new_text) = new_text { + if !new_text.is_empty() { + self.update_review_comment(comment_id, new_text, cx); + } + } + + // Remove the inline editor and its subscription + if let Some(hunk_key) = hunk_key { + if let Some(overlay) = self + .diff_review_overlays + .iter_mut() + .find(|overlay| Self::hunk_keys_match(&overlay.hunk_key, &hunk_key, &snapshot)) + { + overlay.inline_edit_editors.remove(&comment_id); + overlay.inline_edit_subscriptions.remove(&comment_id); + } + } + + // Clear editing state + self.set_comment_editing(comment_id, false, cx); + } + + /// Cancels an inline edit of a review comment. + pub fn cancel_edit_review_comment( + &mut self, + comment_id: usize, + _window: &mut Window, + cx: &mut Context, + ) { + // Find which hunk this comment belongs to + let hunk_key = self + .stored_review_comments + .iter() + .find_map(|(key, comments)| { + if comments.iter().any(|c| c.id == comment_id) { + Some(key.clone()) + } else { + None + } + }); + + // Remove the inline editor and its subscription + if let Some(hunk_key) = hunk_key { + let snapshot = self.buffer.read(cx).snapshot(cx); + if let Some(overlay) = self + .diff_review_overlays + .iter_mut() + .find(|overlay| Self::hunk_keys_match(&overlay.hunk_key, &hunk_key, &snapshot)) + { + overlay.inline_edit_editors.remove(&comment_id); + overlay.inline_edit_subscriptions.remove(&comment_id); + } + } + + // Clear editing state + self.set_comment_editing(comment_id, false, cx); + } + + /// Action handler for ConfirmEditReviewComment. + pub fn confirm_edit_review_comment_action( + &mut self, + action: &ConfirmEditReviewComment, + window: &mut Window, + cx: &mut Context, + ) { + self.confirm_edit_review_comment(action.id, window, cx); + } + + /// Action handler for CancelEditReviewComment. + pub fn cancel_edit_review_comment_action( + &mut self, + action: &CancelEditReviewComment, + window: &mut Window, + cx: &mut Context, + ) { + self.cancel_edit_review_comment(action.id, window, cx); + } + + /// Handles the DeleteReviewComment action - removes a comment. + pub fn delete_review_comment( + &mut self, + action: &DeleteReviewComment, + window: &mut Window, + cx: &mut Context, + ) { + // Get the hunk key before removing the comment + // Find the hunk key from the comment itself + let comment_id = action.id; + let hunk_key = self + .stored_review_comments + .iter() + .find_map(|(key, comments)| { + if comments.iter().any(|c| c.id == comment_id) { + Some(key.clone()) + } else { + None + } + }); + + // Also get it from the overlay for refresh purposes + let overlay_hunk_key = self + .diff_review_overlays + .first() + .map(|o| o.hunk_key.clone()); + + self.remove_review_comment(action.id, cx); + + // Refresh the overlay height after removing a comment + if let Some(hunk_key) = hunk_key.or(overlay_hunk_key) { + self.refresh_diff_review_overlay_height(&hunk_key, window, cx); + } + } + + fn render_diff_review_overlay( + prompt_editor: &Entity, + hunk_key: &DiffHunkKey, + editor_handle: &WeakEntity, + cx: &mut BlockContext, + ) -> AnyElement { + let theme = cx.theme(); + let colors = theme.colors(); + + // Get stored comments, expanded state, inline editors, and user avatar from the editor + let (comments, comments_expanded, inline_editors, user_avatar_uri) = editor_handle + .upgrade() + .map(|editor| { + let editor = editor.read(cx); + let snapshot = editor.buffer().read(cx).snapshot(cx); + let comments = editor.comments_for_hunk(hunk_key, &snapshot).to_vec(); + let snapshot = editor.buffer.read(cx).snapshot(cx); + let (expanded, editors, avatar_uri) = editor + .diff_review_overlays + .iter() + .find(|overlay| Editor::hunk_keys_match(&overlay.hunk_key, hunk_key, &snapshot)) + .as_ref() + .map(|o| { + ( + o.comments_expanded, + o.inline_edit_editors.clone(), + o.user_avatar_uri.clone(), + ) + }) + .unwrap_or((true, HashMap::default(), None)); + (comments, expanded, editors, avatar_uri) + }) + .unwrap_or((Vec::new(), true, HashMap::default(), None)); + + let comment_count = comments.len(); + let avatar_size = px(20.); + let action_icon_size = IconSize::XSmall; + + v_flex() + .w_full() + .bg(colors.editor_background) + .border_b_1() + .border_color(colors.border) + .px_2() + .pb_2() + .gap_2() + // Top row: editable input with user's avatar + .child( + h_flex() + .w_full() + .items_center() + .gap_2() + .px_2() + .py_1p5() + .rounded_md() + .bg(colors.surface_background) + .child( + div() + .size(avatar_size) + .flex_shrink_0() + .rounded_full() + .overflow_hidden() + .child(if let Some(ref avatar_uri) = user_avatar_uri { + Avatar::new(avatar_uri.clone()) + .size(avatar_size) + .into_any_element() + } else { + Icon::new(IconName::Person) + .size(IconSize::Small) + .color(ui::Color::Muted) + .into_any_element() + }), + ) + .child( + div() + .flex_1() + .border_1() + .border_color(colors.border) + .rounded_md() + .bg(colors.editor_background) + .px_2() + .py_1() + .child(prompt_editor.clone()), + ) + .child( + h_flex() + .flex_shrink_0() + .gap_1() + .child( + IconButton::new("diff-review-close", IconName::Close) + .icon_color(ui::Color::Muted) + .icon_size(action_icon_size) + .tooltip(Tooltip::text("Close")) + .on_click(|_, window, cx| { + window + .dispatch_action(Box::new(crate::actions::Cancel), cx); + }), + ) + .child( + IconButton::new("diff-review-add", IconName::Return) + .icon_color(ui::Color::Muted) + .icon_size(action_icon_size) + .tooltip(Tooltip::text("Add comment")) + .on_click(|_, window, cx| { + window.dispatch_action( + Box::new(crate::actions::SubmitDiffReviewComment), + cx, + ); + }), + ), + ), + ) + // Expandable comments section (only shown when there are comments) + .when(comment_count > 0, |el| { + el.child(Self::render_comments_section( + comments, + comments_expanded, + inline_editors, + user_avatar_uri, + avatar_size, + action_icon_size, + colors, + )) + }) + .into_any_element() + } + + fn render_comments_section( + comments: Vec, + expanded: bool, + inline_editors: HashMap>, + user_avatar_uri: Option, + avatar_size: Pixels, + action_icon_size: IconSize, + colors: &theme::ThemeColors, + ) -> impl IntoElement { + let comment_count = comments.len(); + + v_flex() + .w_full() + .gap_1() + // Header with expand/collapse toggle + .child( + h_flex() + .id("review-comments-header") + .w_full() + .items_center() + .gap_1() + .px_2() + .py_1() + .cursor_pointer() + .rounded_md() + .hover(|style| style.bg(colors.ghost_element_hover)) + .on_click(|_, window: &mut Window, cx| { + window.dispatch_action( + Box::new(crate::actions::ToggleReviewCommentsExpanded), + cx, + ); + }) + .child( + Icon::new(if expanded { + IconName::ChevronDown + } else { + IconName::ChevronRight + }) + .size(IconSize::Small) + .color(ui::Color::Muted), + ) + .child( + Label::new(format!( + "{} Comment{}", + comment_count, + if comment_count == 1 { "" } else { "s" } + )) + .size(LabelSize::Small) + .color(Color::Muted), + ), + ) + // Comments list (when expanded) + .when(expanded, |el| { + el.children(comments.into_iter().map(|comment| { + let inline_editor = inline_editors.get(&comment.id).cloned(); + Self::render_comment_row( + comment, + inline_editor, + user_avatar_uri.clone(), + avatar_size, + action_icon_size, + colors, + ) + })) + }) + } + + fn render_comment_row( + comment: StoredReviewComment, + inline_editor: Option>, + user_avatar_uri: Option, + avatar_size: Pixels, + action_icon_size: IconSize, + colors: &theme::ThemeColors, + ) -> impl IntoElement { + let comment_id = comment.id; + let is_editing = inline_editor.is_some(); + + h_flex() + .w_full() + .items_center() + .gap_2() + .px_2() + .py_1p5() + .rounded_md() + .bg(colors.surface_background) + .child( + div() + .size(avatar_size) + .flex_shrink_0() + .rounded_full() + .overflow_hidden() + .child(if let Some(ref avatar_uri) = user_avatar_uri { + Avatar::new(avatar_uri.clone()) + .size(avatar_size) + .into_any_element() + } else { + Icon::new(IconName::Person) + .size(IconSize::Small) + .color(ui::Color::Muted) + .into_any_element() + }), + ) + .child(if let Some(editor) = inline_editor { + // Inline edit mode: show an editable text field + div() + .flex_1() + .border_1() + .border_color(colors.border) + .rounded_md() + .bg(colors.editor_background) + .px_2() + .py_1() + .child(editor) + .into_any_element() + } else { + // Display mode: show the comment text + div() + .flex_1() + .text_sm() + .text_color(colors.text) + .child(comment.comment) + .into_any_element() + }) + .child(if is_editing { + // Editing mode: show close and confirm buttons + h_flex() + .gap_1() + .child( + IconButton::new( + format!("diff-review-cancel-edit-{comment_id}"), + IconName::Close, + ) + .icon_color(ui::Color::Muted) + .icon_size(action_icon_size) + .tooltip(Tooltip::text("Cancel")) + .on_click(move |_, window, cx| { + window.dispatch_action( + Box::new(crate::actions::CancelEditReviewComment { + id: comment_id, + }), + cx, + ); + }), + ) + .child( + IconButton::new( + format!("diff-review-confirm-edit-{comment_id}"), + IconName::Return, + ) + .icon_color(ui::Color::Muted) + .icon_size(action_icon_size) + .tooltip(Tooltip::text("Confirm")) + .on_click(move |_, window, cx| { + window.dispatch_action( + Box::new(crate::actions::ConfirmEditReviewComment { + id: comment_id, + }), + cx, + ); + }), + ) + .into_any_element() + } else { + // Display mode: no action buttons for now (edit/delete not yet implemented) + gpui::Empty.into_any_element() + }) + } + pub fn set_masked(&mut self, masked: bool, cx: &mut Context) { if self.display_map.read(cx).masked != masked { self.display_map.update(cx, |map, _| map.masked = masked); @@ -22237,6 +23426,9 @@ impl Editor { self.update_visible_edit_prediction(window, cx); } + // Clean up orphaned review comments after edits + self.cleanup_orphaned_review_comments(cx); + if let Some(buffer) = edited_buffer { if buffer.read(cx).file().is_none() { cx.emit(EditorEvent::TitleChanged); @@ -25827,6 +27019,11 @@ impl Deref for EditorSnapshot { #[derive(Clone, Debug, PartialEq, Eq)] pub enum EditorEvent { + /// Emitted when the stored review comments change (added, removed, or updated). + ReviewCommentsChanged { + /// The new total count of review comments. + total_count: usize, + }, InputIgnored { text: Arc, }, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 7dc8a20a10191f716eb41f3a7686d1d3eff22871..ad0c1a2db38bdcb91a24955bce83c5eae1432e49 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -30454,6 +30454,745 @@ async fn test_diff_review_button_shown_when_ai_enabled(cx: &mut TestAppContext) }); } +/// Helper function to create a DiffHunkKey for testing. +/// Uses Anchor::min() as a placeholder anchor since these tests don't need +/// real buffer positioning. +fn test_hunk_key(file_path: &str) -> DiffHunkKey { + DiffHunkKey { + file_path: if file_path.is_empty() { + Arc::from(util::rel_path::RelPath::empty()) + } else { + Arc::from(util::rel_path::RelPath::unix(file_path).unwrap()) + }, + hunk_start_anchor: Anchor::min(), + } +} + +/// Helper function to create a DiffHunkKey with a specific anchor for testing. +fn test_hunk_key_with_anchor(file_path: &str, anchor: Anchor) -> DiffHunkKey { + DiffHunkKey { + file_path: if file_path.is_empty() { + Arc::from(util::rel_path::RelPath::empty()) + } else { + Arc::from(util::rel_path::RelPath::unix(file_path).unwrap()) + }, + hunk_start_anchor: anchor, + } +} + +/// Helper function to add a review comment with default anchors for testing. +fn add_test_comment( + editor: &mut Editor, + key: DiffHunkKey, + comment: &str, + display_row: u32, + cx: &mut Context, +) -> usize { + editor.add_review_comment( + key, + comment.to_string(), + DisplayRow(display_row), + Anchor::min()..Anchor::max(), + cx, + ) +} + +#[gpui::test] +fn test_review_comment_add_to_hunk(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor: &mut Editor, _window, cx| { + let key = test_hunk_key(""); + + let id = add_test_comment(editor, key.clone(), "Test comment", 0, cx); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + assert_eq!(editor.total_review_comment_count(), 1); + assert_eq!(editor.hunk_comment_count(&key, &snapshot), 1); + + let comments = editor.comments_for_hunk(&key, &snapshot); + assert_eq!(comments.len(), 1); + assert_eq!(comments[0].comment, "Test comment"); + assert_eq!(comments[0].id, id); + }); +} + +#[gpui::test] +fn test_review_comments_are_per_hunk(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor: &mut Editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor1 = snapshot.anchor_before(Point::new(0, 0)); + let anchor2 = snapshot.anchor_before(Point::new(0, 0)); + let key1 = test_hunk_key_with_anchor("file1.rs", anchor1); + let key2 = test_hunk_key_with_anchor("file2.rs", anchor2); + + add_test_comment(editor, key1.clone(), "Comment for file1", 0, cx); + add_test_comment(editor, key2.clone(), "Comment for file2", 10, cx); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + assert_eq!(editor.total_review_comment_count(), 2); + assert_eq!(editor.hunk_comment_count(&key1, &snapshot), 1); + assert_eq!(editor.hunk_comment_count(&key2, &snapshot), 1); + + assert_eq!( + editor.comments_for_hunk(&key1, &snapshot)[0].comment, + "Comment for file1" + ); + assert_eq!( + editor.comments_for_hunk(&key2, &snapshot)[0].comment, + "Comment for file2" + ); + }); +} + +#[gpui::test] +fn test_review_comment_remove(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor: &mut Editor, _window, cx| { + let key = test_hunk_key(""); + + let id = add_test_comment(editor, key, "To be removed", 0, cx); + + assert_eq!(editor.total_review_comment_count(), 1); + + let removed = editor.remove_review_comment(id, cx); + assert!(removed); + assert_eq!(editor.total_review_comment_count(), 0); + + // Try to remove again + let removed_again = editor.remove_review_comment(id, cx); + assert!(!removed_again); + }); +} + +#[gpui::test] +fn test_review_comment_update(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor: &mut Editor, _window, cx| { + let key = test_hunk_key(""); + + let id = add_test_comment(editor, key.clone(), "Original text", 0, cx); + + let updated = editor.update_review_comment(id, "Updated text".to_string(), cx); + assert!(updated); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + let comments = editor.comments_for_hunk(&key, &snapshot); + assert_eq!(comments[0].comment, "Updated text"); + assert!(!comments[0].is_editing); // Should clear editing flag + }); +} + +#[gpui::test] +fn test_review_comment_take_all(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor: &mut Editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor1 = snapshot.anchor_before(Point::new(0, 0)); + let anchor2 = snapshot.anchor_before(Point::new(0, 0)); + let key1 = test_hunk_key_with_anchor("file1.rs", anchor1); + let key2 = test_hunk_key_with_anchor("file2.rs", anchor2); + + let id1 = add_test_comment(editor, key1.clone(), "Comment 1", 0, cx); + let id2 = add_test_comment(editor, key1.clone(), "Comment 2", 1, cx); + let id3 = add_test_comment(editor, key2.clone(), "Comment 3", 10, cx); + + // IDs should be sequential starting from 0 + assert_eq!(id1, 0); + assert_eq!(id2, 1); + assert_eq!(id3, 2); + + assert_eq!(editor.total_review_comment_count(), 3); + + let taken = editor.take_all_review_comments(cx); + + // Should have 2 entries (one per hunk) + assert_eq!(taken.len(), 2); + + // Total comments should be 3 + let total: usize = taken + .iter() + .map(|(_, comments): &(DiffHunkKey, Vec)| comments.len()) + .sum(); + assert_eq!(total, 3); + + // Storage should be empty + assert_eq!(editor.total_review_comment_count(), 0); + + // After taking all comments, ID counter should reset + // New comments should get IDs starting from 0 again + let new_id1 = add_test_comment(editor, key1, "New Comment 1", 0, cx); + let new_id2 = add_test_comment(editor, key2, "New Comment 2", 0, cx); + + assert_eq!(new_id1, 0, "ID counter should reset after take_all"); + assert_eq!(new_id2, 1, "IDs should be sequential after reset"); + }); +} + +#[gpui::test] +fn test_diff_review_overlay_show_and_dismiss(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Show overlay + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(0), window, cx); + }) + .unwrap(); + + // Verify overlay is shown + editor + .update(cx, |editor, _window, _cx| { + assert!(!editor.diff_review_overlays.is_empty()); + assert_eq!(editor.diff_review_display_row(), Some(DisplayRow(0))); + assert!(editor.diff_review_prompt_editor().is_some()); + }) + .unwrap(); + + // Dismiss overlay + editor + .update(cx, |editor, _window, cx| { + editor.dismiss_all_diff_review_overlays(cx); + }) + .unwrap(); + + // Verify overlay is dismissed + editor + .update(cx, |editor, _window, _cx| { + assert!(editor.diff_review_overlays.is_empty()); + assert_eq!(editor.diff_review_display_row(), None); + assert!(editor.diff_review_prompt_editor().is_none()); + }) + .unwrap(); +} + +#[gpui::test] +fn test_diff_review_overlay_dismiss_via_cancel(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Show overlay + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(0), window, cx); + }) + .unwrap(); + + // Verify overlay is shown + editor + .update(cx, |editor, _window, _cx| { + assert!(!editor.diff_review_overlays.is_empty()); + }) + .unwrap(); + + // Dismiss via dismiss_menus_and_popups (which is called by cancel action) + editor + .update(cx, |editor, window, cx| { + editor.dismiss_menus_and_popups(true, window, cx); + }) + .unwrap(); + + // Verify overlay is dismissed + editor + .update(cx, |editor, _window, _cx| { + assert!(editor.diff_review_overlays.is_empty()); + }) + .unwrap(); +} + +#[gpui::test] +fn test_diff_review_empty_comment_not_submitted(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Show overlay + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(0), window, cx); + }) + .unwrap(); + + // Try to submit without typing anything (empty comment) + editor + .update(cx, |editor, window, cx| { + editor.submit_diff_review_comment(window, cx); + }) + .unwrap(); + + // Verify no comment was added + editor + .update(cx, |editor, _window, _cx| { + assert_eq!(editor.total_review_comment_count(), 0); + }) + .unwrap(); + + // Try to submit with whitespace-only comment + editor + .update(cx, |editor, window, cx| { + if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() { + prompt_editor.update(cx, |pe, cx| { + pe.insert(" \n\t ", window, cx); + }); + } + editor.submit_diff_review_comment(window, cx); + }) + .unwrap(); + + // Verify still no comment was added + editor + .update(cx, |editor, _window, _cx| { + assert_eq!(editor.total_review_comment_count(), 0); + }) + .unwrap(); +} + +#[gpui::test] +fn test_diff_review_inline_edit_flow(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Add a comment directly + let comment_id = editor + .update(cx, |editor, _window, cx| { + let key = test_hunk_key(""); + add_test_comment(editor, key, "Original comment", 0, cx) + }) + .unwrap(); + + // Set comment to editing mode + editor + .update(cx, |editor, _window, cx| { + editor.set_comment_editing(comment_id, true, cx); + }) + .unwrap(); + + // Verify editing flag is set + editor + .update(cx, |editor, _window, cx| { + let key = test_hunk_key(""); + let snapshot = editor.buffer().read(cx).snapshot(cx); + let comments = editor.comments_for_hunk(&key, &snapshot); + assert_eq!(comments.len(), 1); + assert!(comments[0].is_editing); + }) + .unwrap(); + + // Update the comment + editor + .update(cx, |editor, _window, cx| { + let updated = + editor.update_review_comment(comment_id, "Updated comment".to_string(), cx); + assert!(updated); + }) + .unwrap(); + + // Verify comment was updated and editing flag is cleared + editor + .update(cx, |editor, _window, cx| { + let key = test_hunk_key(""); + let snapshot = editor.buffer().read(cx).snapshot(cx); + let comments = editor.comments_for_hunk(&key, &snapshot); + assert_eq!(comments[0].comment, "Updated comment"); + assert!(!comments[0].is_editing); + }) + .unwrap(); +} + +#[gpui::test] +fn test_orphaned_comments_are_cleaned_up(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // Create an editor with some text + let editor = cx.add_window(|window, cx| { + let buffer = cx.new(|cx| Buffer::local("line 1\nline 2\nline 3\n", cx)); + let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + Editor::new(EditorMode::full(), multi_buffer, None, window, cx) + }); + + // Add a comment with an anchor on line 2 + editor + .update(cx, |editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor = snapshot.anchor_after(Point::new(1, 0)); // Line 2 + let key = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::empty()), + hunk_start_anchor: anchor, + }; + editor.add_review_comment( + key, + "Comment on line 2".to_string(), + DisplayRow(1), + anchor..anchor, + cx, + ); + assert_eq!(editor.total_review_comment_count(), 1); + }) + .unwrap(); + + // Delete all content (this should orphan the comment's anchor) + editor + .update(cx, |editor, window, cx| { + editor.select_all(&SelectAll, window, cx); + editor.insert("completely new content", window, cx); + }) + .unwrap(); + + // Trigger cleanup + editor + .update(cx, |editor, _window, cx| { + editor.cleanup_orphaned_review_comments(cx); + // Comment should be removed because its anchor is invalid + assert_eq!(editor.total_review_comment_count(), 0); + }) + .unwrap(); +} + +#[gpui::test] +fn test_orphaned_comments_cleanup_called_on_buffer_edit(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // Create an editor with some text + let editor = cx.add_window(|window, cx| { + let buffer = cx.new(|cx| Buffer::local("line 1\nline 2\nline 3\n", cx)); + let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + Editor::new(EditorMode::full(), multi_buffer, None, window, cx) + }); + + // Add a comment with an anchor on line 2 + editor + .update(cx, |editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor = snapshot.anchor_after(Point::new(1, 0)); // Line 2 + let key = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::empty()), + hunk_start_anchor: anchor, + }; + editor.add_review_comment( + key, + "Comment on line 2".to_string(), + DisplayRow(1), + anchor..anchor, + cx, + ); + assert_eq!(editor.total_review_comment_count(), 1); + }) + .unwrap(); + + // Edit the buffer - this should trigger cleanup via on_buffer_event + // Delete all content which orphans the anchor + editor + .update(cx, |editor, window, cx| { + editor.select_all(&SelectAll, window, cx); + editor.insert("completely new content", window, cx); + // The cleanup is called automatically in on_buffer_event when Edited fires + }) + .unwrap(); + + // Verify cleanup happened automatically (not manually triggered) + editor + .update(cx, |editor, _window, _cx| { + // Comment should be removed because its anchor became invalid + // and cleanup was called automatically on buffer edit + assert_eq!(editor.total_review_comment_count(), 0); + }) + .unwrap(); +} + +#[gpui::test] +fn test_comments_stored_for_multiple_hunks(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // This test verifies that comments can be stored for multiple different hunks + // and that hunk_comment_count correctly identifies comments per hunk. + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + + // Create two different hunk keys (simulating two different files) + let anchor = snapshot.anchor_before(Point::new(0, 0)); + let key1 = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::unix("file1.rs").unwrap()), + hunk_start_anchor: anchor, + }; + let key2 = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::unix("file2.rs").unwrap()), + hunk_start_anchor: anchor, + }; + + // Add comments to first hunk + editor.add_review_comment( + key1.clone(), + "Comment 1 for file1".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + editor.add_review_comment( + key1.clone(), + "Comment 2 for file1".to_string(), + DisplayRow(1), + anchor..anchor, + cx, + ); + + // Add comment to second hunk + editor.add_review_comment( + key2.clone(), + "Comment for file2".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + + // Verify total count + assert_eq!(editor.total_review_comment_count(), 3); + + // Verify per-hunk counts + let snapshot = editor.buffer().read(cx).snapshot(cx); + assert_eq!( + editor.hunk_comment_count(&key1, &snapshot), + 2, + "file1 should have 2 comments" + ); + assert_eq!( + editor.hunk_comment_count(&key2, &snapshot), + 1, + "file2 should have 1 comment" + ); + + // Verify comments_for_hunk returns correct comments + let file1_comments = editor.comments_for_hunk(&key1, &snapshot); + assert_eq!(file1_comments.len(), 2); + assert_eq!(file1_comments[0].comment, "Comment 1 for file1"); + assert_eq!(file1_comments[1].comment, "Comment 2 for file1"); + + let file2_comments = editor.comments_for_hunk(&key2, &snapshot); + assert_eq!(file2_comments.len(), 1); + assert_eq!(file2_comments[0].comment, "Comment for file2"); + }); +} + +#[gpui::test] +fn test_same_hunk_detected_by_matching_keys(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // This test verifies that hunk_keys_match correctly identifies when two + // DiffHunkKeys refer to the same hunk (same file path and anchor point). + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor = snapshot.anchor_before(Point::new(0, 0)); + + // Create two keys with the same file path and anchor + let key1 = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::unix("file.rs").unwrap()), + hunk_start_anchor: anchor, + }; + let key2 = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::unix("file.rs").unwrap()), + hunk_start_anchor: anchor, + }; + + // Add comment to first key + editor.add_review_comment( + key1, + "Test comment".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + + // Verify second key (same hunk) finds the comment + let snapshot = editor.buffer().read(cx).snapshot(cx); + assert_eq!( + editor.hunk_comment_count(&key2, &snapshot), + 1, + "Same hunk should find the comment" + ); + + // Create a key with different file path + let different_file_key = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::unix("other.rs").unwrap()), + hunk_start_anchor: anchor, + }; + + // Different file should not find the comment + assert_eq!( + editor.hunk_comment_count(&different_file_key, &snapshot), + 0, + "Different file should not find the comment" + ); + }); +} + +#[gpui::test] +fn test_overlay_comments_expanded_state(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // This test verifies that set_diff_review_comments_expanded correctly + // updates the expanded state of overlays. + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Show overlay + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(0), window, cx); + }) + .unwrap(); + + // Verify initially expanded (default) + editor + .update(cx, |editor, _window, _cx| { + assert!( + editor.diff_review_overlays[0].comments_expanded, + "Should be expanded by default" + ); + }) + .unwrap(); + + // Set to collapsed using the public method + editor + .update(cx, |editor, _window, cx| { + editor.set_diff_review_comments_expanded(false, cx); + }) + .unwrap(); + + // Verify collapsed + editor + .update(cx, |editor, _window, _cx| { + assert!( + !editor.diff_review_overlays[0].comments_expanded, + "Should be collapsed after setting to false" + ); + }) + .unwrap(); + + // Set back to expanded + editor + .update(cx, |editor, _window, cx| { + editor.set_diff_review_comments_expanded(true, cx); + }) + .unwrap(); + + // Verify expanded again + editor + .update(cx, |editor, _window, _cx| { + assert!( + editor.diff_review_overlays[0].comments_expanded, + "Should be expanded after setting to true" + ); + }) + .unwrap(); +} + +#[gpui::test] +fn test_calculate_overlay_height(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // This test verifies that calculate_overlay_height returns correct heights + // based on comment count and expanded state. + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + _ = editor.update(cx, |editor, _window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let anchor = snapshot.anchor_before(Point::new(0, 0)); + let key = DiffHunkKey { + file_path: Arc::from(util::rel_path::RelPath::empty()), + hunk_start_anchor: anchor, + }; + + // No comments: base height of 2 + let height_no_comments = editor.calculate_overlay_height(&key, true, &snapshot); + assert_eq!( + height_no_comments, 2, + "Base height should be 2 with no comments" + ); + + // Add one comment + editor.add_review_comment( + key.clone(), + "Comment 1".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + + // With comments expanded: base (2) + header (1) + 2 per comment + let height_expanded = editor.calculate_overlay_height(&key, true, &snapshot); + assert_eq!( + height_expanded, + 2 + 1 + 2, // base + header + 1 comment * 2 + "Height with 1 comment expanded" + ); + + // With comments collapsed: base (2) + header (1) + let height_collapsed = editor.calculate_overlay_height(&key, false, &snapshot); + assert_eq!( + height_collapsed, + 2 + 1, // base + header only + "Height with comments collapsed" + ); + + // Add more comments + editor.add_review_comment( + key.clone(), + "Comment 2".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + editor.add_review_comment( + key.clone(), + "Comment 3".to_string(), + DisplayRow(0), + anchor..anchor, + cx, + ); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + + // With 3 comments expanded + let height_3_expanded = editor.calculate_overlay_height(&key, true, &snapshot); + assert_eq!( + height_3_expanded, + 2 + 1 + (3 * 2), // base + header + 3 comments * 2 + "Height with 3 comments expanded" + ); + + // Collapsed height stays the same regardless of comment count + let height_3_collapsed = editor.calculate_overlay_height(&key, false, &snapshot); + assert_eq!( + height_3_collapsed, + 2 + 1, // base + header only + "Height with 3 comments collapsed should be same as 1 comment collapsed" + ); + }); +} + #[gpui::test] async fn test_move_to_start_end_of_larger_syntax_node_single_cursor(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 38a6eac5806507e76962a8afee400992085aa760..1ecb03e22465327a175bb6842ff8c3db07f6e81a 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -504,6 +504,12 @@ impl EditorElement { register_action(editor, window, Editor::unstage_and_next); register_action(editor, window, Editor::expand_all_diff_hunks); register_action(editor, window, Editor::collapse_all_diff_hunks); + register_action(editor, window, Editor::toggle_review_comments_expanded); + register_action(editor, window, Editor::submit_diff_review_comment_action); + register_action(editor, window, Editor::edit_review_comment); + register_action(editor, window, Editor::delete_review_comment); + register_action(editor, window, Editor::confirm_edit_review_comment_action); + register_action(editor, window, Editor::cancel_edit_review_comment_action); register_action(editor, window, Editor::go_to_previous_change); register_action(editor, window, Editor::go_to_next_change); register_action(editor, window, Editor::go_to_prev_reference); @@ -3116,26 +3122,6 @@ impl EditorElement { Some((display_row, buffer_row)) } - fn diff_review_button(width: Pixels, cx: &mut App) -> AnyElement { - let text_c = cx.theme().colors().text; - let icon_c = cx.theme().colors().icon_accent; - - h_flex() - .id("diff_review_button") - .cursor_pointer() - .w(width - px(1.)) - .h(relative(0.9)) - .justify_center() - .rounded_sm() - .border_1() - .border_color(text_c.opacity(0.1)) - .bg(text_c.opacity(0.15)) - .hover(|s| s.bg(icon_c.opacity(0.4)).border_color(icon_c.opacity(0.5))) - .child(Icon::new(IconName::Plus).size(IconSize::Small)) - .tooltip(Tooltip::text("Add Review")) - .into_any_element() - } - #[allow(clippy::too_many_arguments)] fn layout_run_indicators( &self, @@ -10460,8 +10446,13 @@ impl Element for EditorElement { available_width + em_width - px(6.) }; + let button = self.editor.update(cx, |editor, cx| { + editor + .render_diff_review_button(display_row, button_width, cx) + .into_any_element() + }); prepaint_gutter_button( - Self::diff_review_button(button_width, cx), + button, display_row, line_height, &gutter_dimensions, diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index ce85835e3dd0d4afc859db7f3ee2d47564adc04a..e56ee5a814d04da551c0af5cb387acf291c53ba5 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -9,7 +9,7 @@ use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus}; use collections::{HashMap, HashSet}; use editor::{ Addon, Editor, EditorEvent, SelectionEffects, SplittableEditor, - actions::{GoToHunk, GoToPreviousHunk}, + actions::{GoToHunk, GoToPreviousHunk, SendReviewToAgent}, multibuffer_context_lines, scroll::Autoscroll, }; @@ -70,6 +70,7 @@ pub struct ProjectDiff { workspace: WeakEntity, focus_handle: FocusHandle, pending_scroll: Option, + review_comment_count: usize, _task: Task>, _subscription: Subscription, } @@ -311,8 +312,16 @@ impl ProjectDiff { }); diff_display_editor }); - cx.subscribe_in(&editor, window, Self::handle_editor_event) - .detach(); + let editor_subscription = cx.subscribe_in(&editor, window, Self::handle_editor_event); + + let primary_editor = editor.read(cx).primary_editor().clone(); + let review_comment_subscription = + cx.subscribe(&primary_editor, |this, _editor, event: &EditorEvent, cx| { + if let EditorEvent::ReviewCommentsChanged { total_count } = event { + this.review_comment_count = *total_count; + cx.notify(); + } + }); let branch_diff_subscription = cx.subscribe_in( &branch_diff, @@ -363,8 +372,12 @@ impl ProjectDiff { multibuffer, buffer_diff_subscriptions: Default::default(), pending_scroll: None, + review_comment_count: 0, _task: task, - _subscription: branch_diff_subscription, + _subscription: Subscription::join( + branch_diff_subscription, + Subscription::join(editor_subscription, review_comment_subscription), + ), } } @@ -453,6 +466,16 @@ impl ProjectDiff { } } + /// Returns the total count of review comments across all hunks/files. + pub fn total_review_comment_count(&self) -> usize { + self.review_comment_count + } + + /// Returns a reference to the splittable editor. + pub fn editor(&self) -> &Entity { + &self.editor + } + fn button_states(&self, cx: &App) -> ButtonStates { let editor = self.editor.read(cx).primary_editor().read(cx); let snapshot = self.multibuffer.read(cx).snapshot(cx); @@ -1278,6 +1301,7 @@ impl Render for ProjectDiffToolbar { }; let focus_handle = project_diff.focus_handle(cx); let button_states = project_diff.read(cx).button_states(cx); + let review_count = project_diff.read(cx).total_review_comment_count(); h_group_xl() .my_neg_1() @@ -1419,6 +1443,25 @@ impl Render for ProjectDiffToolbar { })), ), ) + // "Send Review to Agent" button (only shown when there are review comments) + .when(review_count > 0, |el| { + el.child(vertical_divider()).child( + Button::new( + "send-review", + format!("Send Review to Agent ({})", review_count), + ) + .icon(IconName::ZedAssistant) + .icon_position(IconPosition::Start) + .tooltip(Tooltip::for_action_title_in( + "Send all review comments to the Agent panel", + &SendReviewToAgent, + &focus_handle, + )) + .on_click(cx.listener(|this, _, window, cx| { + this.dispatch_action(&SendReviewToAgent, window, cx) + })), + ) + }) } } diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 42ceeee479119aefd6de72e0763e4dd51f3908be..18138ff920373d867d794820367dc2046d356f8a 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -50,6 +50,7 @@ use { agent_servers::{AgentServer, AgentServerDelegate}, anyhow::{Context as _, Result}, assets::Assets, + editor::display_map::DisplayRow, feature_flags::FeatureFlagAppExt as _, git_ui::project_diff::ProjectDiff, gpui::{ @@ -1358,6 +1359,198 @@ import { AiPaneTabContext } from 'context'; update_baseline, )?; + // Test 4: Show the diff review overlay on the regular editor + regular_window + .update(cx, |workspace, window, cx| { + // Get the first editor from the workspace + let editors: Vec<_> = workspace.items_of_type::(cx).collect(); + if let Some(editor) = editors.into_iter().next() { + editor.update(cx, |editor, cx| { + editor.show_diff_review_overlay(DisplayRow(1), window, cx); + }); + } + }) + .ok(); + + // Wait for overlay to render + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 4: Regular editor with overlay shown + let test4_result = run_visual_test( + "diff_review_overlay_shown", + regular_window.into(), + cx, + update_baseline, + )?; + + // Test 5: Type text into the diff review prompt and submit it + // First, get the prompt editor from the overlay and type some text + regular_window + .update(cx, |workspace, window, cx| { + let editors: Vec<_> = workspace.items_of_type::(cx).collect(); + if let Some(editor) = editors.into_iter().next() { + editor.update(cx, |editor, cx| { + // Get the prompt editor from the overlay and insert text + if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() { + prompt_editor.update(cx, |prompt_editor: &mut editor::Editor, cx| { + prompt_editor.insert( + "This change needs better error handling", + window, + cx, + ); + }); + } + }); + } + }) + .ok(); + + // Wait for text to be inserted + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 5: Diff review overlay with typed text + let test5_result = run_visual_test( + "diff_review_overlay_with_text", + regular_window.into(), + cx, + update_baseline, + )?; + + // Test 6: Submit a comment to store it locally + regular_window + .update(cx, |workspace, window, cx| { + let editors: Vec<_> = workspace.items_of_type::(cx).collect(); + if let Some(editor) = editors.into_iter().next() { + editor.update(cx, |editor, cx| { + // Submit the comment that was typed in test 5 + editor.submit_diff_review_comment(window, cx); + }); + } + }) + .ok(); + + // Wait for comment to be stored + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 6: Overlay with one stored comment + let test6_result = run_visual_test( + "diff_review_one_comment", + regular_window.into(), + cx, + update_baseline, + )?; + + // Test 7: Add more comments to show multiple comments expanded + regular_window + .update(cx, |workspace, window, cx| { + let editors: Vec<_> = workspace.items_of_type::(cx).collect(); + if let Some(editor) = editors.into_iter().next() { + editor.update(cx, |editor, cx| { + // Add second comment + if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() { + prompt_editor.update(cx, |pe, cx| { + pe.insert("Second comment about imports", window, cx); + }); + } + editor.submit_diff_review_comment(window, cx); + + // Add third comment + if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() { + prompt_editor.update(cx, |pe, cx| { + pe.insert("Third comment about naming conventions", window, cx); + }); + } + editor.submit_diff_review_comment(window, cx); + }); + } + }) + .ok(); + + // Wait for comments to be stored + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 7: Overlay with multiple comments expanded + let test7_result = run_visual_test( + "diff_review_multiple_comments_expanded", + regular_window.into(), + cx, + update_baseline, + )?; + + // Test 8: Collapse the comments section + regular_window + .update(cx, |workspace, _window, cx| { + let editors: Vec<_> = workspace.items_of_type::(cx).collect(); + if let Some(editor) = editors.into_iter().next() { + editor.update(cx, |editor, cx| { + // Toggle collapse using the public method + editor.set_diff_review_comments_expanded(false, cx); + }); + } + }) + .ok(); + + // Wait for UI to update + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 8: Comments collapsed + let test8_result = run_visual_test( + "diff_review_comments_collapsed", + regular_window.into(), + cx, + update_baseline, + )?; + // Clean up: remove worktrees to stop background scanning workspace_window .update(cx, |workspace, _window, cx| { @@ -1391,12 +1584,27 @@ import { AiPaneTabContext } from 'context'; } // Return combined result - match (&test1_result, &test2_result, &test3_result) { - (TestResult::Passed, TestResult::Passed, TestResult::Passed) => Ok(TestResult::Passed), - (TestResult::BaselineUpdated(p), _, _) - | (_, TestResult::BaselineUpdated(p), _) - | (_, _, TestResult::BaselineUpdated(p)) => Ok(TestResult::BaselineUpdated(p.clone())), - } + let all_results = [ + &test1_result, + &test2_result, + &test3_result, + &test4_result, + &test5_result, + &test6_result, + &test7_result, + &test8_result, + ]; + + // Combine results: if any test updated a baseline, return BaselineUpdated; + // otherwise return Passed. The exhaustive match ensures the compiler + // verifies we handle all TestResult variants. + let result = all_results + .iter() + .fold(TestResult::Passed, |acc, r| match r { + TestResult::Passed => acc, + TestResult::BaselineUpdated(p) => TestResult::BaselineUpdated(p.clone()), + }); + Ok(result) } /// A stub AgentServer for visual testing that returns a pre-programmed connection.