Skip to content

Commit

Permalink
Rework diff rendering to allow putting the cursor into deleted text, …
Browse files Browse the repository at this point in the history
…soft-wrapping and scrolling deleted text correctly (#22994)

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 <[email protected]>
Co-authored-by: Cole <[email protected]>
Co-authored-by: Mikayla <[email protected]>
Co-authored-by: Conrad Irwin <[email protected]>
Co-authored-by: Michael <[email protected]>
Co-authored-by: Agus <[email protected]>
Co-authored-by: João <[email protected]>
  • Loading branch information
8 people authored Jan 24, 2025
1 parent 1fdae4b commit d2c55cb
Show file tree
Hide file tree
Showing 64 changed files with 7,947 additions and 5,789 deletions.
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion assets/keymaps/default-linux.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
"ctrl-alt-space": "editor::ShowCharacterPalette",
"ctrl-;": "editor::ToggleLineNumbers",
"ctrl-k ctrl-r": "editor::RevertSelectedHunks",
"ctrl-'": "editor::ToggleHunkDiff",
"ctrl-'": "editor::ToggleSelectedDiffHunks",
"ctrl-\"": "editor::ExpandAllHunkDiffs",
"ctrl-i": "editor::ShowSignatureHelp",
"alt-g b": "editor::ToggleGitBlame",
Expand Down
2 changes: 1 addition & 1 deletion assets/keymaps/default-macos.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
"ctrl-cmd-space": "editor::ShowCharacterPalette",
"cmd-;": "editor::ToggleLineNumbers",
"cmd-alt-z": "editor::RevertSelectedHunks",
"cmd-'": "editor::ToggleHunkDiff",
"cmd-'": "editor::ToggleSelectedDiffHunks",
"cmd-\"": "editor::ExpandAllHunkDiffs",
"cmd-alt-g b": "editor::ToggleGitBlame",
"cmd-i": "editor::ShowSignatureHelp",
Expand Down
2 changes: 1 addition & 1 deletion assets/keymaps/vim.json
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@
"bindings": {
"d": "vim::CurrentLine",
"s": ["vim::PushOperator", "DeleteSurrounds"],
"o": "editor::ToggleHunkDiff", // "d o"
"o": "editor::ToggleSelectedDiffHunks", // "d o"
"p": "editor::RevertSelectedHunks" // "d p"
}
},
Expand Down
35 changes: 16 additions & 19 deletions crates/assistant/src/inline_assistant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,22 +250,19 @@ impl InlineAssistant {
let newest_selection = newest_selection.unwrap();

let mut codegen_ranges = Vec::new();
for (excerpt_id, buffer, buffer_range) in
snapshot.excerpts_in_ranges(selections.iter().map(|selection| {
for (buffer, buffer_range, excerpt_id) in
snapshot.ranges_to_buffer_ranges(selections.iter().map(|selection| {
snapshot.anchor_before(selection.start)..snapshot.anchor_after(selection.end)
}))
{
let start = Anchor {
buffer_id: Some(buffer.remote_id()),
excerpt_id,
text_anchor: buffer.anchor_before(buffer_range.start),
};
let end = Anchor {
buffer_id: Some(buffer.remote_id()),
let start = buffer.anchor_before(buffer_range.start);
let end = buffer.anchor_after(buffer_range.end);

codegen_ranges.push(Anchor::range_in_buffer(
excerpt_id,
text_anchor: buffer.anchor_after(buffer_range.end),
};
codegen_ranges.push(start..end);
buffer.remote_id(),
start..end,
));

if let Some(model) = LanguageModelRegistry::read_global(cx).active_model() {
self.telemetry.report_assistant_event(AssistantEvent {
Expand Down Expand Up @@ -823,7 +820,7 @@ impl InlineAssistant {
let ranges = multibuffer_snapshot.range_to_buffer_ranges(assist.range.clone());
ranges
.first()
.and_then(|(excerpt, _)| excerpt.buffer().language())
.and_then(|(buffer, _, _)| buffer.language())
.map(|language| language.name())
});
report_assistant_event(
Expand Down Expand Up @@ -2648,17 +2645,17 @@ impl CodegenAlternative {
) -> Self {
let snapshot = multi_buffer.read(cx).snapshot(cx);

let (old_excerpt, _) = snapshot
let (buffer, _, _) = snapshot
.range_to_buffer_ranges(range.clone())
.pop()
.unwrap();
let old_buffer = cx.new_model(|cx| {
let text = old_excerpt.buffer().as_rope().clone();
let line_ending = old_excerpt.buffer().line_ending();
let language = old_excerpt.buffer().language().cloned();
let text = buffer.as_rope().clone();
let line_ending = buffer.line_ending();
let language = buffer.language().cloned();
let language_registry = multi_buffer
.read(cx)
.buffer(old_excerpt.buffer_id())
.buffer(buffer.remote_id())
.unwrap()
.read(cx)
.language_registry();
Expand Down Expand Up @@ -2898,7 +2895,7 @@ impl CodegenAlternative {
let ranges = snapshot.range_to_buffer_ranges(self.range.clone());
ranges
.first()
.and_then(|(excerpt, _)| excerpt.buffer().language())
.and_then(|(buffer, _, _)| buffer.language())
.map(|language| language.name())
};

Expand Down
12 changes: 6 additions & 6 deletions crates/assistant2/src/buffer_codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,17 +255,17 @@ impl CodegenAlternative {
) -> Self {
let snapshot = buffer.read(cx).snapshot(cx);

let (old_excerpt, _) = snapshot
let (old_buffer, _, _) = snapshot
.range_to_buffer_ranges(range.clone())
.pop()
.unwrap();
let old_buffer = cx.new_model(|cx| {
let text = old_excerpt.buffer().as_rope().clone();
let line_ending = old_excerpt.buffer().line_ending();
let language = old_excerpt.buffer().language().cloned();
let text = old_buffer.as_rope().clone();
let line_ending = old_buffer.line_ending();
let language = old_buffer.language().cloned();
let language_registry = buffer
.read(cx)
.buffer(old_excerpt.buffer_id())
.buffer(old_buffer.remote_id())
.unwrap()
.read(cx)
.language_registry();
Expand Down Expand Up @@ -475,7 +475,7 @@ impl CodegenAlternative {
let ranges = snapshot.range_to_buffer_ranges(self.range.clone());
ranges
.first()
.and_then(|(excerpt, _)| excerpt.buffer().language())
.and_then(|(buffer, _, _)| buffer.language())
.map(|language| language.name())
};

Expand Down
22 changes: 9 additions & 13 deletions crates/assistant2/src/inline_assistant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,22 +320,18 @@ impl InlineAssistant {
let newest_selection = newest_selection.unwrap();

let mut codegen_ranges = Vec::new();
for (excerpt_id, buffer, buffer_range) in
snapshot.excerpts_in_ranges(selections.iter().map(|selection| {
for (buffer, buffer_range, excerpt_id) in
snapshot.ranges_to_buffer_ranges(selections.iter().map(|selection| {
snapshot.anchor_before(selection.start)..snapshot.anchor_after(selection.end)
}))
{
let start = Anchor {
buffer_id: Some(buffer.remote_id()),
let anchor_range = Anchor::range_in_buffer(
excerpt_id,
text_anchor: buffer.anchor_before(buffer_range.start),
};
let end = Anchor {
buffer_id: Some(buffer.remote_id()),
excerpt_id,
text_anchor: buffer.anchor_after(buffer_range.end),
};
codegen_ranges.push(start..end);
buffer.remote_id(),
buffer.anchor_before(buffer_range.start)..buffer.anchor_after(buffer_range.end),
);

codegen_ranges.push(anchor_range);

if let Some(model) = LanguageModelRegistry::read_global(cx).active_model() {
self.telemetry.report_assistant_event(AssistantEvent {
Expand Down Expand Up @@ -901,7 +897,7 @@ impl InlineAssistant {
let ranges = snapshot.range_to_buffer_ranges(assist.range.clone());
ranges
.first()
.and_then(|(excerpt, _)| excerpt.buffer().language())
.and_then(|(buffer, _, _)| buffer.language())
.map(|language| language.name())
});
report_assistant_event(
Expand Down
33 changes: 28 additions & 5 deletions crates/collab/src/tests/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use editor::{
ToggleCodeActions, Undo,
},
test::editor_test_context::{AssertionContextManager, EditorTestContext},
Editor,
Editor, RowInfo,
};
use fs::Fs;
use futures::StreamExt;
Expand All @@ -20,7 +20,6 @@ use language::{
language_settings::{AllLanguageSettings, InlayHintSettings},
FakeLspAdapter,
};
use multi_buffer::MultiBufferRow;
use project::{
project_settings::{InlineBlameSettings, ProjectSettings},
SERVER_PROGRESS_THROTTLE_TIMEOUT,
Expand Down Expand Up @@ -2019,7 +2018,15 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
let blame = editor_b.blame().expect("editor_b should have blame now");
let entries = blame.update(cx, |blame, cx| {
blame
.blame_for_rows((0..4).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(0..4)
.map(|row| RowInfo {
buffer_row: Some(row),
..Default::default()
})
.collect::<Vec<_>>(),
cx,
)
.collect::<Vec<_>>()
});

Expand Down Expand Up @@ -2058,7 +2065,15 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
let blame = editor_b.blame().expect("editor_b should have blame now");
let entries = blame.update(cx, |blame, cx| {
blame
.blame_for_rows((0..4).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(0..4)
.map(|row| RowInfo {
buffer_row: Some(row),
..Default::default()
})
.collect::<Vec<_>>(),
cx,
)
.collect::<Vec<_>>()
});

Expand All @@ -2085,7 +2100,15 @@ async fn test_git_blame_is_forwarded(cx_a: &mut TestAppContext, cx_b: &mut TestA
let blame = editor_b.blame().expect("editor_b should have blame now");
let entries = blame.update(cx, |blame, cx| {
blame
.blame_for_rows((0..4).map(MultiBufferRow).map(Some), cx)
.blame_for_rows(
&(0..4)
.map(|row| RowInfo {
buffer_row: Some(row),
..Default::default()
})
.collect::<Vec<_>>(),
cx,
)
.collect::<Vec<_>>()
});

Expand Down
16 changes: 8 additions & 8 deletions crates/collab/src/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2593,7 +2593,7 @@ async fn test_git_diff_base_change(
change_set_local_a.read_with(cx_a, |change_set, cx| {
let buffer = buffer_local_a.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(diff_base.as_str())
);
git::diff::assert_hunks(
Expand Down Expand Up @@ -2621,7 +2621,7 @@ async fn test_git_diff_base_change(
change_set_remote_a.read_with(cx_b, |change_set, cx| {
let buffer = buffer_remote_a.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(diff_base.as_str())
);
git::diff::assert_hunks(
Expand All @@ -2643,7 +2643,7 @@ async fn test_git_diff_base_change(
change_set_local_a.read_with(cx_a, |change_set, cx| {
let buffer = buffer_local_a.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(new_diff_base.as_str())
);
git::diff::assert_hunks(
Expand All @@ -2657,7 +2657,7 @@ async fn test_git_diff_base_change(
change_set_remote_a.read_with(cx_b, |change_set, cx| {
let buffer = buffer_remote_a.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(new_diff_base.as_str())
);
git::diff::assert_hunks(
Expand Down Expand Up @@ -2703,7 +2703,7 @@ async fn test_git_diff_base_change(
change_set_local_b.read_with(cx_a, |change_set, cx| {
let buffer = buffer_local_b.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(diff_base.as_str())
);
git::diff::assert_hunks(
Expand All @@ -2730,7 +2730,7 @@ async fn test_git_diff_base_change(
change_set_remote_b.read_with(cx_b, |change_set, cx| {
let buffer = buffer_remote_b.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(diff_base.as_str())
);
git::diff::assert_hunks(
Expand All @@ -2752,7 +2752,7 @@ async fn test_git_diff_base_change(
change_set_local_b.read_with(cx_a, |change_set, cx| {
let buffer = buffer_local_b.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(new_diff_base.as_str())
);
git::diff::assert_hunks(
Expand All @@ -2766,7 +2766,7 @@ async fn test_git_diff_base_change(
change_set_remote_b.read_with(cx_b, |change_set, cx| {
let buffer = buffer_remote_b.read(cx);
assert_eq!(
change_set.base_text_string(cx).as_deref(),
change_set.base_text_string().as_deref(),
Some(new_diff_base.as_str())
);
git::diff::assert_hunks(
Expand Down
4 changes: 2 additions & 2 deletions crates/collab/src/tests/random_project_collaboration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ impl RandomizedTest for ProjectCollaborationTest {
.get_unstaged_changes(host_buffer.read(cx).remote_id())
.unwrap()
.read(cx)
.base_text_string(cx)
.base_text_string()
});
let guest_diff_base = guest_project.read_with(client_cx, |project, cx| {
project
Expand All @@ -1351,7 +1351,7 @@ impl RandomizedTest for ProjectCollaborationTest {
.get_unstaged_changes(guest_buffer.read(cx).remote_id())
.unwrap()
.read(cx)
.base_text_string(cx)
.base_text_string()
});
assert_eq!(
guest_diff_base, host_diff_base,
Expand Down
10 changes: 3 additions & 7 deletions crates/diagnostics/src/items.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::time::Duration;

use editor::{AnchorRangeExt, Editor};
use editor::Editor;
use gpui::{
EventEmitter, IntoElement, ParentElement, Render, Styled, Subscription, Task, View,
ViewContext, WeakView,
};
use language::{Diagnostic, DiagnosticEntry};
use language::Diagnostic;
use ui::{h_flex, prelude::*, Button, ButtonLike, Color, Icon, IconName, Label, Tooltip};
use workspace::{item::ItemHandle, StatusItemView, ToolbarItemEvent, Workspace};

Expand Down Expand Up @@ -148,11 +148,7 @@ impl DiagnosticIndicator {
(buffer, cursor_position)
});
let new_diagnostic = buffer
.diagnostics_in_range(cursor_position..cursor_position, false)
.map(|DiagnosticEntry { diagnostic, range }| DiagnosticEntry {
diagnostic,
range: range.to_offset(&buffer),
})
.diagnostics_in_range::<_, usize>(cursor_position..cursor_position)
.filter(|entry| !entry.range.is_empty())
.min_by_key(|entry| (entry.diagnostic.severity, entry.range.len()))
.map(|entry| entry.diagnostic);
Expand Down
2 changes: 1 addition & 1 deletion crates/editor/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ gpui::actions!(
ToggleAutoSignatureHelp,
ToggleGitBlame,
ToggleGitBlameInline,
ToggleHunkDiff,
ToggleSelectedDiffHunks,
ToggleIndentGuides,
ToggleInlayHints,
ToggleInlineCompletions,
Expand Down
Loading

0 comments on commit d2c55cb

Please sign in to comment.