Add an apply button to hunks in proposed changes editor (#18592)

Max Brunsfeld , Antonio , and Nathan created

Release Notes:

- N/A

---------

Co-authored-by: Antonio <antonio@zed.dev>
Co-authored-by: Nathan <nathan@zed.dev>

Change summary

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(-)

Detailed changes

crates/editor/src/actions.rs 🔗

@@ -193,6 +193,7 @@ gpui::actions!(
         AcceptPartialInlineCompletion,
         AddSelectionAbove,
         AddSelectionBelow,
+        ApplyDiffHunk,
         Backspace,
         Cancel,
         CancelLanguageServerWork,

crates/editor/src/editor.rs 🔗

@@ -6205,6 +6205,20 @@ impl Editor {
         }
     }
 
+    fn apply_selected_diff_hunks(&mut self, _: &ApplyDiffHunk, cx: &mut ViewContext<Self>) {
+        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<Self>) {
         if let Some(working_directory) = self.active_excerpt(cx).and_then(|(_, buffer, _)| {
             let project_path = buffer.read(cx).project_path(cx)?;

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)
     }
 

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::<DiffRowHighlight>(
-                    hunk_start..hunk_end,
+                    hunk_range.clone(),
                     added_hunk_color(cx),
                     false,
                     cx,
@@ -304,7 +299,7 @@ impl Editor {
             }
             DiffHunkStatus::Modified => {
                 self.highlight_rows::<DiffRowHighlight>(
-                    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<Anchor>,
+        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<Self>) {
+        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<Anchor> {
+        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<Self>) {
+        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<Self>) {
+        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(

crates/editor/src/proposed_changes_editor.rs 🔗

@@ -18,7 +18,7 @@ pub struct ProposedChangesEditor {
     editor: View<Editor>,
     _subscriptions: Vec<Subscription>,
     _recalculate_diffs_task: Task<Option<()>>,
-    recalculate_diffs_tx: mpsc::UnboundedSender<Model<Buffer>>,
+    recalculate_diffs_tx: mpsc::UnboundedSender<RecalculateDiff>,
 }
 
 pub struct ProposedChangesBuffer<T> {
@@ -30,6 +30,11 @@ pub struct ProposedChangesEditorToolbar {
     current_editor: Option<View<ProposedChangesEditor>>,
 }
 
+struct RecalculateDiff {
+    buffer: Model<Buffer>,
+    debounce: bool,
+}
+
 impl ProposedChangesEditor {
     pub fn new<T: ToOffset>(
         buffers: Vec<ProposedChangesBuffer<T>>,
@@ -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<Self>,
     ) {
-        if let BufferEvent::Edited = event {
-            self.recalculate_diffs_tx.unbounded_send(buffer).ok();
-        }
-    }
-
-    fn apply_all_changes(&self, cx: &mut ViewContext<Self>) {
-        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);
+                    })
                 });
             }
         })

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<Self>,
-        range: Option<Range<Anchor>>,
-        cx: &mut ModelContext<Self>,
-    ) {
-        let edits = branch.read_with(cx, |branch, _| {
-            branch
-                .edits_since_in_range::<usize>(
-                    &self.version,
-                    range.unwrap_or(Anchor::MIN..Anchor::MAX),
-                )
-                .map(|edit| {
-                    (
-                        edit.old,
-                        branch.text_for_range(edit.new).collect::<String>(),
-                    )
+    /// 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<Range<usize>>, cx: &mut ModelContext<Self>) {
+        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::<usize>(&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::<String>()))
+                    } else {
+                        None
+                    }
                 })
-                .collect::<Vec<_>>()
-        });
-        let operation = self.edit(edits, None, cx);
+                .collect::<Vec<_>>();
+
+            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);
         });
     }
 

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<Buffer>, cx: &mut TestAppContext) {
     buffer
         .update(cx, |buffer, cx| buffer.recalculate_diff(cx).unwrap())