Skip to content

Commit

Permalink
Improve diff hunks (#18283)
Browse files Browse the repository at this point in the history 
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

<img width="1624" alt="Screenshot 2024-09-24 at 11 13 26 AM"
src="https://github.com/user-attachments/assets/f864dc83-cbbc-4d81-a62b-65c406ed310a">

#### Expanded

<img width="1624" alt="Screenshot 2024-09-24 at 11 13 35 AM"
src="https://github.com/user-attachments/assets/04d382ca-e0e6-4f1c-92eb-cd1e3a031c2c">


Release Notes:

- Improved the appearance of diff hunks in the editor.

---------

Co-authored-by: Max <max@zed.dev>
  • Loading branch information
@maxdeviant @maxbrunsfeld
maxdeviant and maxbrunsfeld authored Sep 24, 2024
1 parent 2470db4 commit 21be70f
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 82 deletions.
5 changes: 0 additions & 5 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12473,11 +12473,6 @@ impl Editor {
Some(gpui::Point::new(source_x, source_y))
}

fn gutter_bounds(&self) -> Option<Bounds<Pixels>> {
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(_))
Expand Down
120 changes: 91 additions & 29 deletions crates/editor/src/element.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,7 @@ impl EditorElement {
line_height: Pixels,
gutter_hitbox: &Hitbox,
display_rows: Range<DisplayRow>,
anchor_range: Range<Anchor>,
snapshot: &EditorSnapshot,
cx: &mut WindowContext,
) -> Vec<(DisplayDiffHunk, Option<Hitbox>)> {
Expand All @@ -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)]
Expand Down Expand Up @@ -3187,20 +3242,20 @@ impl EditorElement {
Some((
hunk_bounds,
cx.theme().status().modified,
Corners::all(1. * line_height),
Corners::all(px(0.)),
))
}
DisplayDiffHunk::Unfolded { status, .. } => {
hitbox.as_ref().map(|hunk_hitbox| match status {
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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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| {
Expand Down Expand Up @@ -5158,6 +5219,7 @@ impl Element for EditorElement {
line_height,
&gutter_hitbox,
start_row..end_row,
start_anchor..end_anchor,
&snapshot,
cx,
);
Expand Down
4 changes: 2 additions & 2 deletions crates/editor/src/git.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
77 changes: 31 additions & 46 deletions crates/editor/src/hunk_diff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -38,7 +38,7 @@ pub(super) struct HoveredHunk {

#[derive(Debug, Default)]
pub(super) struct ExpandedHunks {
hunks: Vec<ExpandedHunk>,
pub(crate) hunks: Vec<ExpandedHunk>,
diff_base: HashMap<BufferId, DiffBaseBuffer>,
hunk_update_tasks: HashMap<Option<BufferId>, Task<()>>,
}
Expand Down Expand Up @@ -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)
Expand All @@ -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,
);
});
Expand Down

0 comments on commit 21be70f

Please sign in to comment.