oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rework diff rendering to allow putting the cursor into deleted text, soft-wrapping and scrolling deleted text correctly (#22994)

Browse Source 
Closes #12553

* [x] Fix `diff_hunk_before`
* [x] Fix failure to show deleted text when expanding hunk w/ cursor on
second line of the hunk
* [x] Failure to expand diff hunk below the cursor.
* [x] Delete the whole file, and expand the diff. Backspace over the
deleted hunk, panic!
* [x] Go-to-line now counts the diff hunks, but it should not
* [x] backspace at the beginning of a deleted hunk deletes too much text
* [x] Indent guides are rendered incorrectly 
* [ ] Fix randomized multi buffer tests

Maybe:
* [ ] Buffer search should include deleted text (in vim mode it turns
out I use `/x` all the time to jump to the next x I can see).
* [ ] vim: should refuse to switch into insert mode if selection is
fully within a diff.
* [ ] vim `o` command when cursor is on last line of deleted hunk.
* [ ] vim `shift-o` on first line of deleted hunk moves cursor but
doesn't insert line
* [x] `enter` at end of diff hunk inserts a new line but doesn't move
cursor
* [x] (`shift-enter` at start of diff hunk does nothing)
* [ ] Inserting a line just before an expanded hunk collapses it

Release Notes:


- Improved diff rendering, allowing you to navigate with your cursor
inside of deleted text in diff hunks.

---------

Co-authored-by: Conrad <conrad@zed.dev>
Co-authored-by: Cole <cole@zed.dev>
Co-authored-by: Mikayla <mikayla@zed.dev>
Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Co-authored-by: Michael <michael@zed.dev>
Co-authored-by: Agus <agus@zed.dev>
Co-authored-by: João <joao@zed.dev>
This commit is contained in:
Max Brunsfeld
2025-01-24 14:18:22 -07:00
committed by GitHub
co-authored by Conrad Cole Mikayla Conrad Irwin Michael Agus João
parent 1fdae4bae0
commit d2c55cbe3d
64 changed files with 7653 additions and 5495 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/editor/src/git/blame.rs
+95 -39
View File
@@ -1,5 +1,3 @@
use std::{sync::Arc, time::Duration};
use anyhow::Result;
use collections::HashMap;
use git::{
@@ -9,9 +7,10 @@ use git::{
use gpui::{AppContext, Model, ModelContext, Subscription, Task};
use http_client::HttpClient;
use language::{markdown, Bias, Buffer, BufferSnapshot, Edit, LanguageRegistry, ParsedMarkdown};
use multi_buffer::MultiBufferRow;
use multi_buffer::RowInfo;
use project::{Project, ProjectItem};
use smallvec::SmallVec;
use std::{sync::Arc, time::Duration};
use sum_tree::SumTree;
use url::Url;
@@ -194,15 +193,15 @@ impl GitBlame {
pub fn blame_for_rows<'a>(
&'a mut self,
rows: impl 'a + IntoIterator<Item = Option<MultiBufferRow>>,
rows: &'a [RowInfo],
cx: &AppContext,
) -> impl 'a + Iterator<Item = Option<BlameEntry>> {
self.sync(cx);
let mut cursor = self.entries.cursor::<u32>(&());
rows.into_iter().map(move |row| {
let row = row?;
cursor.seek_forward(&row.0, Bias::Right, &());
rows.into_iter().map(move |info| {
let row = info.buffer_row?;
cursor.seek_forward(&row, Bias::Right, &());
cursor.item()?.blame.clone()
})
}
@@ -563,15 +562,38 @@ mod tests {
use unindent::Unindent as _;
use util::RandomCharIter;
macro_rules! assert_blame_rows {
($blame:expr, $rows:expr, $expected:expr, $cx:expr) => {
assert_eq!(
$blame
.blame_for_rows($rows.map(MultiBufferRow).map(Some), $cx)
.collect::<Vec<_>>(),
$expected
);
};
// macro_rules! assert_blame_rows {
// ($blame:expr, $rows:expr, $expected:expr, $cx:expr) => {
// assert_eq!(
// $blame
// .blame_for_rows($rows.map(MultiBufferRow).map(Some), $cx)
// .collect::<Vec<_>>(),
// $expected
// );
// };
// }
#[track_caller]
fn assert_blame_rows(
blame: &mut GitBlame,
rows: Range<u32>,
expected: Vec<Option<BlameEntry>>,
cx: &mut ModelContext<GitBlame>,
) {
assert_eq!(
blame
.blame_for_rows(
&rows
.map(|row| RowInfo {
buffer_row: Some(row),
..Default::default()
})
.collect::<Vec<_>>(),
cx
)
.collect::<Vec<_>>(),
expected
);
}
fn init_test(cx: &mut gpui::TestAppContext) {
@@ -634,7 +656,15 @@ mod tests {
blame.update(cx, |blame, cx| {
assert_eq!(
blame
.blame_for_rows((0..1).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(0..1)
.map(|row| RowInfo {
buffer_row: Some(row),
..Default::default()
})
.collect::<Vec<_>>(),
cx
)
.collect::<Vec<_>>(),
vec![None]
);
@@ -698,7 +728,15 @@ mod tests {
// All lines
assert_eq!(
blame
.blame_for_rows((0..8).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(0..8)
.map(|buffer_row| RowInfo {
buffer_row: Some(buffer_row),
..Default::default()
})
.collect::<Vec<_>>(),
cx
)
.collect::<Vec<_>>(),
vec![
Some(blame_entry("1b1b1b", 0..1)),
@@ -714,7 +752,15 @@ mod tests {
// Subset of lines
assert_eq!(
blame
.blame_for_rows((1..4).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(1..4)
.map(|buffer_row| RowInfo {
buffer_row: Some(buffer_row),
..Default::default()
})
.collect::<Vec<_>>(),
cx
)
.collect::<Vec<_>>(),
vec![
Some(blame_entry("0d0d0d", 1..2)),
@@ -725,7 +771,17 @@ mod tests {
// Subset of lines, with some not displayed
assert_eq!(
blame
.blame_for_rows(vec![Some(MultiBufferRow(1)), None, None], cx)
.blame_for_rows(
&[
RowInfo {
buffer_row: Some(1),
..Default::default()
},
Default::default(),
Default::default(),
],
cx
)
.collect::<Vec<_>>(),
vec![Some(blame_entry("0d0d0d", 1..2)), None, None]
);
@@ -777,16 +833,16 @@ mod tests {
git_blame.update(cx, |blame, cx| {
// Sanity check before edits: make sure that we get the same blame entry for all
// lines.
assert_blame_rows!(
assert_blame_rows(
blame,
(0..4),
0..4,
vec![
Some(blame_entry("1b1b1b", 0..4)),
Some(blame_entry("1b1b1b", 0..4)),
Some(blame_entry("1b1b1b", 0..4)),
Some(blame_entry("1b1b1b", 0..4)),
],
cx
cx,
);
});
@@ -795,11 +851,11 @@ mod tests {
buffer.edit([(Point::new(0, 0)..Point::new(0, 0), "X")], None, cx);
});
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(
assert_blame_rows(
blame,
(0..2),
0..2,
vec![None, Some(blame_entry("1b1b1b", 0..4))],
cx
cx,
);
});
// Modify a single line, in the middle of the line
@@ -807,21 +863,21 @@ mod tests {
buffer.edit([(Point::new(1, 2)..Point::new(1, 2), "X")], None, cx);
});
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(
assert_blame_rows(
blame,
(1..4),
1..4,
vec![
None,
Some(blame_entry("1b1b1b", 0..4)),
Some(blame_entry("1b1b1b", 0..4))
Some(blame_entry("1b1b1b", 0..4)),
],
cx
cx,
);
});
// Before we insert a newline at the end, sanity check:
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(blame, (3..4), vec![Some(blame_entry("1b1b1b", 0..4))], cx);
assert_blame_rows(blame, 3..4, vec![Some(blame_entry("1b1b1b", 0..4))], cx);
});
// Insert a newline at the end
buffer.update(cx, |buffer, cx| {
@@ -829,17 +885,17 @@ mod tests {
});
// Only the new line is marked as edited:
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(
assert_blame_rows(
blame,
(3..5),
3..5,
vec![Some(blame_entry("1b1b1b", 0..4)), None],
cx
cx,
);
});
// Before we insert a newline at the start, sanity check:
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(blame, (2..3), vec![Some(blame_entry("1b1b1b", 0..4)),], cx);
assert_blame_rows(blame, 2..3, vec![Some(blame_entry("1b1b1b", 0..4))], cx);
});
// Usage example
@@ -849,11 +905,11 @@ mod tests {
});
// Only the new line is marked as edited:
git_blame.update(cx, |blame, cx| {
assert_blame_rows!(
assert_blame_rows(
blame,
(2..4),
vec![None, Some(blame_entry("1b1b1b", 0..4)),],
cx
2..4,
vec![None, Some(blame_entry("1b1b1b", 0..4))],
cx,
);
});
}
crates/editor/src/git/project_diff.rs
+22 -22
View File
@@ -146,7 +146,7 @@ impl ProjectDiffEditor {
let editor = cx.new_view(|cx| {
let mut diff_display_editor =
Editor::for_multibuffer(excerpts.clone(), Some(project.clone()), true, cx);
diff_display_editor.set_expand_all_diff_hunks();
diff_display_editor.set_expand_all_diff_hunks(cx);
diff_display_editor
});
@@ -310,9 +310,11 @@ impl ProjectDiffEditor {
.update(&mut cx, |project_diff_editor, cx| {
project_diff_editor.update_excerpts(id, new_changes, new_entry_order, cx);
project_diff_editor.editor.update(cx, |editor, cx| {
for change_set in change_sets {
editor.diff_map.add_change_set(change_set, cx)
}
editor.buffer.update(cx, |buffer, cx| {
for change_set in change_sets {
buffer.add_change_set(change_set, cx)
}
});
});
})
.ok();
@@ -1105,6 +1107,8 @@ mod tests {
path::{Path, PathBuf},
};
use crate::test::editor_test_context::assert_state_with_diff;
use super::*;
// TODO finish
@@ -1183,19 +1187,13 @@ mod tests {
let change_set = cx.new_model(|cx| {
BufferChangeSet::new_with_base_text(
old_text.clone(),
file_a_editor
.buffer()
.read(cx)
.as_singleton()
.unwrap()
.read(cx)
.text_snapshot(),
&file_a_editor.buffer().read(cx).as_singleton().unwrap(),
cx,
)
});
file_a_editor
.diff_map
.add_change_set(change_set.clone(), cx);
file_a_editor.buffer.update(cx, |buffer, cx| {
buffer.add_change_set(change_set.clone(), cx)
});
project.update(cx, |project, cx| {
project.buffer_store().update(cx, |buffer_store, cx| {
buffer_store.set_change_set(
@@ -1225,15 +1223,17 @@ mod tests {
cx.executor()
.advance_clock(UPDATE_DEBOUNCE + Duration::from_millis(100));
cx.run_until_parked();
let editor = project_diff_editor.update(cx, |view, _| view.editor.clone());
project_diff_editor.update(cx, |project_diff_editor, cx| {
assert_eq!(
// TODO assert it better: extract added text (based on the background changes) and deleted text (based on the deleted blocks added)
project_diff_editor.editor.read(cx).text(cx),
format!("{change}{old_text}"),
"Should have a new change shown in the beginning, and the old text shown as deleted text afterwards"
);
});
assert_state_with_diff(
&editor,
cx,
indoc::indoc! {
"
- This is file_a
+ an edit after git addThis is file_aˇ",
},
);
}
fn init_test(cx: &mut gpui::TestAppContext) {