From 21be70f278acc2818c628bb0e8d33c8648655e34 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 24 Sep 2024 11:40:08 -0400 Subject: [PATCH] Improve diff hunks (#18283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR improves the display of diff hunks: - Deleted hunks now show a regular line indicator in the gutter when expanded - The rounding on the diff indicators in the gutter has been removed. We also did some refactoring to ensure the sizing of the diff indicators in the gutter were consistent. #### Collapsed Screenshot 2024-09-24 at 11 13 26 AM #### Expanded Screenshot 2024-09-24 at 11 13 35 AM Release Notes: - Improved the appearance of diff hunks in the editor. --------- Co-authored-by: Max --- crates/editor/src/editor.rs | 5 -- crates/editor/src/element.rs | 120 +++++++++++++++++++++++++-------- crates/editor/src/git.rs | 4 +- crates/editor/src/hunk_diff.rs | 77 +++++++++------------ 4 files changed, 124 insertions(+), 82 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index dc536471023f0893d77fd0a73b19838b8b01a0e7..a32910e78ab973b76db3ba42f996360e6af58042 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -12473,11 +12473,6 @@ impl Editor { Some(gpui::Point::new(source_x, source_y)) } - fn gutter_bounds(&self) -> Option> { - let bounds = self.last_bounds?; - Some(element::gutter_bounds(bounds, self.gutter_dimensions)) - } - pub fn has_active_completions_menu(&self) -> bool { self.context_menu.read().as_ref().map_or(false, |menu| { menu.visible() && matches!(menu, ContextMenu::Completions(_)) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index d4075431ff602bd6595374b8f30ceb780e7c4e1c..3be71aeefba94228803ccf03bbddaccfab7ddcf7 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1269,6 +1269,7 @@ impl EditorElement { line_height: Pixels, gutter_hitbox: &Hitbox, display_rows: Range, + anchor_range: Range, snapshot: &EditorSnapshot, cx: &mut WindowContext, ) -> Vec<(DisplayDiffHunk, Option)> { @@ -1289,30 +1290,84 @@ impl EditorElement { .git .git_gutter .unwrap_or_default(); - let display_hunks = buffer_snapshot - .git_diff_hunks_in_range(buffer_start_row..buffer_end_row) - .map(|hunk| diff_hunk_to_display(&hunk, snapshot)) - .dedup() - .map(|hunk| match git_gutter_setting { - GitGutterSetting::TrackedFiles => { - let hitbox = match hunk { - DisplayDiffHunk::Unfolded { .. } => { - let hunk_bounds = Self::diff_hunk_bounds( - snapshot, - line_height, - gutter_hitbox.bounds, - &hunk, - ); - Some(cx.insert_hitbox(hunk_bounds, true)) + + self.editor.update(cx, |editor, cx| { + let expanded_hunks = &editor.expanded_hunks.hunks; + let expanded_hunks_start_ix = expanded_hunks + .binary_search_by(|hunk| { + hunk.hunk_range + .end + .cmp(&anchor_range.start, &buffer_snapshot) + .then(Ordering::Less) + }) + .unwrap_err(); + let mut expanded_hunks = expanded_hunks[expanded_hunks_start_ix..].iter().peekable(); + + let display_hunks = buffer_snapshot + .git_diff_hunks_in_range(buffer_start_row..buffer_end_row) + .filter_map(|hunk| { + let mut display_hunk = diff_hunk_to_display(&hunk, snapshot); + + if let DisplayDiffHunk::Unfolded { + multi_buffer_range, + status, + .. + } = &mut display_hunk + { + let mut is_expanded = false; + while let Some(expanded_hunk) = expanded_hunks.peek() { + match expanded_hunk + .hunk_range + .start + .cmp(&multi_buffer_range.start, &buffer_snapshot) + { + Ordering::Less => { + expanded_hunks.next(); + } + Ordering::Equal => { + is_expanded = true; + break; + } + Ordering::Greater => { + break; + } + } } - DisplayDiffHunk::Folded { .. } => None, - }; - (hunk, hitbox) - } - GitGutterSetting::Hide => (hunk, None), - }) - .collect(); - display_hunks + match status { + DiffHunkStatus::Added => {} + DiffHunkStatus::Modified => {} + DiffHunkStatus::Removed => { + if is_expanded { + return None; + } + } + } + } + + Some(display_hunk) + }) + .dedup() + .map(|hunk| match git_gutter_setting { + GitGutterSetting::TrackedFiles => { + let hitbox = match hunk { + DisplayDiffHunk::Unfolded { .. } => { + let hunk_bounds = Self::diff_hunk_bounds( + snapshot, + line_height, + gutter_hitbox.bounds, + &hunk, + ); + Some(cx.insert_hitbox(hunk_bounds, true)) + } + DisplayDiffHunk::Folded { .. } => None, + }; + (hunk, hitbox) + } + GitGutterSetting::Hide => (hunk, None), + }) + .collect(); + display_hunks + }) } #[allow(clippy::too_many_arguments)] @@ -3187,7 +3242,7 @@ impl EditorElement { Some(( hunk_bounds, cx.theme().status().modified, - Corners::all(1. * line_height), + Corners::all(px(0.)), )) } DisplayDiffHunk::Unfolded { status, .. } => { @@ -3195,12 +3250,12 @@ impl EditorElement { DiffHunkStatus::Added => ( hunk_hitbox.bounds, cx.theme().status().created, - Corners::all(0.05 * line_height), + Corners::all(px(0.)), ), DiffHunkStatus::Modified => ( hunk_hitbox.bounds, cx.theme().status().modified, - Corners::all(0.05 * line_height), + Corners::all(px(0.)), ), DiffHunkStatus::Removed => ( Bounds::new( @@ -3244,7 +3299,7 @@ impl EditorElement { let start_y = display_row.as_f32() * line_height - scroll_top; let end_y = start_y + line_height; - let width = 0.275 * line_height; + let width = Self::diff_hunk_strip_width(line_height); let highlight_origin = gutter_bounds.origin + point(px(0.), start_y); let highlight_size = size(width, end_y - start_y); Bounds::new(highlight_origin, highlight_size) @@ -3277,7 +3332,7 @@ impl EditorElement { let start_y = start_row.as_f32() * line_height - scroll_top; let end_y = end_row_in_current_excerpt.as_f32() * line_height - scroll_top; - let width = 0.275 * line_height; + let width = Self::diff_hunk_strip_width(line_height); let highlight_origin = gutter_bounds.origin + point(px(0.), start_y); let highlight_size = size(width, end_y - start_y); Bounds::new(highlight_origin, highlight_size) @@ -3289,7 +3344,7 @@ impl EditorElement { let start_y = row.as_f32() * line_height - offset - scroll_top; let end_y = start_y + line_height; - let width = 0.35 * line_height; + let width = (0.35 * line_height).floor(); let highlight_origin = gutter_bounds.origin + point(px(0.), start_y); let highlight_size = size(width, end_y - start_y); Bounds::new(highlight_origin, highlight_size) @@ -3298,6 +3353,12 @@ impl EditorElement { } } + /// Returns the width of the diff strip that will be displayed in the gutter. + pub(super) fn diff_hunk_strip_width(line_height: Pixels) -> Pixels { + // We floor the value to prevent pixel rounding. + (0.275 * line_height).floor() + } + fn paint_gutter_indicators(&self, layout: &mut EditorLayout, cx: &mut WindowContext) { cx.paint_layer(layout.gutter_hitbox.bounds, |cx| { cx.with_element_namespace("gutter_fold_toggles", |cx| { @@ -5158,6 +5219,7 @@ impl Element for EditorElement { line_height, &gutter_hitbox, start_row..end_row, + start_anchor..end_anchor, &snapshot, cx, ); diff --git a/crates/editor/src/git.rs b/crates/editor/src/git.rs index 79b78d5d14848889c5d56849f56e9436e567e33c..fb18ca45a2a2ff4b14fe621f4c7855bc9865435e 100644 --- a/crates/editor/src/git.rs +++ b/crates/editor/src/git.rs @@ -90,8 +90,8 @@ pub fn diff_hunk_to_display( let hunk_end_row = hunk.row_range.end.max(hunk.row_range.start); let hunk_end_point = Point::new(hunk_end_row.0, 0); - let multi_buffer_start = snapshot.buffer_snapshot.anchor_after(hunk_start_point); - let multi_buffer_end = snapshot.buffer_snapshot.anchor_before(hunk_end_point); + let multi_buffer_start = snapshot.buffer_snapshot.anchor_before(hunk_start_point); + let multi_buffer_end = snapshot.buffer_snapshot.anchor_after(hunk_end_point); let end = hunk_end_point.to_display_point(snapshot).row(); DisplayDiffHunk::Unfolded { diff --git a/crates/editor/src/hunk_diff.rs b/crates/editor/src/hunk_diff.rs index 917d07ec4ee85bd1d9c75d0acf15bdfc1049d354..90836cee51683c4f76e09d06c104608cdeb6fec6 100644 --- a/crates/editor/src/hunk_diff.rs +++ b/crates/editor/src/hunk_diff.rs @@ -14,8 +14,8 @@ use multi_buffer::{ use settings::SettingsStore; use text::{BufferId, Point}; use ui::{ - div, h_flex, rems, v_flex, ActiveTheme, Context as _, ContextMenu, InteractiveElement, - IntoElement, ParentElement, Pixels, Styled, ViewContext, VisualContext, + prelude::*, ActiveTheme, ContextMenu, InteractiveElement, IntoElement, ParentElement, Pixels, + Styled, ViewContext, VisualContext, }; use util::{debug_panic, RangeExt}; @@ -38,7 +38,7 @@ pub(super) struct HoveredHunk { #[derive(Debug, Default)] pub(super) struct ExpandedHunks { - hunks: Vec, + pub(crate) hunks: Vec, diff_base: HashMap, hunk_update_tasks: HashMap, Task<()>>, } @@ -414,39 +414,22 @@ impl Editor { style: BlockStyle::Flex, disposition: BlockDisposition::Above, render: Box::new(move |cx| { - let Some(gutter_bounds) = editor.read(cx).gutter_bounds() else { - return div().into_any_element(); - }; - let (gutter_dimensions, hunk_bounds, close_button) = - editor.update(cx.context, |editor, cx| { - let editor_snapshot = editor.snapshot(cx); - let hunk_display_range = hunk - .multi_buffer_range - .clone() - .to_display_points(&editor_snapshot); - let gutter_dimensions = editor.gutter_dimensions; - let hunk_bounds = EditorElement::diff_hunk_bounds( - &editor_snapshot, - cx.line_height(), - gutter_bounds, - &DisplayDiffHunk::Unfolded { - diff_base_byte_range: hunk.diff_base_byte_range.clone(), - multi_buffer_range: hunk.multi_buffer_range.clone(), - display_row_range: hunk_display_range.start.row() - ..hunk_display_range.end.row(), - status: hunk.status, - }, - ); - - let close_button = editor.close_hunk_diff_button( - hunk.clone(), - hunk_display_range.start.row(), - cx, - ); - (gutter_dimensions, hunk_bounds, close_button) - }); - let click_editor = editor.clone(); - let clicked_hunk = hunk.clone(); + let width = EditorElement::diff_hunk_strip_width(cx.line_height()); + let gutter_dimensions = editor.read(cx.context).gutter_dimensions; + + let close_button = editor.update(cx.context, |editor, cx| { + let editor_snapshot = editor.snapshot(cx); + let hunk_display_range = hunk + .multi_buffer_range + .clone() + .to_display_points(&editor_snapshot); + editor.close_hunk_diff_button( + hunk.clone(), + hunk_display_range.start.row(), + cx, + ) + }); + h_flex() .id("gutter with editor") .bg(deleted_hunk_color) @@ -461,27 +444,29 @@ impl Editor { .child( h_flex() .id("gutter hunk") + .bg(cx.theme().status().deleted) .pl(gutter_dimensions.margin + gutter_dimensions .git_blame_entries_width .unwrap_or_default()) - .max_w(hunk_bounds.size.width) - .min_w(hunk_bounds.size.width) + .max_w(width) + .min_w(width) .size_full() .cursor(CursorStyle::PointingHand) .on_mouse_down(MouseButton::Left, { - let click_hunk = hunk.clone(); - move |e, cx| { - let modifiers = e.modifiers; + let editor = editor.clone(); + let hunk = hunk.clone(); + move |event, cx| { + let modifiers = event.modifiers; if modifiers.control || modifiers.platform { - click_editor.update(cx, |editor, cx| { - editor.toggle_hovered_hunk(&click_hunk, cx); + editor.update(cx, |editor, cx| { + editor.toggle_hovered_hunk(&hunk, cx); }); } else { - click_editor.update(cx, |editor, cx| { + editor.update(cx, |editor, cx| { editor.open_hunk_context_menu( - clicked_hunk.clone(), - e.position, + hunk.clone(), + event.position, cx, ); });