Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More git hunk highlighting fixes #18459

Merged
merged 4 commits into from
Sep 27, 2024
Merged

More git hunk highlighting fixes #18459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
274dc7c
Fix another bug in diff hunk highlighting
maxbrunsfeld Sep 27, 2024
f05845e
Use pretty assertions in editor test context assertion methods
maxbrunsfeld Sep 27, 2024
794b8c9
Fix go-to-line test
maxbrunsfeld Sep 27, 2024
91a9ff9
Adjust git hunk test to cover deleting at boundary of hunk
maxbrunsfeld Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/assistant/src/inline_assistant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ impl InlineAssistant {
editor.set_read_only(true);
editor.set_show_inline_completions(Some(false), cx);
editor.highlight_rows::<DeletedLines>(
Anchor::min()..=Anchor::max(),
Anchor::min()..Anchor::max(),
cx.theme().status().deleted_background,
false,
cx,
Expand Down Expand Up @@ -2557,7 +2557,7 @@ enum CodegenStatus {
#[derive(Default)]
struct Diff {
deleted_row_ranges: Vec<(Anchor, RangeInclusive<u32>)>,
inserted_row_ranges: Vec<RangeInclusive<Anchor>>,
inserted_row_ranges: Vec<Range<Anchor>>,
}

impl Diff {
Expand Down Expand Up @@ -3103,7 +3103,7 @@ impl CodegenAlternative {
new_end_row,
new_snapshot.line_len(MultiBufferRow(new_end_row)),
));
self.diff.inserted_row_ranges.push(start..=end);
self.diff.inserted_row_ranges.push(start..end);
new_row += lines;
}
}
Expand Down Expand Up @@ -3181,7 +3181,7 @@ impl CodegenAlternative {
new_end_row,
new_snapshot.line_len(MultiBufferRow(new_end_row)),
));
inserted_row_ranges.push(start..=end);
inserted_row_ranges.push(start..end);
new_row += line_count;
}
}
Expand Down
63 changes: 31 additions & 32 deletions crates/editor/src/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ impl SelectionHistory {

struct RowHighlight {
index: usize,
range: RangeInclusive<Anchor>,
range: Range<Anchor>,
color: Hsla,
should_autoscroll: bool,
}
Expand Down Expand Up @@ -11502,9 +11502,11 @@ impl Editor {

/// Adds a row highlight for the given range. If a row has multiple highlights, the
/// last highlight added will be used.
///
/// If the range ends at the beginning of a line, then that line will not be highlighted.
pub fn highlight_rows<T: 'static>(
&mut self,
range: RangeInclusive<Anchor>,
range: Range<Anchor>,
color: Hsla,
should_autoscroll: bool,
cx: &mut ViewContext<Self>,
Expand All @@ -11513,8 +11515,8 @@ impl Editor {
let row_highlights = self.highlighted_rows.entry(TypeId::of::<T>()).or_default();
let ix = row_highlights.binary_search_by(|highlight| {
Ordering::Equal
.then_with(|| highlight.range.start().cmp(&range.start(), &snapshot))
.then_with(|| highlight.range.end().cmp(&range.end(), &snapshot))
.then_with(|| highlight.range.start.cmp(&range.start, &snapshot))
.then_with(|| highlight.range.end.cmp(&range.end, &snapshot))
});

if let Err(mut ix) = ix {
Expand All @@ -11527,18 +11529,13 @@ impl Editor {
let prev_highlight = &mut row_highlights[ix - 1];
if prev_highlight
.range
.end()
.cmp(&range.start(), &snapshot)
.end
.cmp(&range.start, &snapshot)
.is_ge()
{
ix -= 1;
if prev_highlight
.range
.end()
.cmp(&range.end(), &snapshot)
.is_lt()
{
prev_highlight.range = *prev_highlight.range.start()..=*range.end();
if prev_highlight.range.end.cmp(&range.end, &snapshot).is_lt() {
prev_highlight.range.end = range.end;
}
merged = true;
prev_highlight.index = index;
Expand All @@ -11564,18 +11561,17 @@ impl Editor {
let highlight = &row_highlights[ix];
if next_highlight
.range
.start()
.cmp(&highlight.range.end(), &snapshot)
.start
.cmp(&highlight.range.end, &snapshot)
.is_le()
{
if next_highlight
.range
.end()
.cmp(&highlight.range.end(), &snapshot)
.end
.cmp(&highlight.range.end, &snapshot)
.is_gt()
{
row_highlights[ix].range =
*highlight.range.start()..=*next_highlight.range.end();
row_highlights[ix].range.end = next_highlight.range.end;
}
row_highlights.remove(ix + 1);
} else {
Expand All @@ -11597,15 +11593,12 @@ impl Editor {
let mut ranges_to_remove = ranges_to_remove.iter().peekable();
row_highlights.retain(|highlight| {
while let Some(range_to_remove) = ranges_to_remove.peek() {
match range_to_remove.end.cmp(&highlight.range.start(), &snapshot) {
Ordering::Less => {
match range_to_remove.end.cmp(&highlight.range.start, &snapshot) {
Ordering::Less | Ordering::Equal => {
ranges_to_remove.next();
}
Ordering::Equal => {
return false;
}
Ordering::Greater => {
match range_to_remove.start.cmp(&highlight.range.end(), &snapshot) {
match range_to_remove.start.cmp(&highlight.range.end, &snapshot) {
Ordering::Less | Ordering::Equal => {
return false;
}
Expand All @@ -11625,9 +11618,7 @@ impl Editor {
}

/// For a highlight given context type, gets all anchor ranges that will be used for row highlighting.
pub fn highlighted_rows<T: 'static>(
&self,
) -> impl '_ + Iterator<Item = (RangeInclusive<Anchor>, Hsla)> {
pub fn highlighted_rows<T: 'static>(&self) -> impl '_ + Iterator<Item = (Range<Anchor>, Hsla)> {
self.highlighted_rows
.get(&TypeId::of::<T>())
.map_or(&[] as &[_], |vec| vec.as_slice())
Expand All @@ -11650,9 +11641,17 @@ impl Editor {
.fold(
BTreeMap::<DisplayRow, Hsla>::new(),
|mut unique_rows, highlight| {
let start_row = highlight.range.start().to_display_point(&snapshot).row();
let end_row = highlight.range.end().to_display_point(&snapshot).row();
for row in start_row.0..=end_row.0 {
let start = highlight.range.start.to_display_point(&snapshot);
let end = highlight.range.end.to_display_point(&snapshot);
let start_row = start.row().0;
let end_row = if highlight.range.end.text_anchor != text::Anchor::MAX
&& end.column() == 0
{
end.row().0.saturating_sub(1)
} else {
end.row().0
};
for row in start_row..=end_row {
let used_index =
used_highlight_orders.entry(row).or_insert(highlight.index);
if highlight.index >= *used_index {
Expand All @@ -11674,7 +11673,7 @@ impl Editor {
.flat_map(|highlighted_rows| highlighted_rows.iter())
.filter_map(|highlight| {
if highlight.should_autoscroll {
Some(highlight.range.start().to_display_point(snapshot).row())
Some(highlight.range.start.to_display_point(snapshot).row())
} else {
None
}
Expand Down
11 changes: 6 additions & 5 deletions crates/editor/src/editor_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11832,7 +11832,6 @@ async fn test_edits_around_expanded_insertion_hunks(
);

cx.update_editor(|editor, cx| {
editor.move_up(&MoveUp, cx);
editor.delete_line(&DeleteLine, cx);
});
executor.run_until_parked();
Expand All @@ -11846,7 +11845,7 @@ async fn test_edits_around_expanded_insertion_hunks(
+ const B: u32 = 42;
+ const C: u32 = 42;
+ const D: u32 = 42;
+
+ const E: u32 = 42;

fn main() {
println!("hello");
Expand All @@ -11872,8 +11871,8 @@ async fn test_edits_around_expanded_insertion_hunks(
use some::mod2;

const A: u32 = 42;
const B: u32 = 42;
ˇ

fn main() {
println!("hello");

Expand All @@ -11889,8 +11888,8 @@ async fn test_edits_around_expanded_insertion_hunks(
use some::mod2;

const A: u32 = 42;
+ const B: u32 = 42;

+
fn main() {
println!("hello");

Expand All @@ -11907,7 +11906,9 @@ async fn test_edits_around_expanded_insertion_hunks(
executor.run_until_parked();
cx.assert_diff_hunks(
r#"

use some::mod1;
- use some::mod2;
-
- const A: u32 = 42;

fn main() {
Expand Down
29 changes: 5 additions & 24 deletions crates/editor/src/hunk_diff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use multi_buffer::{
Anchor, AnchorRangeExt, ExcerptRange, MultiBuffer, MultiBufferDiffHunk, MultiBufferRow,
MultiBufferSnapshot, ToPoint,
};
use std::{
ops::{Range, RangeInclusive},
sync::Arc,
};
use std::{ops::Range, sync::Arc};
use ui::{
prelude::*, ActiveTheme, ContextMenu, IconButtonShape, InteractiveElement, IntoElement,
ParentElement, PopoverMenu, Styled, Tooltip, ViewContext, VisualContext,
Expand All @@ -19,7 +16,7 @@ use util::RangeExt;
use crate::{
editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, BlockDisposition,
BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, DisplayRow, DisplaySnapshot,
Editor, EditorElement, EditorSnapshot, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertFile,
Editor, EditorElement, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertFile,
RevertSelectedHunks, ToDisplayPoint, ToggleHunkDiff,
};

Expand Down Expand Up @@ -298,7 +295,7 @@ impl Editor {
}
DiffHunkStatus::Added => {
self.highlight_rows::<DiffRowHighlight>(
to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
hunk_start..hunk_end,
added_hunk_color(cx),
false,
cx,
Expand All @@ -307,7 +304,7 @@ impl Editor {
}
DiffHunkStatus::Modified => {
self.highlight_rows::<DiffRowHighlight>(
to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
hunk_start..hunk_end,
added_hunk_color(cx),
false,
cx,
Expand Down Expand Up @@ -960,7 +957,7 @@ fn editor_with_deleted_text(
editor.set_read_only(true);
editor.set_show_inline_completions(Some(false), cx);
editor.highlight_rows::<DiffRowHighlight>(
Anchor::min()..=Anchor::max(),
Anchor::min()..Anchor::max(),
deleted_color,
false,
cx,
Expand Down Expand Up @@ -1039,22 +1036,6 @@ fn buffer_diff_hunk(
None
}

fn to_inclusive_row_range(
row_range: Range<Anchor>,
snapshot: &EditorSnapshot,
) -> RangeInclusive<Anchor> {
let mut end = row_range.end.to_point(&snapshot.buffer_snapshot);
if end.column == 0 && end.row > 0 {
end = Point::new(
end.row - 1,
snapshot
.buffer_snapshot
.line_len(MultiBufferRow(end.row - 1)),
);
}
row_range.start..=snapshot.buffer_snapshot.anchor_after(end)
}

impl DisplayDiffHunk {
pub fn start_display_row(&self) -> DisplayRow {
match self {
Expand Down
32 changes: 9 additions & 23 deletions crates/editor/src/test/editor_test_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use gpui::{
AnyWindowHandle, AppContext, Keystroke, ModelContext, Pixels, Point, View, ViewContext,
VisualTestContext, WindowHandle,
};
use indoc::indoc;
use itertools::Itertools;
use language::{Buffer, BufferSnapshot, LanguageRegistry};
use multi_buffer::{ExcerptRange, ToPoint};
Expand Down Expand Up @@ -337,8 +336,9 @@ impl EditorTestContext {
let insertions = editor
.highlighted_rows::<DiffRowHighlight>()
.map(|(range, _)| {
range.start().to_point(&snapshot.buffer_snapshot).row
..range.end().to_point(&snapshot.buffer_snapshot).row + 1
let start = range.start.to_point(&snapshot.buffer_snapshot);
let end = range.end.to_point(&snapshot.buffer_snapshot);
start.row..end.row
})
.collect::<Vec<_>>();
let deletions = editor
Expand Down Expand Up @@ -384,13 +384,8 @@ impl EditorTestContext {
/// See the `util::test::marked_text_ranges` function for more information.
#[track_caller]
pub fn assert_editor_state(&mut self, marked_text: &str) {
let (unmarked_text, expected_selections) = marked_text_ranges(marked_text, true);
let buffer_text = self.buffer_text();

if buffer_text != unmarked_text {
panic!("Unmarked text doesn't match buffer text\nBuffer text: {buffer_text:?}\nUnmarked text: {unmarked_text:?}\nRaw buffer text\n{buffer_text}\nRaw unmarked text\n{unmarked_text}");
}

let (expected_text, expected_selections) = marked_text_ranges(marked_text, true);
pretty_assertions::assert_eq!(self.buffer_text(), expected_text, "unexpected buffer text");
self.assert_selections(expected_selections, marked_text.to_string())
}

Expand Down Expand Up @@ -463,20 +458,11 @@ impl EditorTestContext {
let actual_marked_text =
generate_marked_text(&self.buffer_text(), &actual_selections, true);
if expected_selections != actual_selections {
panic!(
indoc! {"

{}Editor has unexpected selections.

Expected selections:
{}

Actual selections:
{}
"},
self.assertion_context(),
expected_marked_text,
pretty_assertions::assert_eq!(
actual_marked_text,
expected_marked_text,
"{}Editor has unexpected selections",
self.assertion_context(),
);
}
}
Expand Down
22 changes: 12 additions & 10 deletions crates/go_to_line/src/go_to_line.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,13 @@ impl GoToLine {
if let Some(point) = self.point_from_query(cx) {
self.active_editor.update(cx, |active_editor, cx| {
let snapshot = active_editor.snapshot(cx).display_snapshot;
let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left);
let anchor = snapshot.buffer_snapshot.anchor_before(point);
let start = snapshot.buffer_snapshot.clip_point(point, Bias::Left);
let end = start + Point::new(1, 0);
let start = snapshot.buffer_snapshot.anchor_before(start);
let end = snapshot.buffer_snapshot.anchor_after(end);
active_editor.clear_row_highlights::<GoToLineRowHighlights>();
active_editor.highlight_rows::<GoToLineRowHighlights>(
anchor..=anchor,
start..end,
cx.theme().colors().editor_highlighted_line_background,
true,
cx,
Expand Down Expand Up @@ -244,13 +246,13 @@ mod tests {
field_1: i32, // display line 3
field_2: i32, // display line 4
} // display line 5
// display line 7
struct Another { // display line 8
field_1: i32, // display line 9
field_2: i32, // display line 10
field_3: i32, // display line 11
field_4: i32, // display line 12
} // display line 13
// display line 6
struct Another { // display line 7
field_1: i32, // display line 8
field_2: i32, // display line 9
field_3: i32, // display line 10
field_4: i32, // display line 11
} // display line 12
"}
}),
)
Expand Down
Loading
Loading