From d14e36b3231071d2da7492bc89e12b6f964fee3a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 1 Oct 2024 11:07:52 -0600 Subject: [PATCH] Add an apply button to hunks in proposed changes editor (#18592) Release Notes: - N/A --------- Co-authored-by: Antonio Co-authored-by: Nathan --- crates/editor/src/actions.rs | 1 + crates/editor/src/editor.rs | 14 + crates/editor/src/element.rs | 1 + crates/editor/src/hunk_diff.rs | 272 +++++++++++-------- crates/editor/src/proposed_changes_editor.rs | 52 ++-- crates/language/src/buffer.rs | 55 ++-- crates/language/src/buffer_tests.rs | 16 +- 7 files changed, 248 insertions(+), 163 deletions(-) diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index b5935782580ba..502b70361b4f8 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -193,6 +193,7 @@ gpui::actions!( AcceptPartialInlineCompletion, AddSelectionAbove, AddSelectionBelow, + ApplyDiffHunk, Backspace, Cancel, CancelLanguageServerWork, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 61a47d7f631fd..b43433e3f41e6 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6205,6 +6205,20 @@ impl Editor { } } + fn apply_selected_diff_hunks(&mut self, _: &ApplyDiffHunk, cx: &mut ViewContext) { + let snapshot = self.buffer.read(cx).snapshot(cx); + let hunks = hunks_for_selections(&snapshot, &self.selections.disjoint_anchors()); + self.transact(cx, |editor, cx| { + for hunk in hunks { + if let Some(buffer) = editor.buffer.read(cx).buffer(hunk.buffer_id) { + buffer.update(cx, |buffer, cx| { + buffer.merge_into_base(Some(hunk.buffer_range.to_offset(buffer)), cx); + }); + } + } + }); + } + pub fn open_active_item_in_terminal(&mut self, _: &OpenInTerminal, cx: &mut ViewContext) { if let Some(working_directory) = self.active_excerpt(cx).and_then(|(_, buffer, _)| { let project_path = buffer.read(cx).project_path(cx)?; diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 98a5ff7f4dff5..8a0735354720d 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -436,6 +436,7 @@ impl EditorElement { register_action(view, cx, Editor::accept_inline_completion); register_action(view, cx, Editor::revert_file); register_action(view, cx, Editor::revert_selected_hunks); + register_action(view, cx, Editor::apply_selected_diff_hunks); register_action(view, cx, Editor::open_active_item_in_terminal) } diff --git a/crates/editor/src/hunk_diff.rs b/crates/editor/src/hunk_diff.rs index 2ccd60c668de7..c8caa30b59c49 100644 --- a/crates/editor/src/hunk_diff.rs +++ b/crates/editor/src/hunk_diff.rs @@ -14,9 +14,9 @@ use ui::{ use util::RangeExt; use crate::{ - editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, BlockDisposition, - BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, DisplayRow, DisplaySnapshot, - Editor, EditorElement, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertFile, + editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, ApplyDiffHunk, + BlockDisposition, BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, DisplayRow, + DisplaySnapshot, Editor, EditorElement, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertFile, RevertSelectedHunks, ToDisplayPoint, ToggleHunkDiff, }; @@ -238,19 +238,14 @@ impl Editor { cx: &mut ViewContext<'_, Editor>, ) -> Option<()> { let multi_buffer_snapshot = self.buffer().read(cx).snapshot(cx); - let multi_buffer_row_range = hunk - .multi_buffer_range - .start - .to_point(&multi_buffer_snapshot) - ..hunk.multi_buffer_range.end.to_point(&multi_buffer_snapshot); - let hunk_start = hunk.multi_buffer_range.start; - let hunk_end = hunk.multi_buffer_range.end; + let hunk_range = hunk.multi_buffer_range.clone(); + let hunk_point_range = hunk_range.to_point(&multi_buffer_snapshot); let buffer = self.buffer().clone(); let snapshot = self.snapshot(cx); let (diff_base_buffer, deleted_text_lines) = buffer.update(cx, |buffer, cx| { - let hunk = buffer_diff_hunk(&snapshot.buffer_snapshot, multi_buffer_row_range.clone())?; - let mut buffer_ranges = buffer.range_to_buffer_ranges(multi_buffer_row_range, cx); + let hunk = buffer_diff_hunk(&snapshot.buffer_snapshot, hunk_point_range.clone())?; + let mut buffer_ranges = buffer.range_to_buffer_ranges(hunk_point_range, cx); if buffer_ranges.len() == 1 { let (buffer, _, _) = buffer_ranges.pop()?; let diff_base_buffer = diff_base_buffer @@ -275,7 +270,7 @@ impl Editor { probe .hunk_range .start - .cmp(&hunk_start, &multi_buffer_snapshot) + .cmp(&hunk_range.start, &multi_buffer_snapshot) }) { Ok(_already_present) => return None, Err(ix) => ix, @@ -295,7 +290,7 @@ impl Editor { } DiffHunkStatus::Added => { self.highlight_rows::( - hunk_start..hunk_end, + hunk_range.clone(), added_hunk_color(cx), false, cx, @@ -304,7 +299,7 @@ impl Editor { } DiffHunkStatus::Modified => { self.highlight_rows::( - hunk_start..hunk_end, + hunk_range.clone(), added_hunk_color(cx), false, cx, @@ -323,7 +318,7 @@ impl Editor { block_insert_index, ExpandedHunk { blocks, - hunk_range: hunk_start..hunk_end, + hunk_range, status: hunk.status, folded: false, diff_base_byte_range: hunk.diff_base_byte_range.clone(), @@ -333,11 +328,47 @@ impl Editor { Some(()) } + fn apply_changes_in_range( + &mut self, + range: Range, + cx: &mut ViewContext<'_, Editor>, + ) -> Option<()> { + let (buffer, range, _) = self + .buffer + .read(cx) + .range_to_buffer_ranges(range, cx) + .into_iter() + .next()?; + + buffer.update(cx, |branch_buffer, cx| { + branch_buffer.merge_into_base(Some(range), cx); + }); + + None + } + + pub(crate) fn apply_all_changes(&self, cx: &mut ViewContext) { + let buffers = self.buffer.read(cx).all_buffers(); + for branch_buffer in buffers { + branch_buffer.update(cx, |branch_buffer, cx| { + branch_buffer.merge_into_base(None, cx); + }); + } + } + fn hunk_header_block( &self, hunk: &HoveredHunk, cx: &mut ViewContext<'_, Editor>, ) -> BlockProperties { + let is_branch_buffer = self + .buffer + .read(cx) + .point_to_buffer_offset(hunk.multi_buffer_range.start, cx) + .map_or(false, |(buffer, _, _)| { + buffer.read(cx).diff_base_buffer().is_some() + }); + let border_color = cx.theme().colors().border_variant; let gutter_color = match hunk.status { DiffHunkStatus::Added => cx.theme().status().created, @@ -388,6 +419,10 @@ impl Editor { .pr_6() .size_full() .justify_between() + .border_t_1() + .pl_6() + .pr_6() + .border_color(border_color) .child( h_flex() .gap_1() @@ -411,43 +446,10 @@ impl Editor { let hunk = hunk.clone(); move |_event, cx| { editor.update(cx, |editor, cx| { - let snapshot = editor.snapshot(cx); - let position = hunk - .multi_buffer_range - .end - .to_point( - &snapshot.buffer_snapshot, - ); - if let Some(hunk) = editor - .go_to_hunk_after_position( - &snapshot, position, cx, - ) - { - let multi_buffer_start = snapshot - .buffer_snapshot - .anchor_before(Point::new( - hunk.row_range.start.0, - 0, - )); - let multi_buffer_end = snapshot - .buffer_snapshot - .anchor_after(Point::new( - hunk.row_range.end.0, - 0, - )); - editor.expand_diff_hunk( - None, - &HoveredHunk { - multi_buffer_range: - multi_buffer_start - ..multi_buffer_end, - status: hunk_status(&hunk), - diff_base_byte_range: hunk - .diff_base_byte_range, - }, - cx, - ); - } + editor.go_to_subsequent_hunk( + hunk.multi_buffer_range.end, + cx, + ); }); } }), @@ -472,43 +474,10 @@ impl Editor { let hunk = hunk.clone(); move |_event, cx| { editor.update(cx, |editor, cx| { - let snapshot = editor.snapshot(cx); - let position = hunk - .multi_buffer_range - .start - .to_point( - &snapshot.buffer_snapshot, - ); - let hunk = editor - .go_to_hunk_before_position( - &snapshot, position, cx, - ); - if let Some(hunk) = hunk { - let multi_buffer_start = snapshot - .buffer_snapshot - .anchor_before(Point::new( - hunk.row_range.start.0, - 0, - )); - let multi_buffer_end = snapshot - .buffer_snapshot - .anchor_after(Point::new( - hunk.row_range.end.0, - 0, - )); - editor.expand_diff_hunk( - None, - &HoveredHunk { - multi_buffer_range: - multi_buffer_start - ..multi_buffer_end, - status: hunk_status(&hunk), - diff_base_byte_range: hunk - .diff_base_byte_range, - }, - cx, - ); - } + editor.go_to_preceding_hunk( + hunk.multi_buffer_range.start, + cx, + ); }); } }), @@ -558,6 +527,36 @@ impl Editor { } }), ) + .when(is_branch_buffer, |this| { + this.child( + IconButton::new("apply", IconName::Check) + .shape(IconButtonShape::Square) + .icon_size(IconSize::Small) + .tooltip({ + let focus_handle = editor.focus_handle(cx); + move |cx| { + Tooltip::for_action_in( + "Apply Hunk", + &ApplyDiffHunk, + &focus_handle, + cx, + ) + } + }) + .on_click({ + let editor = editor.clone(); + let hunk = hunk.clone(); + move |_event, cx| { + editor.update(cx, |editor, cx| { + editor.apply_changes_in_range( + hunk.multi_buffer_range.clone(), + cx, + ); + }); + } + }), + ) + }) .child({ let focus = editor.focus_handle(cx); PopoverMenu::new("hunk-controls-dropdown") @@ -597,31 +596,29 @@ impl Editor { }), ) .child( - div().child( - IconButton::new("collapse", IconName::Close) - .shape(IconButtonShape::Square) - .icon_size(IconSize::Small) - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |cx| { - Tooltip::for_action_in( - "Collapse Hunk", - &ToggleHunkDiff, - &focus_handle, - cx, - ) - } - }) - .on_click({ - let editor = editor.clone(); - let hunk = hunk.clone(); - move |_event, cx| { - editor.update(cx, |editor, cx| { - editor.toggle_hovered_hunk(&hunk, cx); - }); - } - }), - ), + IconButton::new("collapse", IconName::Close) + .shape(IconButtonShape::Square) + .icon_size(IconSize::Small) + .tooltip({ + let focus_handle = editor.focus_handle(cx); + move |cx| { + Tooltip::for_action_in( + "Collapse Hunk", + &ToggleHunkDiff, + &focus_handle, + cx, + ) + } + }) + .on_click({ + let editor = editor.clone(); + let hunk = hunk.clone(); + move |_event, cx| { + editor.update(cx, |editor, cx| { + editor.toggle_hovered_hunk(&hunk, cx); + }); + } + }), ), ) .into_any_element() @@ -876,6 +873,51 @@ impl Editor { } }) } + + fn go_to_subsequent_hunk(&mut self, position: Anchor, cx: &mut ViewContext) { + let snapshot = self.snapshot(cx); + let position = position.to_point(&snapshot.buffer_snapshot); + if let Some(hunk) = self.go_to_hunk_after_position(&snapshot, position, cx) { + let multi_buffer_start = snapshot + .buffer_snapshot + .anchor_before(Point::new(hunk.row_range.start.0, 0)); + let multi_buffer_end = snapshot + .buffer_snapshot + .anchor_after(Point::new(hunk.row_range.end.0, 0)); + self.expand_diff_hunk( + None, + &HoveredHunk { + multi_buffer_range: multi_buffer_start..multi_buffer_end, + status: hunk_status(&hunk), + diff_base_byte_range: hunk.diff_base_byte_range, + }, + cx, + ); + } + } + + fn go_to_preceding_hunk(&mut self, position: Anchor, cx: &mut ViewContext) { + let snapshot = self.snapshot(cx); + let position = position.to_point(&snapshot.buffer_snapshot); + let hunk = self.go_to_hunk_before_position(&snapshot, position, cx); + if let Some(hunk) = hunk { + let multi_buffer_start = snapshot + .buffer_snapshot + .anchor_before(Point::new(hunk.row_range.start.0, 0)); + let multi_buffer_end = snapshot + .buffer_snapshot + .anchor_after(Point::new(hunk.row_range.end.0, 0)); + self.expand_diff_hunk( + None, + &HoveredHunk { + multi_buffer_range: multi_buffer_start..multi_buffer_end, + status: hunk_status(&hunk), + diff_base_byte_range: hunk.diff_base_byte_range, + }, + cx, + ); + } + } } fn to_diff_hunk( diff --git a/crates/editor/src/proposed_changes_editor.rs b/crates/editor/src/proposed_changes_editor.rs index 0666346e48776..62e37bc677f5f 100644 --- a/crates/editor/src/proposed_changes_editor.rs +++ b/crates/editor/src/proposed_changes_editor.rs @@ -18,7 +18,7 @@ pub struct ProposedChangesEditor { editor: View, _subscriptions: Vec, _recalculate_diffs_task: Task>, - recalculate_diffs_tx: mpsc::UnboundedSender>, + recalculate_diffs_tx: mpsc::UnboundedSender, } pub struct ProposedChangesBuffer { @@ -30,6 +30,11 @@ pub struct ProposedChangesEditorToolbar { current_editor: Option>, } +struct RecalculateDiff { + buffer: Model, + debounce: bool, +} + impl ProposedChangesEditor { pub fn new( buffers: Vec>, @@ -63,16 +68,18 @@ impl ProposedChangesEditor { recalculate_diffs_tx, _recalculate_diffs_task: cx.spawn(|_, mut cx| async move { let mut buffers_to_diff = HashSet::default(); - while let Some(buffer) = recalculate_diffs_rx.next().await { - buffers_to_diff.insert(buffer); + while let Some(mut recalculate_diff) = recalculate_diffs_rx.next().await { + buffers_to_diff.insert(recalculate_diff.buffer); - loop { + while recalculate_diff.debounce { cx.background_executor() .timer(Duration::from_millis(250)) .await; let mut had_further_changes = false; - while let Ok(next_buffer) = recalculate_diffs_rx.try_next() { - buffers_to_diff.insert(next_buffer?); + while let Ok(next_recalculate_diff) = recalculate_diffs_rx.try_next() { + let next_recalculate_diff = next_recalculate_diff?; + recalculate_diff.debounce &= next_recalculate_diff.debounce; + buffers_to_diff.insert(next_recalculate_diff.buffer); had_further_changes = true; } if !had_further_changes { @@ -99,19 +106,24 @@ impl ProposedChangesEditor { event: &BufferEvent, _cx: &mut ViewContext, ) { - if let BufferEvent::Edited = event { - self.recalculate_diffs_tx.unbounded_send(buffer).ok(); - } - } - - fn apply_all_changes(&self, cx: &mut ViewContext) { - let buffers = self.editor.read(cx).buffer.read(cx).all_buffers(); - for branch_buffer in buffers { - if let Some(base_buffer) = branch_buffer.read(cx).diff_base_buffer() { - base_buffer.update(cx, |base_buffer, cx| { - base_buffer.merge(&branch_buffer, None, cx) - }); + match event { + BufferEvent::Operation { .. } => { + self.recalculate_diffs_tx + .unbounded_send(RecalculateDiff { + buffer, + debounce: true, + }) + .ok(); + } + BufferEvent::DiffBaseChanged => { + self.recalculate_diffs_tx + .unbounded_send(RecalculateDiff { + buffer, + debounce: false, + }) + .ok(); } + _ => (), } } } @@ -208,7 +220,9 @@ impl Render for ProposedChangesEditorToolbar { Button::new("apply-changes", "Apply All").on_click(move |_, cx| { if let Some(editor) = &editor { editor.update(cx, |editor, cx| { - editor.apply_all_changes(cx); + editor.editor.update(cx, |editor, cx| { + editor.apply_all_changes(cx); + }) }); } }) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 7abc9b8dba146..8afc4d389db7f 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -62,7 +62,7 @@ pub use text::{ use theme::SyntaxTheme; #[cfg(any(test, feature = "test-support"))] use util::RandomCharIter; -use util::RangeExt; +use util::{debug_panic, RangeExt}; #[cfg(any(test, feature = "test-support"))] pub use {tree_sitter_rust, tree_sitter_typescript}; @@ -823,40 +823,41 @@ impl Buffer { }) } - /// Applies all of the changes in `branch` buffer that intersect the given `range` - /// to this buffer. - pub fn merge( - &mut self, - branch: &Model, - range: Option>, - cx: &mut ModelContext, - ) { - let edits = branch.read_with(cx, |branch, _| { - branch - .edits_since_in_range::( - &self.version, - range.unwrap_or(Anchor::MIN..Anchor::MAX), - ) - .map(|edit| { - ( - edit.old, - branch.text_for_range(edit.new).collect::(), - ) + /// Applies all of the changes in this buffer that intersect the given `range` + /// to its base buffer. This buffer must be a branch buffer to call this method. + pub fn merge_into_base(&mut self, range: Option>, cx: &mut ModelContext) { + let Some(base_buffer) = self.diff_base_buffer() else { + debug_panic!("not a branch buffer"); + return; + }; + + base_buffer.update(cx, |base_buffer, cx| { + let edits = self + .edits_since::(&base_buffer.version) + .filter_map(|edit| { + if range + .as_ref() + .map_or(true, |range| range.overlaps(&edit.new)) + { + Some((edit.old, self.text_for_range(edit.new).collect::())) + } else { + None + } }) - .collect::>() - }); - let operation = self.edit(edits, None, cx); + .collect::>(); + + let operation = base_buffer.edit(edits, None, cx); - // Prevent this operation from being reapplied to the branch. - branch.update(cx, |branch, cx| { + // Prevent this operation from being reapplied to the branch. if let Some(BufferDiffBase::PastBufferVersion { operations_to_ignore, .. - }) = &mut branch.diff_base + }) = &mut self.diff_base { operations_to_ignore.extend(operation); } - cx.emit(BufferEvent::Edited) + + cx.emit(BufferEvent::DiffBaseChanged); }); } diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 49cc31067b93a..da53d5a7637b9 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -2471,8 +2471,8 @@ fn test_branch_and_merge(cx: &mut TestAppContext) { }); // Merging the branch applies all of its changes to the base. - base_buffer.update(cx, |base_buffer, cx| { - base_buffer.merge(&branch_buffer, None, cx); + branch_buffer.update(cx, |branch_buffer, cx| { + branch_buffer.merge_into_base(None, cx); }); branch_buffer.update(cx, |branch_buffer, cx| { @@ -2484,6 +2484,18 @@ fn test_branch_and_merge(cx: &mut TestAppContext) { }); } +#[gpui::test] +fn test_merge_into_base(cx: &mut AppContext) { + init_settings(cx, |_| {}); + let base = cx.new_model(|cx| Buffer::local("abcdefghijk", cx)); + let branch = base.update(cx, |buffer, cx| buffer.branch(cx)); + branch.update(cx, |branch, cx| { + branch.edit([(0..3, "ABC"), (7..9, "HI")], None, cx); + branch.merge_into_base(Some(5..8), cx); + }); + assert_eq!(base.read(cx).text(), "abcdefgHIjk"); +} + fn start_recalculating_diff(buffer: &Model, cx: &mut TestAppContext) { buffer .update(cx, |buffer, cx| buffer.recalculate_diff(cx).unwrap())