agent: Multiline review comments (#47448)

Multiline review comments. Also changes a few existing review comment
types to use anchors instead of absolute offsets

Release Notes:

- N/A
This commit is contained in:
Cameron Mcloughlin authored and GitHub committed 2026-01-23 06:25:42 +00:00
1 parent 7ba0bda2c1
commit 8273e6d868
9 files changed
+685 -243

No files matched your search

Generated
+1
View File
@@ -869,6 +869,7 @@ dependencies = [
"html_to_markdown",
"http_client",
"language",
"multi_buffer",
"pretty_assertions",
"project",
"prompt_store",
+2 -2
View File
@@ -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()
@@ -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"] }
@@ -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::<String>();
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())
} 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::<Vec<_>>()
.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::<Vec<_>>()
.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}")
} else {
format!("{path}, Lines {start_line} to {end_line}")
}
} else {
"Quoted selection".to_string()
};
creases.push((text, crease_title));
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<BufferOffset>,
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::<Vec<_>>()
.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::<Vec<_>>()
.join(" > "),
)
} else {
None
};
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 {
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<Point>,
snapshot: &MultiBufferSnapshot,
cx: &App,
) -> Option<(String, String)> {
let selected_text = snapshot.text_for_range(range.clone()).collect::<String>();
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::<Vec<_>>()
.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::<Vec<_>>()
.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(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 {
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"));
}
}
+213 -94
View File
@@ -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<DisplayRow> {
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<Anchor>,
pub range: Range<Anchor>,
/// 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<Anchor>,
) -> Self {
pub fn new(id: usize, comment: String, anchor_range: Range<Anchor>) -> 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<Anchor>,
/// 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<Entity<BreakpointStore>>,
gutter_breakpoint_indicator: (Option<PhantomBreakpointIndicator>, Option<Task<()>>),
pub(crate) gutter_diff_review_indicator: (Option<PhantomDiffReviewIndicator>, Option<Task<()>>),
pub(crate) diff_review_drag_state: Option<DiffReviewDragState>,
/// Active diff review overlays. Multiple overlays can be open simultaneously
/// when hunks have comments stored.
pub(crate) diff_review_overlays: Vec<DiffReviewOverlay>,
@@ -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<Self>,
) {
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<Self>,
) {
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<Self>) {
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>) {
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<DisplayRow>,
window: &mut Window,
cx: &mut Context<Self>,
) {
// 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<dyn language::File>| 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<DisplayRow> {
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<Anchor>,
cx: &mut Context<Self>,
) -> 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<Editor>,
cx: &mut BlockContext,
) -> AnyElement {
fn format_line_ranges(ranges: &[(u32, u32)]) -> Option<String> {
if ranges.is_empty() {
return None;
}
let formatted: Vec<String> = 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()
+188 -72
View File
@@ -30564,16 +30564,9 @@ fn add_test_comment(
editor: &mut Editor,
key: DiffHunkKey,
comment: &str,
display_row: u32,
cx: &mut Context<Editor>,
) -> 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);
+53 -4
View File
@@ -952,6 +952,13 @@ impl EditorElement {
window: &mut Window,
cx: &mut Context<Editor>,
) {
// 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<DisplayRow>,
row_infos: &[RowInfo],
snapshot: &EditorSnapshot,
cx: &App,
) -> Option<(DisplayRow, Option<u32>)> {
if !cx.has_flag::<DiffReviewFeatureFlag>() {
@@ -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
+1
View File
@@ -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",
+1 -1
View File
@@ -1399,7 +1399,7 @@ import { AiPaneTabContext } from 'context';
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(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);
});
}
})