diff --git a/Cargo.lock b/Cargo.lock index 97900b60263db0e58348b3d62bc3fbacecd31d78..05183e5309253fe4b36b4f0a7877273281e32cf7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -869,6 +869,7 @@ dependencies = [ "html_to_markdown", "http_client", "language", + "multi_buffer", "pretty_assertions", "project", "prompt_store", diff --git a/crates/agent_ui/src/text_thread_editor.rs b/crates/agent_ui/src/text_thread_editor.rs index d75e9b59e61155357c22262f3d41f60cf1903e31..18c7026606b5bb29701eeeaf87ae046b650367f8 100644 --- a/crates/agent_ui/src/text_thread_editor.rs +++ b/crates/agent_ui/src/text_thread_editor.rs @@ -1637,8 +1637,8 @@ impl TextThreadEditor { > = 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); + let start = comment.range.start.to_point(&snapshot); + let end = comment.range.end.to_point(&snapshot); comments_by_range .entry((start, end)) .or_default() diff --git a/crates/assistant_slash_commands/Cargo.toml b/crates/assistant_slash_commands/Cargo.toml index b2a70449f449f73c7d0017c5d2ba3707e271559a..843aab9b60255fb5952921439d2ca1fea8b108b7 100644 --- a/crates/assistant_slash_commands/Cargo.toml +++ b/crates/assistant_slash_commands/Cargo.toml @@ -41,6 +41,7 @@ worktree.workspace = true [dev-dependencies] fs = { workspace = true, features = ["test-support"] } gpui = { workspace = true, features = ["test-support"] } +multi_buffer = { workspace = true, features = ["test-support"] } pretty_assertions.workspace = true project = { workspace = true, features = ["test-support"] } settings = { workspace = true, features = ["test-support"] } diff --git a/crates/assistant_slash_commands/src/selection_command.rs b/crates/assistant_slash_commands/src/selection_command.rs index ce6c0b931411d8073ffd6c97b648bb044ad857e7..98d66b7b0b21b265813ce2671e0628cde72ee4ea 100644 --- a/crates/assistant_slash_commands/src/selection_command.rs +++ b/crates/assistant_slash_commands/src/selection_command.rs @@ -3,10 +3,11 @@ use assistant_slash_command::{ ArgumentCompletion, SlashCommand, SlashCommandContent, SlashCommandEvent, SlashCommandOutputSection, SlashCommandResult, }; -use editor::{Editor, MultiBufferSnapshot}; +use editor::{BufferOffset, Editor, MultiBufferSnapshot}; use futures::StreamExt; use gpui::{App, SharedString, Task, WeakEntity, Window}; use language::{BufferSnapshot, CodeLabel, LspAdapterDelegate}; + use rope::Point; use std::ops::Range; use std::sync::Arc; @@ -125,78 +126,232 @@ pub fn selections_creases( ) -> Vec<(String, String)> { let mut creases = Vec::new(); for range in selection_ranges { - let selected_text = snapshot.text_for_range(range.clone()).collect::(); - if selected_text.is_empty() { + let buffer_ranges = snapshot.range_to_buffer_ranges(range.clone()); + + if buffer_ranges.is_empty() { + creases.extend(crease_for_range(range, &snapshot, cx)); continue; } - let start_language = snapshot.language_at(range.start); - let end_language = snapshot.language_at(range.end); - let language_name = if start_language == end_language { - start_language.map(|language| language.code_fence_block_name()) + + for (buffer_snapshot, buffer_range, _excerpt_id) in buffer_ranges { + creases.extend(crease_for_buffer_range(buffer_snapshot, buffer_range, cx)); + } + } + creases +} + +/// Creates a crease for a range within a specific buffer (excerpt). +/// This is used when we know the exact buffer and range within it. +fn crease_for_buffer_range( + buffer: &BufferSnapshot, + Range { start, end }: Range, + cx: &App, +) -> Option<(String, String)> { + let selected_text: String = buffer.text_for_range(start.0..end.0).collect(); + + if selected_text.is_empty() { + return None; + } + + let start_point = buffer.offset_to_point(start.0); + let end_point = buffer.offset_to_point(end.0); + let start_buffer_row = start_point.row; + let end_buffer_row = end_point.row; + + let language = buffer.language_at(start.0); + let language_name_arc = language.map(|l| l.code_fence_block_name()); + let language_name = language_name_arc.as_deref().unwrap_or_default(); + + let filename = buffer + .file() + .map(|file| file.full_path(cx).to_string_lossy().into_owned()); + + let text = if language_name == "markdown" { + selected_text + .lines() + .map(|line| format!("> {}", line)) + .collect::>() + .join("\n") + } else { + let start_symbols = buffer.symbols_containing(start, None); + let end_symbols = buffer.symbols_containing(end, None); + + let outline_text = if !start_symbols.is_empty() && !end_symbols.is_empty() { + Some( + start_symbols + .into_iter() + .zip(end_symbols) + .take_while(|(a, b)| a == b) + .map(|(a, _)| a.text) + .collect::>() + .join(" > "), + ) } else { None }; - let language_name = language_name.as_deref().unwrap_or(""); - let filename = snapshot - .file_at(range.start) - .map(|file| file.full_path(cx).to_string_lossy().into_owned()); - let text = if language_name == "markdown" { - selected_text - .lines() - .map(|line| format!("> {}", line)) - .collect::>() - .join("\n") + + let line_comment_prefix = + language.and_then(|l| l.default_scope().line_comment_prefixes().first().cloned()); + + let fence = + codeblock_fence_for_path(filename.as_deref(), Some(start_buffer_row..=end_buffer_row)); + + if let Some((line_comment_prefix, outline_text)) = line_comment_prefix.zip(outline_text) { + let breadcrumb = format!("{line_comment_prefix}Excerpt from: {outline_text}\n"); + format!("{fence}{breadcrumb}{selected_text}\n```") } else { - let start_symbols = snapshot - .symbols_containing(range.start, None) - .map(|(_, symbols)| symbols); - let end_symbols = snapshot - .symbols_containing(range.end, None) - .map(|(_, symbols)| symbols); - - let outline_text = - if let Some((start_symbols, end_symbols)) = start_symbols.zip(end_symbols) { - Some( - start_symbols - .into_iter() - .zip(end_symbols) - .take_while(|(a, b)| a == b) - .map(|(a, _)| a.text) - .collect::>() - .join(" > "), - ) - } else { - None - }; - - let line_comment_prefix = start_language - .and_then(|l| l.default_scope().line_comment_prefixes().first().cloned()); - - let fence = codeblock_fence_for_path( - filename.as_deref(), - Some(range.start.row..=range.end.row), - ); - - if let Some((line_comment_prefix, outline_text)) = line_comment_prefix.zip(outline_text) - { - let breadcrumb = format!("{line_comment_prefix}Excerpt from: {outline_text}\n"); - format!("{fence}{breadcrumb}{selected_text}\n```") - } else { - format!("{fence}{selected_text}\n```") - } - }; - let crease_title = if let Some(path) = filename { - let start_line = range.start.row + 1; - let end_line = range.end.row + 1; - if start_line == end_line { - format!("{path}, Line {start_line}") + format!("{fence}{selected_text}\n```") + } + }; + + let crease_title = if let Some(path) = filename { + let start_line = start_buffer_row + 1; + let end_line = end_buffer_row + 1; + if start_line == end_line { + format!("{path}, Line {start_line}") + } else { + format!("{path}, Lines {start_line} to {end_line}") + } + } else { + "Quoted selection".to_string() + }; + + Some((text, crease_title)) +} + +/// Fallback function to create a crease from a multibuffer range when we can't split by excerpt. +fn crease_for_range( + range: Range, + snapshot: &MultiBufferSnapshot, + cx: &App, +) -> Option<(String, String)> { + let selected_text = snapshot.text_for_range(range.clone()).collect::(); + if selected_text.is_empty() { + return None; + } + + // Get actual file line numbers (not multibuffer row numbers) + let start_buffer_row = snapshot + .point_to_buffer_point(range.start) + .map(|(_, point, _)| point.row) + .unwrap_or(range.start.row); + let end_buffer_row = snapshot + .point_to_buffer_point(range.end) + .map(|(_, point, _)| point.row) + .unwrap_or(range.end.row); + + let start_language = snapshot.language_at(range.start); + let end_language = snapshot.language_at(range.end); + let language_name = if start_language == end_language { + start_language.map(|language| language.code_fence_block_name()) + } else { + None + }; + let language_name = language_name.as_deref().unwrap_or(""); + + let filename = snapshot + .file_at(range.start) + .map(|file| file.full_path(cx).to_string_lossy().into_owned()); + + let text = if language_name == "markdown" { + selected_text + .lines() + .map(|line| format!("> {}", line)) + .collect::>() + .join("\n") + } else { + let start_symbols = snapshot + .symbols_containing(range.start, None) + .map(|(_, symbols)| symbols); + let end_symbols = snapshot + .symbols_containing(range.end, None) + .map(|(_, symbols)| symbols); + + let outline_text = + if let Some((start_symbols, end_symbols)) = start_symbols.zip(end_symbols) { + Some( + start_symbols + .into_iter() + .zip(end_symbols) + .take_while(|(a, b)| a == b) + .map(|(a, _)| a.text) + .collect::>() + .join(" > "), + ) } else { - format!("{path}, Lines {start_line} to {end_line}") - } + None + }; + + let line_comment_prefix = + start_language.and_then(|l| l.default_scope().line_comment_prefixes().first().cloned()); + + let fence = + codeblock_fence_for_path(filename.as_deref(), Some(start_buffer_row..=end_buffer_row)); + + if let Some((line_comment_prefix, outline_text)) = line_comment_prefix.zip(outline_text) { + let breadcrumb = format!("{line_comment_prefix}Excerpt from: {outline_text}\n"); + format!("{fence}{breadcrumb}{selected_text}\n```") } else { - "Quoted selection".to_string() - }; - creases.push((text, crease_title)); + format!("{fence}{selected_text}\n```") + } + }; + + let crease_title = if let Some(path) = filename { + let start_line = start_buffer_row + 1; + let end_line = end_buffer_row + 1; + if start_line == end_line { + format!("{path}, Line {start_line}") + } else { + format!("{path}, Lines {start_line} to {end_line}") + } + } else { + "Quoted selection".to_string() + }; + + Some((text, crease_title)) +} + +#[cfg(test)] +mod tests { + use super::*; + use gpui::TestAppContext; + use multi_buffer::MultiBuffer; + + #[gpui::test] + fn test_selections_creases_single_excerpt(cx: &mut TestAppContext) { + let buffer = cx.update(|cx| { + MultiBuffer::build_multi( + [("a\nb\nc\n", vec![Point::new(0, 0)..Point::new(3, 0)])], + cx, + ) + }); + let creases = cx.update(|cx| { + let snapshot = buffer.read(cx).snapshot(cx); + selections_creases(vec![Point::new(0, 0)..Point::new(2, 1)], snapshot, cx) + }); + assert_eq!(creases.len(), 1); + assert_eq!(creases[0].0, "```untitled:1-3\na\nb\nc\n```"); + assert_eq!(creases[0].1, "Quoted selection"); + } + + #[gpui::test] + fn test_selections_creases_spans_multiple_excerpts(cx: &mut TestAppContext) { + let buffer = cx.update(|cx| { + MultiBuffer::build_multi( + [ + ("aaa\nbbb\n", vec![Point::new(0, 0)..Point::new(2, 0)]), + ("111\n222\n", vec![Point::new(0, 0)..Point::new(2, 0)]), + ], + cx, + ) + }); + let creases = cx.update(|cx| { + let snapshot = buffer.read(cx).snapshot(cx); + let end = snapshot.offset_to_point(snapshot.len()); + selections_creases(vec![Point::new(0, 0)..end], snapshot, cx) + }); + assert_eq!(creases.len(), 2); + assert!(creases[0].0.contains("aaa") && !creases[0].0.contains("111")); + assert!(creases[1].0.contains("111") && !creases[1].0.contains("aaa")); } - creases } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2c8753c7c59a671bf7541dbb0e83d7e41d2ae973..232ca9efa3943016065d79380effc7420416c1fd 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1027,12 +1027,30 @@ struct PhantomBreakpointIndicator { /// in diff view mode. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct PhantomDiffReviewIndicator { - pub display_row: DisplayRow, + /// The starting anchor of the selection (or the only row if not dragging). + pub start: Anchor, + /// The ending anchor of the selection. Equal to start_anchor for single-line selection. + pub end: Anchor, /// There's a small debounce between hovering over the line and showing the indicator. /// We don't want to show the indicator when moving the mouse from editor to e.g. project panel. pub is_active: bool, } +#[derive(Clone, Debug)] +pub(crate) struct DiffReviewDragState { + pub start_anchor: Anchor, + pub current_anchor: Anchor, +} + +impl DiffReviewDragState { + pub fn row_range(&self, snapshot: &DisplaySnapshot) -> std::ops::RangeInclusive { + let start = self.start_anchor.to_display_point(snapshot).row(); + let current = self.current_anchor.to_display_point(snapshot).row(); + + (start..=current).sorted() + } +} + /// Identifies a specific hunk in the diff buffer. /// Used as a key to group comments by their location. #[derive(Clone, Debug)] @@ -1050,10 +1068,8 @@ pub struct StoredReviewComment { 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, + pub range: Range, /// Timestamp when the comment was created (for chronological ordering). pub created_at: Instant, /// Whether this comment is currently being edited inline. @@ -1061,17 +1077,11 @@ pub struct StoredReviewComment { } impl StoredReviewComment { - pub fn new( - id: usize, - comment: String, - display_row: DisplayRow, - anchor_range: Range, - ) -> Self { + pub fn new(id: usize, comment: String, anchor_range: Range) -> Self { Self { id, comment, - display_row, - anchor_range, + range: anchor_range, created_at: Instant::now(), is_editing: false, } @@ -1080,8 +1090,7 @@ impl StoredReviewComment { /// 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, + pub anchor_range: Range, /// The block ID for the overlay. pub block_id: CustomBlockId, /// The editor entity for the review input. @@ -1270,6 +1279,7 @@ pub struct Editor { breakpoint_store: Option>, gutter_breakpoint_indicator: (Option, Option>), pub(crate) gutter_diff_review_indicator: (Option, Option>), + pub(crate) diff_review_drag_state: Option, /// Active diff review overlays. Multiple overlays can be open simultaneously /// when hunks have comments stored. pub(crate) diff_review_overlays: Vec, @@ -2457,6 +2467,7 @@ impl Editor { breakpoint_store, gutter_breakpoint_indicator: (None, None), gutter_diff_review_indicator: (None, None), + diff_review_drag_state: None, diff_review_overlays: Vec::new(), stored_review_comments: Vec::new(), next_review_comment_id: 0, @@ -4385,6 +4396,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_drag_state.is_some() { + self.cancel_diff_review_drag(cx); + dismissed = true; + } if !self.diff_review_overlays.is_empty() { self.dismiss_all_diff_review_overlays(cx); dismissed = true; @@ -21111,10 +21126,65 @@ impl Editor { .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); - })) + .tooltip(Tooltip::text("Add Review (drag to select multiple lines)")) + .on_mouse_down( + gpui::MouseButton::Left, + cx.listener(move |editor, _event: &gpui::MouseDownEvent, window, cx| { + editor.start_diff_review_drag(display_row, window, cx); + }), + ) + } + + pub fn start_diff_review_drag( + &mut self, + display_row: DisplayRow, + window: &mut Window, + cx: &mut Context, + ) { + let snapshot = self.snapshot(window, cx); + let point = snapshot + .display_snapshot + .display_point_to_point(DisplayPoint::new(display_row, 0), Bias::Left); + let anchor = snapshot.buffer_snapshot().anchor_before(point); + self.diff_review_drag_state = Some(DiffReviewDragState { + start_anchor: anchor, + current_anchor: anchor, + }); + cx.notify(); + } + + pub fn update_diff_review_drag( + &mut self, + display_row: DisplayRow, + window: &mut Window, + cx: &mut Context, + ) { + if self.diff_review_drag_state.is_none() { + return; + } + let snapshot = self.snapshot(window, cx); + let point = snapshot + .display_snapshot + .display_point_to_point(display_row.as_display_point(), Bias::Left); + let anchor = snapshot.buffer_snapshot().anchor_before(point); + if let Some(drag_state) = &mut self.diff_review_drag_state { + drag_state.current_anchor = anchor; + cx.notify(); + } + } + + pub fn end_diff_review_drag(&mut self, window: &mut Window, cx: &mut Context) { + if let Some(drag_state) = self.diff_review_drag_state.take() { + let snapshot = self.snapshot(window, cx); + let range = drag_state.row_range(&snapshot.display_snapshot); + self.show_diff_review_overlay(*range.start()..*range.end(), window, cx); + } + cx.notify(); + } + + pub fn cancel_diff_review_drag(&mut self, cx: &mut Context) { + self.diff_review_drag_state = None; + cx.notify(); } /// Calculates the appropriate block height for the diff review overlay. @@ -21142,24 +21212,38 @@ impl Editor { pub fn show_diff_review_overlay( &mut self, - display_row: DisplayRow, + display_range: Range, window: &mut Window, cx: &mut Context, ) { - // Check if there's already an overlay for the same hunk - if so, just focus it + let Range { start, end } = display_range.sorted(); + 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 + + // Convert display rows to multibuffer points + let start_point = editor_snapshot .display_snapshot - .display_point_to_point(display_point, Bias::Left); + .display_point_to_point(start.as_display_point(), Bias::Left); + let end_point = editor_snapshot + .display_snapshot + .display_point_to_point(end.as_display_point(), Bias::Left); + let end_multi_buffer_row = MultiBufferRow(end_point.row); + + // Create anchor range for the selected lines (start of first line to end of last line) + let line_end = Point::new( + end_point.row, + buffer_snapshot.line_len(end_multi_buffer_row), + ); + let anchor_range = + buffer_snapshot.anchor_after(start_point)..buffer_snapshot.anchor_before(line_end); // Compute the hunk key for this display row let file_path = buffer_snapshot - .file_at(Point::new(buffer_point.row, 0)) + .file_at(start_point) .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 hunk_start_anchor = buffer_snapshot.anchor_before(start_point); let new_hunk_key = DiffHunkKey { file_path, hunk_start_anchor, @@ -21187,9 +21271,10 @@ impl Editor { .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)); + // Create anchor at the end of the last row so the block appears immediately below it + // Use multibuffer coordinates for anchor creation + let line_len = buffer_snapshot.line_len(end_multi_buffer_row); + let anchor = buffer_snapshot.anchor_after(Point::new(end_multi_buffer_row.0, line_len)); // Use the hunk key we already computed let hunk_key = new_hunk_key; @@ -21245,7 +21330,7 @@ impl Editor { }; self.diff_review_overlays.push(DiffReviewOverlay { - display_row, + anchor_range, block_id, prompt_editor: prompt_editor.clone(), hunk_key, @@ -21383,45 +21468,15 @@ impl Editor { }; 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 anchor_range = overlay.anchor_range.clone(); 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, - ); + self.add_review_comment(hunk_key.clone(), comment_text, anchor_range, cx); // Clear the prompt editor but keep the overlay open if let Some(overlay) = self.diff_review_overlays.get(overlay_index) { @@ -21444,11 +21499,22 @@ impl Editor { .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) + /// Returns the line range for the first diff review overlay, if one is active. + /// Returns (start_row, end_row) as physical line numbers in the underlying file. + pub fn diff_review_line_range(&self, cx: &App) -> Option<(u32, u32)> { + let overlay = self.diff_review_overlays.first()?; + let snapshot = self.buffer.read(cx).snapshot(cx); + let start_point = overlay.anchor_range.start.to_point(&snapshot); + let end_point = overlay.anchor_range.end.to_point(&snapshot); + let start_row = snapshot + .point_to_buffer_point(start_point) + .map(|(_, p, _)| p.row) + .unwrap_or(start_point.row); + let end_row = snapshot + .point_to_buffer_point(end_point) + .map(|(_, p, _)| p.row) + .unwrap_or(end_point.row); + Some((start_row, end_row)) } /// Sets whether the comments section is expanded in the diff review overlay. @@ -21507,14 +21573,13 @@ impl Editor { &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 stored_comment = StoredReviewComment::new(id, comment, anchor_range); let snapshot = self.buffer.read(cx).snapshot(cx); let key_point = hunk_key.hunk_start_anchor.to_point(&snapshot); @@ -21616,8 +21681,7 @@ impl Editor { // 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) + comment.range.start.is_valid(&snapshot) && comment.range.end.is_valid(&snapshot) }); } @@ -21901,33 +21965,74 @@ impl Editor { editor_handle: &WeakEntity, cx: &mut BlockContext, ) -> AnyElement { + fn format_line_ranges(ranges: &[(u32, u32)]) -> Option { + if ranges.is_empty() { + return None; + } + let formatted: Vec = ranges + .iter() + .map(|(start, end)| { + let start_line = start + 1; + let end_line = end + 1; + if start_line == end_line { + format!("Line {start_line}") + } else { + format!("Lines {start_line}-{end_line}") + } + }) + .collect(); + // Don't show label for single line in single excerpt + if ranges.len() == 1 && ranges[0].0 == ranges[0].1 { + return None; + } + Some(formatted.join(" ⋯ ")) + } + 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 (comments, comments_expanded, inline_editors, user_avatar_uri, line_ranges) = + 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 (expanded, editors, avatar_uri, line_ranges) = editor + .diff_review_overlays + .iter() + .find(|overlay| { + Editor::hunk_keys_match(&overlay.hunk_key, hunk_key, &snapshot) + }) + .map(|o| { + let start_point = o.anchor_range.start.to_point(&snapshot); + let end_point = o.anchor_range.end.to_point(&snapshot); + // Get line ranges per excerpt to detect discontinuities + let buffer_ranges = + snapshot.range_to_buffer_ranges(start_point..end_point); + let ranges: Vec<(u32, u32)> = buffer_ranges + .iter() + .map(|(buffer, range, _)| { + let start = buffer.offset_to_point(range.start.0).row; + let end = buffer.offset_to_point(range.end.0).row; + (start, end) + }) + .collect(); + ( + o.comments_expanded, + o.inline_edit_editors.clone(), + o.user_avatar_uri.clone(), + if ranges.is_empty() { + None + } else { + Some(ranges) + }, + ) + }) + .unwrap_or((true, HashMap::default(), None, None)); + (comments, expanded, editors, avatar_uri, line_ranges) + }) + .unwrap_or((Vec::new(), true, HashMap::default(), None, None)); let comment_count = comments.len(); let avatar_size = px(20.); @@ -21941,6 +22046,20 @@ impl Editor { .px_2() .pb_2() .gap_2() + // Line range indicator (only shown for multi-line selections or multiple excerpts) + .when_some(line_ranges, |el, ranges| { + let label = format_line_ranges(&ranges); + if let Some(label) = label { + el.child( + h_flex() + .w_full() + .px_2() + .child(Label::new(label).size(LabelSize::Small).color(Color::Muted)), + ) + } else { + el + } + }) // Top row: editable input with user's avatar .child( h_flex() diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 5c4157d4c44d16ff34fab3984a84ef5a3c2493dd..811bc865128f17c683f186448f411be95e9a9b32 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -30564,16 +30564,9 @@ 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, - ) + editor.add_review_comment(key, comment.to_string(), Anchor::min()..Anchor::max(), cx) } #[gpui::test] @@ -30585,7 +30578,7 @@ fn test_review_comment_add_to_hunk(cx: &mut TestAppContext) { _ = 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 id = add_test_comment(editor, key.clone(), "Test comment", cx); let snapshot = editor.buffer().read(cx).snapshot(cx); assert_eq!(editor.total_review_comment_count(), 1); @@ -30611,8 +30604,8 @@ fn test_review_comments_are_per_hunk(cx: &mut TestAppContext) { 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); + add_test_comment(editor, key1.clone(), "Comment for file1", cx); + add_test_comment(editor, key2.clone(), "Comment for file2", cx); let snapshot = editor.buffer().read(cx).snapshot(cx); assert_eq!(editor.total_review_comment_count(), 2); @@ -30639,7 +30632,7 @@ fn test_review_comment_remove(cx: &mut TestAppContext) { _ = 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); + let id = add_test_comment(editor, key, "To be removed", cx); assert_eq!(editor.total_review_comment_count(), 1); @@ -30662,7 +30655,7 @@ fn test_review_comment_update(cx: &mut TestAppContext) { _ = 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 id = add_test_comment(editor, key.clone(), "Original text", cx); let updated = editor.update_review_comment(id, "Updated text".to_string(), cx); assert!(updated); @@ -30687,9 +30680,9 @@ fn test_review_comment_take_all(cx: &mut TestAppContext) { 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); + let id1 = add_test_comment(editor, key1.clone(), "Comment 1", cx); + let id2 = add_test_comment(editor, key1.clone(), "Comment 2", cx); + let id3 = add_test_comment(editor, key2.clone(), "Comment 3", cx); // IDs should be sequential starting from 0 assert_eq!(id1, 0); @@ -30715,8 +30708,8 @@ fn test_review_comment_take_all(cx: &mut TestAppContext) { // 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); + let new_id1 = add_test_comment(editor, key1, "New Comment 1", cx); + let new_id2 = add_test_comment(editor, key2, "New Comment 2", cx); assert_eq!(new_id1, 0, "ID counter should reset after take_all"); assert_eq!(new_id2, 1, "IDs should be sequential after reset"); @@ -30732,15 +30725,15 @@ fn test_diff_review_overlay_show_and_dismiss(cx: &mut TestAppContext) { // Show overlay editor .update(cx, |editor, window, cx| { - editor.show_diff_review_overlay(DisplayRow(0), window, cx); + editor.show_diff_review_overlay(DisplayRow(0)..DisplayRow(0), window, cx); }) .unwrap(); // Verify overlay is shown editor - .update(cx, |editor, _window, _cx| { + .update(cx, |editor, _window, cx| { assert!(!editor.diff_review_overlays.is_empty()); - assert_eq!(editor.diff_review_display_row(), Some(DisplayRow(0))); + assert_eq!(editor.diff_review_line_range(cx), Some((0, 0))); assert!(editor.diff_review_prompt_editor().is_some()); }) .unwrap(); @@ -30754,9 +30747,9 @@ fn test_diff_review_overlay_show_and_dismiss(cx: &mut TestAppContext) { // Verify overlay is dismissed editor - .update(cx, |editor, _window, _cx| { + .update(cx, |editor, _window, cx| { assert!(editor.diff_review_overlays.is_empty()); - assert_eq!(editor.diff_review_display_row(), None); + assert_eq!(editor.diff_review_line_range(cx), None); assert!(editor.diff_review_prompt_editor().is_none()); }) .unwrap(); @@ -30771,7 +30764,7 @@ fn test_diff_review_overlay_dismiss_via_cancel(cx: &mut TestAppContext) { // Show overlay editor .update(cx, |editor, window, cx| { - editor.show_diff_review_overlay(DisplayRow(0), window, cx); + editor.show_diff_review_overlay(DisplayRow(0)..DisplayRow(0), window, cx); }) .unwrap(); @@ -30806,7 +30799,7 @@ fn test_diff_review_empty_comment_not_submitted(cx: &mut TestAppContext) { // Show overlay editor .update(cx, |editor, window, cx| { - editor.show_diff_review_overlay(DisplayRow(0), window, cx); + editor.show_diff_review_overlay(DisplayRow(0)..DisplayRow(0), window, cx); }) .unwrap(); @@ -30854,7 +30847,7 @@ fn test_diff_review_inline_edit_flow(cx: &mut TestAppContext) { let comment_id = editor .update(cx, |editor, _window, cx| { let key = test_hunk_key(""); - add_test_comment(editor, key, "Original comment", 0, cx) + add_test_comment(editor, key, "Original comment", cx) }) .unwrap(); @@ -30917,13 +30910,7 @@ fn test_orphaned_comments_are_cleaned_up(cx: &mut TestAppContext) { 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, - ); + editor.add_review_comment(key, "Comment on line 2".to_string(), anchor..anchor, cx); assert_eq!(editor.total_review_comment_count(), 1); }) .unwrap(); @@ -30966,13 +30953,7 @@ fn test_orphaned_comments_cleanup_called_on_buffer_edit(cx: &mut TestAppContext) 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, - ); + editor.add_review_comment(key, "Comment on line 2".to_string(), anchor..anchor, cx); assert_eq!(editor.total_review_comment_count(), 1); }) .unwrap(); @@ -31023,14 +31004,12 @@ fn test_comments_stored_for_multiple_hunks(cx: &mut TestAppContext) { 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, ); @@ -31039,7 +31018,6 @@ fn test_comments_stored_for_multiple_hunks(cx: &mut TestAppContext) { editor.add_review_comment( key2.clone(), "Comment for file2".to_string(), - DisplayRow(0), anchor..anchor, cx, ); @@ -31095,13 +31073,7 @@ fn test_same_hunk_detected_by_matching_keys(cx: &mut TestAppContext) { }; // Add comment to first key - editor.add_review_comment( - key1, - "Test comment".to_string(), - DisplayRow(0), - anchor..anchor, - cx, - ); + editor.add_review_comment(key1, "Test comment".to_string(), anchor..anchor, cx); // Verify second key (same hunk) finds the comment let snapshot = editor.buffer().read(cx).snapshot(cx); @@ -31137,7 +31109,7 @@ fn test_overlay_comments_expanded_state(cx: &mut TestAppContext) { // Show overlay editor .update(cx, |editor, window, cx| { - editor.show_diff_review_overlay(DisplayRow(0), window, cx); + editor.show_diff_review_overlay(DisplayRow(0)..DisplayRow(0), window, cx); }) .unwrap(); @@ -31186,6 +31158,168 @@ fn test_overlay_comments_expanded_state(cx: &mut TestAppContext) { .unwrap(); } +#[gpui::test] +fn test_diff_review_multiline_selection(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // Create an editor with multiple lines of text + let editor = cx.add_window(|window, cx| { + let buffer = cx.new(|cx| Buffer::local("line 1\nline 2\nline 3\nline 4\nline 5\n", cx)); + let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + Editor::new(EditorMode::full(), multi_buffer, None, window, cx) + }); + + // Test showing overlay with a multi-line selection (lines 1-3, which are rows 0-2) + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(0)..DisplayRow(2), window, cx); + }) + .unwrap(); + + // Verify line range + editor + .update(cx, |editor, _window, cx| { + assert!(!editor.diff_review_overlays.is_empty()); + assert_eq!(editor.diff_review_line_range(cx), Some((0, 2))); + }) + .unwrap(); + + // Dismiss and test with reversed range (end < start) + editor + .update(cx, |editor, _window, cx| { + editor.dismiss_all_diff_review_overlays(cx); + }) + .unwrap(); + + // Show overlay with reversed range - should normalize it + editor + .update(cx, |editor, window, cx| { + editor.show_diff_review_overlay(DisplayRow(3)..DisplayRow(1), window, cx); + }) + .unwrap(); + + // Verify range is normalized (start <= end) + editor + .update(cx, |editor, _window, cx| { + assert_eq!(editor.diff_review_line_range(cx), Some((1, 3))); + }) + .unwrap(); +} + +#[gpui::test] +fn test_diff_review_drag_state(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + 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) + }); + + // Initially no drag state + editor + .update(cx, |editor, _window, _cx| { + assert!(editor.diff_review_drag_state.is_none()); + }) + .unwrap(); + + // Start drag at row 1 + editor + .update(cx, |editor, window, cx| { + editor.start_diff_review_drag(DisplayRow(1), window, cx); + }) + .unwrap(); + + // Verify drag state is set + editor + .update(cx, |editor, window, cx| { + assert!(editor.diff_review_drag_state.is_some()); + let snapshot = editor.snapshot(window, cx); + let range = editor + .diff_review_drag_state + .as_ref() + .unwrap() + .row_range(&snapshot.display_snapshot); + assert_eq!(*range.start(), DisplayRow(1)); + assert_eq!(*range.end(), DisplayRow(1)); + }) + .unwrap(); + + // Update drag to row 3 + editor + .update(cx, |editor, window, cx| { + editor.update_diff_review_drag(DisplayRow(3), window, cx); + }) + .unwrap(); + + // Verify drag state is updated + editor + .update(cx, |editor, window, cx| { + assert!(editor.diff_review_drag_state.is_some()); + let snapshot = editor.snapshot(window, cx); + let range = editor + .diff_review_drag_state + .as_ref() + .unwrap() + .row_range(&snapshot.display_snapshot); + assert_eq!(*range.start(), DisplayRow(1)); + assert_eq!(*range.end(), DisplayRow(3)); + }) + .unwrap(); + + // End drag - should show overlay + editor + .update(cx, |editor, window, cx| { + editor.end_diff_review_drag(window, cx); + }) + .unwrap(); + + // Verify drag state is cleared and overlay is shown + editor + .update(cx, |editor, _window, cx| { + assert!(editor.diff_review_drag_state.is_none()); + assert!(!editor.diff_review_overlays.is_empty()); + assert_eq!(editor.diff_review_line_range(cx), Some((1, 3))); + }) + .unwrap(); +} + +#[gpui::test] +fn test_diff_review_drag_cancel(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let editor = cx.add_window(|window, cx| Editor::single_line(window, cx)); + + // Start drag + editor + .update(cx, |editor, window, cx| { + editor.start_diff_review_drag(DisplayRow(0), window, cx); + }) + .unwrap(); + + // Verify drag state is set + editor + .update(cx, |editor, _window, _cx| { + assert!(editor.diff_review_drag_state.is_some()); + }) + .unwrap(); + + // Cancel drag + editor + .update(cx, |editor, _window, cx| { + editor.cancel_diff_review_drag(cx); + }) + .unwrap(); + + // Verify drag state is cleared and no overlay was created + editor + .update(cx, |editor, _window, _cx| { + assert!(editor.diff_review_drag_state.is_none()); + assert!(editor.diff_review_overlays.is_empty()); + }) + .unwrap(); +} + #[gpui::test] fn test_calculate_overlay_height(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -31210,13 +31344,7 @@ fn test_calculate_overlay_height(cx: &mut TestAppContext) { ); // Add one comment - editor.add_review_comment( - key.clone(), - "Comment 1".to_string(), - DisplayRow(0), - anchor..anchor, - cx, - ); + editor.add_review_comment(key.clone(), "Comment 1".to_string(), anchor..anchor, cx); let snapshot = editor.buffer().read(cx).snapshot(cx); @@ -31237,20 +31365,8 @@ fn test_calculate_overlay_height(cx: &mut TestAppContext) { ); // 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, - ); + editor.add_review_comment(key.clone(), "Comment 2".to_string(), anchor..anchor, cx); + editor.add_review_comment(key.clone(), "Comment 3".to_string(), anchor..anchor, cx); let snapshot = editor.buffer().read(cx).snapshot(cx); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index bf6346b21124b39f02e21f3ef02628d43ea47f76..471af3741b95acf0578e0efe8e218288e2368379 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -952,6 +952,13 @@ impl EditorElement { window: &mut Window, cx: &mut Context, ) { + // Handle diff review drag completion + if editor.diff_review_drag_state.is_some() { + editor.end_diff_review_drag(window, cx); + cx.stop_propagation(); + return; + } + let text_hitbox = &position_map.text_hitbox; let end_selection = editor.has_pending_selection(); let pending_nonempty_selections = editor.has_pending_nonempty_selection(); @@ -1251,6 +1258,11 @@ impl EditorElement { let point_for_position = position_map.point_for_position(event.position); let valid_point = point_for_position.previous_valid; + // Update diff review drag state if we're dragging + if editor.diff_review_drag_state.is_some() { + editor.update_diff_review_drag(valid_point.row(), window, cx); + } + let hovered_diff_control = position_map .diff_hunk_control_bounds .iter() @@ -1345,8 +1357,12 @@ impl EditorElement { }); } + let anchor = position_map + .snapshot + .display_point_to_anchor(valid_point, Bias::Left); Some(PhantomDiffReviewIndicator { - display_row: valid_point.row(), + start: anchor, + end: anchor, is_active: is_visible, }) } else { @@ -1361,7 +1377,11 @@ impl EditorElement { // Don't show breakpoint indicator when diff review indicator is active on this row let is_on_diff_review_button_row = diff_review_indicator.is_some_and(|indicator| { - indicator.is_active && indicator.display_row == valid_point.row() + let start_row = indicator + .start + .to_display_point(&position_map.snapshot.display_snapshot) + .row(); + indicator.is_active && start_row == valid_point.row() }); let breakpoint_indicator = if gutter_hovered @@ -3145,6 +3165,7 @@ impl EditorElement { &self, range: Range, row_infos: &[RowInfo], + snapshot: &EditorSnapshot, cx: &App, ) -> Option<(DisplayRow, Option)> { if !cx.has_flag::() { @@ -3161,7 +3182,10 @@ impl EditorElement { return None; } - let display_row = indicator.display_row; + let display_row = indicator + .start + .to_display_point(&snapshot.display_snapshot) + .row(); let row_index = (display_row.0.saturating_sub(range.start.0)) as usize; let row_info = row_infos.get(row_index); @@ -9777,6 +9801,26 @@ impl Element for EditorElement { .or_insert(background); } + // Add diff review drag selection highlight to text area + if let Some(drag_state) = &self.editor.read(cx).diff_review_drag_state { + let range = drag_state.row_range(&snapshot.display_snapshot); + let start_row = range.start().0; + let end_row = range.end().0; + let drag_highlight_color = + cx.theme().colors().editor_active_line_background; + let drag_highlight = LineHighlight { + background: solid_background(drag_highlight_color), + border: Some(cx.theme().colors().border_focused), + include_gutter: true, + type_id: None, + }; + for row_num in start_row..=end_row { + highlighted_rows + .entry(DisplayRow(row_num)) + .or_insert(drag_highlight); + } + } + let highlighted_gutter_ranges = self.editor.read(cx).gutter_highlights_in_range( start_anchor..end_anchor, @@ -10549,7 +10593,12 @@ impl Element for EditorElement { + 1; let diff_review_button = self - .should_render_diff_review_button(start_row..end_row, &row_infos, cx) + .should_render_diff_review_button( + start_row..end_row, + &row_infos, + &snapshot, + cx, + ) .map(|(display_row, buffer_row)| { let is_wide = max_line_number_length >= EditorSettings::get_global(cx).gutter.min_line_number_digits diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 956d63580404da351d34af3b5cf5fd531d5a0011..6f4aced4259acdc986e2a3f14aee191fe497c23b 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -19,6 +19,7 @@ test-support = [ "http_client/test-support", "db/test-support", "project/test-support", + "remote/test-support", "session/test-support", "settings/test-support", "gpui/test-support", diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index b9c535fa603b763decc12f3ef3bfd94b0eb3e279..e839ecf68607f70276147ab1f64229550489e13c 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -1399,7 +1399,7 @@ import { AiPaneTabContext } from 'context'; 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); + editor.show_diff_review_overlay(DisplayRow(1)..DisplayRow(1), window, cx); }); } })