From 856a8ef5e8ea966214994c45958898b7b18c2c0f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 25 Jul 2024 15:50:57 +0300 Subject: [PATCH] Layout gutter hunk diff close button (X) better (#15178) Closes https://github.com/zed-industries/zed/issues/15164 * Deleted hunk Before: ![before_top](https://github.com/user-attachments/assets/27c72ee5-719f-4787-b222-4f597840936a) After: ![after_top](https://github.com/user-attachments/assets/58095b21-698d-4778-8412-b680d5253e2c) * Modified hunk Before: ![before_down](https://github.com/user-attachments/assets/3fffb73b-7101-493c-b63b-15c3ccaf5362) After: ![after_down](https://github.com/user-attachments/assets/f07a05ed-1260-4e2a-9388-c9cb93020d78) * Added hunk Before: ![before_mid](https://github.com/user-attachments/assets/0dd3f0f9-51e8-449a-bdd7-4c3ba5cd7791) After: ![after_mid](https://github.com/user-attachments/assets/92f749bd-2d95-4650-8334-1d5c38c6aeb6) Release Notes: - N/A --- crates/editor/src/editor.rs | 23 +++----- crates/editor/src/element.rs | 78 +++++++++++++++++++------ crates/editor/src/hunk_diff.rs | 102 ++++++++++++++++++++++++--------- 3 files changed, 142 insertions(+), 61 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ea804f4b22..f402be288b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -594,7 +594,7 @@ pub struct EditorSnapshot { const GIT_BLAME_GUTTER_WIDTH_CHARS: f32 = 53.; -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct GutterDimensions { pub left_padding: Pixels, pub right_padding: Pixels, @@ -617,18 +617,6 @@ impl GutterDimensions { } } -impl Default for GutterDimensions { - fn default() -> Self { - Self { - left_padding: Pixels::ZERO, - right_padding: Pixels::ZERO, - width: Pixels::ZERO, - margin: Pixels::ZERO, - git_blame_entries_width: None, - } - } -} - #[derive(Debug)] pub struct RemoteSelection { pub replica_id: ReplicaId, @@ -5167,7 +5155,7 @@ impl Editor { })) } - fn render_close_hunk_diff_button( + fn close_hunk_diff_button( &self, hunk: HoveredHunk, row: DisplayRow, @@ -11854,6 +11842,11 @@ impl Editor { let source_y = line_height * (source.row() - first_visible_line.row()).0 as f32; 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)) + } } fn hunks_for_selections( @@ -12240,7 +12233,7 @@ impl EditorSnapshot { self.scroll_anchor.scroll_position(&self.display_snapshot) } - pub fn gutter_dimensions( + fn gutter_dimensions( &self, font_id: FontId, font_size: Pixels, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 1d1387bd70..2deae35d3d 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1248,7 +1248,7 @@ impl EditorElement { // Folds contained in a hunk are ignored apart from shrinking visual size // If a fold contains any hunks then that fold line is marked as modified - fn layout_git_gutters( + fn layout_gutter_git_hunks( &self, line_height: Pixels, gutter_hitbox: &Hitbox, @@ -1553,12 +1553,14 @@ impl EditorElement { (offset_y, length) } + #[allow(clippy::too_many_arguments)] fn layout_run_indicators( &self, line_height: Pixels, scroll_pixel_position: gpui::Point, gutter_dimensions: &GutterDimensions, gutter_hitbox: &Hitbox, + rows_with_hunk_bounds: &HashMap>, snapshot: &EditorSnapshot, cx: &mut WindowContext, ) -> Vec { @@ -1602,6 +1604,7 @@ impl EditorElement { gutter_dimensions, scroll_pixel_position, gutter_hitbox, + rows_with_hunk_bounds, cx, ); Some(button) @@ -1610,6 +1613,7 @@ impl EditorElement { }) } + #[allow(clippy::too_many_arguments)] fn layout_code_actions_indicator( &self, line_height: Pixels, @@ -1617,6 +1621,7 @@ impl EditorElement { scroll_pixel_position: gpui::Point, gutter_dimensions: &GutterDimensions, gutter_hitbox: &Hitbox, + rows_with_hunk_bounds: &HashMap>, cx: &mut WindowContext, ) -> Option { let mut active = false; @@ -1640,6 +1645,7 @@ impl EditorElement { gutter_dimensions, scroll_pixel_position, gutter_hitbox, + rows_with_hunk_bounds, cx, ); @@ -3170,7 +3176,7 @@ impl EditorElement { }); } - fn diff_hunk_bounds( + pub(super) fn diff_hunk_bounds( snapshot: &EditorSnapshot, line_height: Pixels, gutter_bounds: Bounds, @@ -4040,20 +4046,22 @@ impl EditorElement { self.column_pixels(digit_count, cx) } + #[allow(clippy::too_many_arguments)] fn layout_hunk_diff_close_indicators( &self, - expanded_hunks_by_rows: HashMap, line_height: Pixels, scroll_pixel_position: gpui::Point, gutter_dimensions: &GutterDimensions, gutter_hitbox: &Hitbox, + rows_with_hunk_bounds: &HashMap>, + expanded_hunks_by_rows: HashMap, cx: &mut WindowContext, ) -> Vec { self.editor.update(cx, |editor, cx| { expanded_hunks_by_rows .into_iter() .map(|(display_row, hunk)| { - let button = editor.render_close_hunk_diff_button( + let button = editor.close_hunk_diff_button( HoveredHunk { multi_buffer_range: hunk.hunk_range, status: hunk.status, @@ -4070,6 +4078,7 @@ impl EditorElement { gutter_dimensions, scroll_pixel_position, gutter_hitbox, + rows_with_hunk_bounds, cx, ) }) @@ -4078,6 +4087,7 @@ impl EditorElement { } } +#[allow(clippy::too_many_arguments)] fn prepaint_gutter_button( button: IconButton, row: DisplayRow, @@ -4085,6 +4095,7 @@ fn prepaint_gutter_button( gutter_dimensions: &GutterDimensions, scroll_pixel_position: gpui::Point, gutter_hitbox: &Hitbox, + rows_with_hunk_bounds: &HashMap>, cx: &mut WindowContext<'_>, ) -> AnyElement { let mut button = button.into_any_element(); @@ -4094,14 +4105,16 @@ fn prepaint_gutter_button( ); let indicator_size = button.layout_as_root(available_space, cx); - let blame_width = gutter_dimensions - .git_blame_entries_width - .unwrap_or(Pixels::ZERO); + let blame_offset = gutter_dimensions.git_blame_entries_width; + let gutter_offset = rows_with_hunk_bounds + .get(&row) + .map(|bounds| bounds.origin.x + bounds.size.width); + let left_offset = blame_offset.max(gutter_offset).unwrap_or(Pixels::ZERO); - let mut x = blame_width; + let mut x = left_offset; let available_width = gutter_dimensions.margin + gutter_dimensions.left_padding - indicator_size.width - - blame_width; + - left_offset; x += available_width / 2.; let mut y = row.as_f32() * line_height - scroll_pixel_position.y; @@ -4949,13 +4962,8 @@ impl Element for EditorElement { .collect::>(); let hitbox = cx.insert_hitbox(bounds, false); - let gutter_hitbox = cx.insert_hitbox( - Bounds { - origin: bounds.origin, - size: size(gutter_dimensions.width, bounds.size.height), - }, - false, - ); + let gutter_hitbox = + cx.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false); let text_hitbox = cx.insert_hitbox( Bounds { origin: gutter_hitbox.upper_right(), @@ -5078,7 +5086,7 @@ impl Element for EditorElement { self.layout_crease_trailers(buffer_rows.iter().copied(), &snapshot, cx) }); - let display_hunks = self.layout_git_gutters( + let display_hunks = self.layout_gutter_git_hunks( line_height, &gutter_hitbox, start_row..end_row, @@ -5305,6 +5313,27 @@ impl Element for EditorElement { .collect::>() }); + let rows_with_hunk_bounds = display_hunks + .iter() + .filter_map(|(hunk, hitbox)| Some((hunk, hitbox.as_ref()?.bounds))) + .fold( + HashMap::default(), + |mut rows_with_hunk_bounds, (hunk, bounds)| { + match hunk { + DisplayDiffHunk::Folded { display_row } => { + rows_with_hunk_bounds.insert(*display_row, bounds); + } + DisplayDiffHunk::Unfolded { + display_row_range, .. + } => { + for display_row in display_row_range.iter_rows() { + rows_with_hunk_bounds.insert(display_row, bounds); + } + } + } + rows_with_hunk_bounds + }, + ); let mut _context_menu_visible = false; let mut code_actions_indicator = None; if let Some(newest_selection_head) = newest_selection_head { @@ -5353,6 +5382,7 @@ impl Element for EditorElement { scroll_pixel_position, &gutter_dimensions, &gutter_hitbox, + &rows_with_hunk_bounds, cx, ); } @@ -5368,6 +5398,7 @@ impl Element for EditorElement { scroll_pixel_position, &gutter_dimensions, &gutter_hitbox, + &rows_with_hunk_bounds, &snapshot, cx, ) @@ -5376,11 +5407,12 @@ impl Element for EditorElement { }; let close_indicators = self.layout_hunk_diff_close_indicators( - expanded_add_hunks_by_rows, line_height, scroll_pixel_position, &gutter_dimensions, &gutter_hitbox, + &rows_with_hunk_bounds, + expanded_add_hunks_by_rows, cx, ); @@ -5591,6 +5623,16 @@ impl Element for EditorElement { } } +pub(super) fn gutter_bounds( + editor_bounds: Bounds, + gutter_dimensions: GutterDimensions, +) -> Bounds { + Bounds { + origin: editor_bounds.origin, + size: size(gutter_dimensions.width, editor_bounds.size.height), + } +} + impl IntoElement for EditorElement { type Element = Self; diff --git a/crates/editor/src/hunk_diff.rs b/crates/editor/src/hunk_diff.rs index 8d70c1392d..3e0256d1da 100644 --- a/crates/editor/src/hunk_diff.rs +++ b/crates/editor/src/hunk_diff.rs @@ -5,7 +5,7 @@ use std::{ use collections::{hash_map, HashMap, HashSet}; use git::diff::{DiffHunk, DiffHunkStatus}; -use gpui::{Action, AppContext, Hsla, Model, MouseButton, Subscription, Task, View}; +use gpui::{Action, AppContext, CursorStyle, Hsla, Model, MouseButton, Subscription, Task, View}; use language::Buffer; use multi_buffer::{ Anchor, AnchorRangeExt, ExcerptRange, MultiBuffer, MultiBufferRow, MultiBufferSnapshot, ToPoint, @@ -13,7 +13,7 @@ use multi_buffer::{ use settings::SettingsStore; use text::{BufferId, Point}; use ui::{ - h_flex, v_flex, ActiveTheme, Context as _, ContextMenu, InteractiveElement, IntoElement, + div, h_flex, v_flex, ActiveTheme, Context as _, ContextMenu, InteractiveElement, IntoElement, ParentElement, Pixels, Styled, ViewContext, VisualContext, }; use util::{debug_panic, RangeExt}; @@ -24,8 +24,8 @@ use crate::{ hunk_status, hunks_for_selections, mouse_context_menu::MouseContextMenu, BlockDisposition, BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, Editor, - EditorSnapshot, ExpandAllHunkDiffs, RangeToAnchorExt, RevertSelectedHunks, ToDisplayPoint, - ToggleHunkDiff, + EditorElement, EditorSnapshot, ExpandAllHunkDiffs, RangeToAnchorExt, RevertSelectedHunks, + ToDisplayPoint, ToggleHunkDiff, }; #[derive(Debug, Clone)] @@ -430,7 +430,6 @@ impl Editor { let (editor_height, editor_with_deleted_text) = editor_with_deleted_text(diff_base_buffer, deleted_hunk_color, hunk, cx); let editor = cx.view().clone(); - let editor_model = cx.model().clone(); let hunk = hunk.clone(); let mut new_block_ids = self.insert_blocks( Some(BlockProperties { @@ -439,37 +438,84 @@ impl Editor { style: BlockStyle::Flex, disposition: BlockDisposition::Above, render: Box::new(move |cx| { - let close_button = editor.update(cx.context, |editor, cx| { - let editor_snapshot = editor.snapshot(cx); - let hunk_start_row = hunk - .multi_buffer_range - .start - .to_display_point(&editor_snapshot) - .row(); - editor.render_close_hunk_diff_button(hunk.clone(), hunk_start_row, cx) - }); - let gutter_dimensions = editor_model.read(cx).gutter_dimensions; + 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(); h_flex() + .id("gutter with editor") .bg(deleted_hunk_color) .size_full() .child( - v_flex() + h_flex() + .id("gutter") .max_w(gutter_dimensions.full_width()) .min_w(gutter_dimensions.full_width()) .size_full() - .on_mouse_down(MouseButton::Left, { - let click_hunk = hunk.clone(); - move |e, cx| { - let modifiers = e.modifiers; - if modifiers.control || modifiers.platform { - click_editor.update(cx, |editor, cx| { - editor.toggle_hovered_hunk(&click_hunk, cx); - }); - } - } - }) - .child(close_button), + .child( + h_flex() + .id("gutter hunk") + .pl(hunk_bounds.origin.x) + .max_w(hunk_bounds.size.width) + .min_w(hunk_bounds.size.width) + .size_full() + .cursor(CursorStyle::PointingHand) + .on_mouse_down(MouseButton::Left, { + let click_hunk = hunk.clone(); + move |e, cx| { + let modifiers = e.modifiers; + if modifiers.control || modifiers.platform { + click_editor.update(cx, |editor, cx| { + editor.toggle_hovered_hunk(&click_hunk, cx); + }); + } else { + click_editor.update(cx, |editor, cx| { + editor.open_hunk_context_menu( + clicked_hunk.clone(), + e.position, + cx, + ); + }); + } + } + }), + ) + .child( + v_flex() + .size_full() + .pt(ui::rems(0.25)) + .justify_start() + .child(close_button), + ), ) .child(editor_with_deleted_text.clone()) .into_any_element()