Styling for Apply/Discard buttons (#21017)

Richard Feldman , Max , Danilo Leal , Danilo Leal , and Bennet Bo Fenner created

Change the "Apply" and "Discard" buttons to match @danilo-leal's design!
Here are some different states:

### Cursor in the first hunk

Now that the cursor is in a particular hunk, we show the "Apply" and
"Discard" names, and the keyboard shortcut. If I press the keyboard
shortcut, it will only apply to this hunk.

<img width="759" alt="Screenshot 2024-11-23 at 10 54 45 PM"
src="https://github.com/user-attachments/assets/68e0f109-9493-4ca2-a99c-dfcbb4d1ce0c">

### Cursor in the second hunk

Moving the cursor to a different hunk changes which buttons get the
keyboard shortcut treatment. Now the keyboard shortcut is shown next to
the hunk that will actually be affected if you press that shortcut.

<img width="749" alt="Screenshot 2024-11-23 at 10 56 27 PM"
src="https://github.com/user-attachments/assets/59c2ace3-6972-4a60-b806-f45e8c25eaae">


Release Notes:

- Restyled Apply/Discard buttons

---------

Co-authored-by: Max <max@zed.dev>
Co-authored-by: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: Bennet Bo Fenner <53836821+bennetbo@users.noreply.github.com>

Change summary

assets/keymaps/default-linux.json            |   2 
assets/keymaps/default-macos.json            |   2 
crates/editor/src/actions.rs                 |   2 
crates/editor/src/editor.rs                  |  64 +-
crates/editor/src/editor_tests.rs            |   2 
crates/editor/src/element.rs                 |  23 
crates/editor/src/hunk_diff.rs               | 479 +++++++++++----------
crates/editor/src/proposed_changes_editor.rs | 118 ++++
crates/zed/src/zed.rs                        |   8 
9 files changed, 409 insertions(+), 291 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -522,7 +522,7 @@
   {
     "context": "ProposedChangesEditor",
     "bindings": {
-      "ctrl-shift-y": "editor::ApplyDiffHunk",
+      "ctrl-shift-y": "editor::ApplySelectedDiffHunks",
       "ctrl-alt-a": "editor::ApplyAllDiffHunks"
     }
   },

assets/keymaps/default-macos.json 🔗

@@ -562,7 +562,7 @@
   {
     "context": "ProposedChangesEditor",
     "bindings": {
-      "cmd-shift-y": "editor::ApplyDiffHunk",
+      "cmd-shift-y": "editor::ApplySelectedDiffHunks",
       "cmd-shift-a": "editor::ApplyAllDiffHunks"
     }
   },

crates/editor/src/actions.rs 🔗

@@ -209,7 +209,7 @@ gpui::actions!(
         AddSelectionAbove,
         AddSelectionBelow,
         ApplyAllDiffHunks,
-        ApplyDiffHunk,
+        ApplySelectedDiffHunks,
         Backspace,
         Cancel,
         CancelLanguageServerWork,

crates/editor/src/editor.rs 🔗

@@ -99,7 +99,8 @@ use language::{
 use language::{point_to_lsp, BufferRow, CharClassifier, Runnable, RunnableRange};
 use linked_editing_ranges::refresh_linked_ranges;
 pub use proposed_changes_editor::{
-    ProposedChangeLocation, ProposedChangesEditor, ProposedChangesEditorToolbar,
+    ProposedChangeLocation, ProposedChangesEditor, ProposedChangesToolbar,
+    ProposedChangesToolbarControls,
 };
 use similar::{ChangeTag, TextDiff};
 use std::iter::Peekable;
@@ -160,7 +161,7 @@ use theme::{
 };
 use ui::{
     h_flex, prelude::*, ButtonSize, ButtonStyle, Disclosure, IconButton, IconName, IconSize,
-    ListItem, Popover, PopoverMenuHandle, Tooltip,
+    ListItem, Popover, Tooltip,
 };
 use util::{defer, maybe, post_inc, RangeExt, ResultExt, TryFutureExt};
 use workspace::item::{ItemHandle, PreviewTabsSettings};
@@ -590,7 +591,6 @@ pub struct Editor {
     nav_history: Option<ItemNavHistory>,
     context_menu: RwLock<Option<ContextMenu>>,
     mouse_context_menu: Option<MouseContextMenu>,
-    hunk_controls_menu_handle: PopoverMenuHandle<ui::ContextMenu>,
     completion_tasks: Vec<(CompletionId, Task<Option<()>>)>,
     signature_help_state: SignatureHelpState,
     auto_signature_help: Option<bool>,
@@ -2112,7 +2112,6 @@ impl Editor {
             nav_history: None,
             context_menu: RwLock::new(None),
             mouse_context_menu: None,
-            hunk_controls_menu_handle: PopoverMenuHandle::default(),
             completion_tasks: Default::default(),
             signature_help_state: SignatureHelpState::default(),
             auto_signature_help: None,
@@ -13558,20 +13557,24 @@ fn test_wrap_with_prefix() {
     );
 }
 
+fn is_hunk_selected(hunk: &MultiBufferDiffHunk, selections: &[Selection<Point>]) -> bool {
+    let mut buffer_rows_for_selections = selections.iter().map(|selection| {
+        let start = MultiBufferRow(selection.start.row);
+        let end = MultiBufferRow(selection.end.row);
+        start..end
+    });
+
+    buffer_rows_for_selections.any(|range| does_selection_touch_hunk(&range, hunk))
+}
+
 fn hunks_for_selections(
     multi_buffer_snapshot: &MultiBufferSnapshot,
     selections: &[Selection<Anchor>],
 ) -> Vec<MultiBufferDiffHunk> {
     let buffer_rows_for_selections = selections.iter().map(|selection| {
-        let head = selection.head();
-        let tail = selection.tail();
-        let start = MultiBufferRow(tail.to_point(multi_buffer_snapshot).row);
-        let end = MultiBufferRow(head.to_point(multi_buffer_snapshot).row);
-        if start > end {
-            end..start
-        } else {
-            start..end
-        }
+        let start = MultiBufferRow(selection.start.to_point(multi_buffer_snapshot).row);
+        let end = MultiBufferRow(selection.end.to_point(multi_buffer_snapshot).row);
+        start..end
     });
 
     hunks_for_rows(buffer_rows_for_selections, multi_buffer_snapshot)
@@ -13588,19 +13591,8 @@ pub fn hunks_for_rows(
         let query_rows =
             selected_multi_buffer_rows.start..selected_multi_buffer_rows.end.next_row();
         for hunk in multi_buffer_snapshot.git_diff_hunks_in_range(query_rows.clone()) {
-            // Deleted hunk is an empty row range, no caret can be placed there and Zed allows to revert it
-            // when the caret is just above or just below the deleted hunk.
-            let allow_adjacent = hunk_status(&hunk) == DiffHunkStatus::Removed;
-            let related_to_selection = if allow_adjacent {
-                hunk.row_range.overlaps(&query_rows)
-                    || hunk.row_range.start == query_rows.end
-                    || hunk.row_range.end == query_rows.start
-            } else {
-                // `selected_multi_buffer_rows` are inclusive (e.g. [2..2] means 2nd row is selected)
-                // `hunk.row_range` is exclusive (e.g. [2..3] means 2nd row is selected)
-                hunk.row_range.overlaps(&selected_multi_buffer_rows)
-                    || selected_multi_buffer_rows.end == hunk.row_range.start
-            };
+            let related_to_selection =
+                does_selection_touch_hunk(&selected_multi_buffer_rows, &hunk);
             if related_to_selection {
                 if !processed_buffer_rows
                     .entry(hunk.buffer_id)
@@ -13617,6 +13609,26 @@ pub fn hunks_for_rows(
     hunks
 }
 
+fn does_selection_touch_hunk(
+    selected_multi_buffer_rows: &Range<MultiBufferRow>,
+    hunk: &MultiBufferDiffHunk,
+) -> bool {
+    let query_rows = selected_multi_buffer_rows.start..selected_multi_buffer_rows.end.next_row();
+    // Deleted hunk is an empty row range, no caret can be placed there and Zed allows to revert it
+    // when the caret is just above or just below the deleted hunk.
+    let allow_adjacent = hunk_status(hunk) == DiffHunkStatus::Removed;
+    if allow_adjacent {
+        hunk.row_range.overlaps(&query_rows)
+            || hunk.row_range.start == query_rows.end
+            || hunk.row_range.end == query_rows.start
+    } else {
+        // `selected_multi_buffer_rows` are inclusive (e.g. [2..2] means 2nd row is selected)
+        // `hunk.row_range` is exclusive (e.g. [2..3] means 2nd row is selected)
+        hunk.row_range.overlaps(selected_multi_buffer_rows)
+            || selected_multi_buffer_rows.end == hunk.row_range.start
+    }
+}
+
 pub trait CollaborationHub {
     fn collaborators<'a>(&self, cx: &'a AppContext) -> &'a HashMap<PeerId, Collaborator>;
     fn user_participant_indices<'a>(

crates/editor/src/editor_tests.rs 🔗

@@ -12552,7 +12552,7 @@ async fn test_edits_around_expanded_insertion_hunks(
     executor.run_until_parked();
     cx.assert_diff_hunks(
         r#"
-        use some::mod1;
+      - use some::mod1;
       - use some::mod2;
       -
       - const A: u32 = 42;

crates/editor/src/element.rs 🔗

@@ -2509,6 +2509,7 @@ impl EditorElement {
                 element,
                 available_space: size(AvailableSpace::MinContent, element_size.height.into()),
                 style: BlockStyle::Fixed,
+                is_zero_height: block.height() == 0,
             });
         }
         for (row, block) in non_fixed_blocks {
@@ -2555,6 +2556,7 @@ impl EditorElement {
                 element,
                 available_space: size(width.into(), element_size.height.into()),
                 style,
+                is_zero_height: block.height() == 0,
             });
         }
 
@@ -2602,6 +2604,7 @@ impl EditorElement {
                             element,
                             available_space: size(width, element_size.height.into()),
                             style,
+                            is_zero_height: block.height() == 0,
                         });
                     }
                 }
@@ -3947,8 +3950,23 @@ impl EditorElement {
     }
 
     fn paint_blocks(&mut self, layout: &mut EditorLayout, cx: &mut WindowContext) {
-        for mut block in layout.blocks.drain(..) {
-            block.element.paint(cx);
+        cx.paint_layer(layout.text_hitbox.bounds, |cx| {
+            layout.blocks.retain_mut(|block| {
+                if !block.is_zero_height {
+                    block.element.paint(cx);
+                }
+
+                block.is_zero_height
+            });
+        });
+
+        // Paint all the zero-height blocks in a higher layer (if there were any remaining to paint).
+        if !layout.blocks.is_empty() {
+            cx.paint_layer(layout.text_hitbox.bounds, |cx| {
+                for mut block in layout.blocks.drain(..) {
+                    block.element.paint(cx);
+                }
+            });
         }
     }
 
@@ -6011,6 +6029,7 @@ struct BlockLayout {
     element: AnyElement,
     available_space: Size<AvailableSpace>,
     style: BlockStyle,
+    is_zero_height: bool,
 }
 
 fn layout_line(

crates/editor/src/hunk_diff.rs 🔗

@@ -1,6 +1,8 @@
 use collections::{hash_map, HashMap, HashSet};
 use git::diff::DiffHunkStatus;
-use gpui::{Action, AnchorCorner, AppContext, CursorStyle, Hsla, Model, MouseButton, Task, View};
+use gpui::{
+    AppContext, ClickEvent, CursorStyle, FocusableView, Hsla, Model, MouseButton, Task, View,
+};
 use language::{Buffer, BufferId, Point};
 use multi_buffer::{
     Anchor, AnchorRangeExt, ExcerptRange, MultiBuffer, MultiBufferDiffHunk, MultiBufferRow,
@@ -9,17 +11,18 @@ use multi_buffer::{
 use std::{ops::Range, sync::Arc};
 use text::OffsetRangeExt;
 use ui::{
-    prelude::*, ActiveTheme, ContextMenu, IconButtonShape, InteractiveElement, IntoElement,
-    ParentElement, PopoverMenu, Styled, Tooltip, ViewContext, VisualContext,
+    prelude::*, ActiveTheme, IconButtonShape, InteractiveElement, IntoElement, KeyBinding,
+    ParentElement, Styled, TintColor, Tooltip, ViewContext, VisualContext,
 };
 use util::RangeExt;
 use workspace::Item;
 
 use crate::{
-    editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, ApplyAllDiffHunks,
-    ApplyDiffHunk, BlockPlacement, BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight,
-    DisplayRow, DisplaySnapshot, Editor, EditorElement, ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk,
-    RevertFile, RevertSelectedHunks, ToDisplayPoint, ToggleHunkDiff,
+    editor_settings::CurrentLineHighlight, hunk_status, hunks_for_selections, is_hunk_selected,
+    ApplyAllDiffHunks, ApplySelectedDiffHunks, BlockPlacement, BlockProperties, BlockStyle,
+    CustomBlockId, DiffRowHighlight, DisplayRow, DisplaySnapshot, Editor, EditorElement,
+    ExpandAllHunkDiffs, GoToHunk, GoToPrevHunk, RevertSelectedHunks, ToDisplayPoint,
+    ToggleHunkDiff,
 };
 
 #[derive(Debug, Clone)]
@@ -57,7 +60,6 @@ pub enum DisplayDiffHunk {
     Folded {
         display_row: DisplayRow,
     },
-
     Unfolded {
         diff_base_byte_range: Range<usize>,
         display_row_range: Range<DisplayRow>,
@@ -371,26 +373,35 @@ impl Editor {
 
     pub(crate) fn apply_selected_diff_hunks(
         &mut self,
-        _: &ApplyDiffHunk,
+        _: &ApplySelectedDiffHunks,
         cx: &mut ViewContext<Self>,
     ) {
         let snapshot = self.buffer.read(cx).snapshot(cx);
         let hunks = hunks_for_selections(&snapshot, &self.selections.disjoint_anchors());
-        let mut ranges_by_buffer = HashMap::default();
+
         self.transact(cx, |editor, cx| {
-            for hunk in hunks {
-                if let Some(buffer) = editor.buffer.read(cx).buffer(hunk.buffer_id) {
-                    ranges_by_buffer
-                        .entry(buffer.clone())
-                        .or_insert_with(Vec::new)
-                        .push(hunk.buffer_range.to_offset(buffer.read(cx)));
+            if hunks.is_empty() {
+                // If there are no selected hunks, e.g. because we're using the keybinding with nothing selected, apply the first hunk.
+                if let Some(first_hunk) = editor.expanded_hunks.hunks.first() {
+                    editor.apply_diff_hunks_in_range(first_hunk.hunk_range.clone(), cx);
+                }
+            } else {
+                let mut ranges_by_buffer = HashMap::default();
+
+                for hunk in hunks {
+                    if let Some(buffer) = editor.buffer.read(cx).buffer(hunk.buffer_id) {
+                        ranges_by_buffer
+                            .entry(buffer.clone())
+                            .or_insert_with(Vec::new)
+                            .push(hunk.buffer_range.to_offset(buffer.read(cx)));
+                    }
                 }
-            }
 
-            for (buffer, ranges) in ranges_by_buffer {
-                buffer.update(cx, |buffer, cx| {
-                    buffer.merge_into_base(ranges, cx);
-                });
+                for (buffer, ranges) in ranges_by_buffer {
+                    buffer.update(cx, |buffer, cx| {
+                        buffer.merge_into_base(ranges, cx);
+                    });
+                }
             }
         });
 
@@ -412,246 +423,238 @@ impl Editor {
                 buffer.read(cx).diff_base_buffer().is_some()
             });
 
-        let border_color = cx.theme().colors().border_variant;
-        let bg_color = cx.theme().colors().editor_background;
-        let gutter_color = match hunk.status {
-            DiffHunkStatus::Added => cx.theme().status().created,
-            DiffHunkStatus::Modified => cx.theme().status().modified,
-            DiffHunkStatus::Removed => cx.theme().status().deleted,
-        };
-
         BlockProperties {
             placement: BlockPlacement::Above(hunk.multi_buffer_range.start),
-            height: 1,
+            height: 0,
             style: BlockStyle::Sticky,
-            priority: 0,
+            priority: 1,
             render: Arc::new({
                 let editor = cx.view().clone();
                 let hunk = hunk.clone();
 
                 move |cx| {
-                    let hunk_controls_menu_handle =
-                        editor.read(cx).hunk_controls_menu_handle.clone();
+                    let is_hunk_selected = editor.update(&mut **cx, |editor, cx| {
+                        let snapshot = editor.buffer.read(cx).snapshot(cx);
+                        let selections = &editor.selections.all::<Point>(cx);
+
+                        if editor.focus_handle(cx).is_focused(cx) && !selections.is_empty() {
+                            if let Some(hunk) = to_diff_hunk(&hunk, &snapshot) {
+                                is_hunk_selected(&hunk, selections)
+                            } else {
+                                false
+                            }
+                        } else {
+                            // If we have no cursor, or aren't focused, then default to the first hunk
+                            // because that's what the keyboard shortcuts do.
+                            editor
+                                .expanded_hunks
+                                .hunks
+                                .first()
+                                .map(|first_hunk| first_hunk.hunk_range == hunk.multi_buffer_range)
+                                .unwrap_or(false)
+                        }
+                    });
+
+                    let focus_handle = editor.focus_handle(cx);
+
+                    let handle_discard_click = {
+                        let editor = editor.clone();
+                        let hunk = hunk.clone();
+                        move |_event: &ClickEvent, cx: &mut WindowContext| {
+                            let multi_buffer = editor.read(cx).buffer().clone();
+                            let multi_buffer_snapshot = multi_buffer.read(cx).snapshot(cx);
+                            let mut revert_changes = HashMap::default();
+                            if let Some(hunk) =
+                                crate::hunk_diff::to_diff_hunk(&hunk, &multi_buffer_snapshot)
+                            {
+                                Editor::prepare_revert_change(
+                                    &mut revert_changes,
+                                    &multi_buffer,
+                                    &hunk,
+                                    cx,
+                                );
+                            }
+                            if !revert_changes.is_empty() {
+                                editor.update(cx, |editor, cx| editor.revert(revert_changes, cx));
+                            }
+                        }
+                    };
+
+                    let handle_apply_click = {
+                        let editor = editor.clone();
+                        let hunk = hunk.clone();
+                        move |_event: &ClickEvent, cx: &mut WindowContext| {
+                            editor.update(cx, |editor, cx| {
+                                editor
+                                    .apply_diff_hunks_in_range(hunk.multi_buffer_range.clone(), cx);
+                            });
+                        }
+                    };
+
+                    let discard_key_binding =
+                        KeyBinding::for_action_in(&RevertSelectedHunks, &focus_handle, cx);
+
+                    let discard_tooltip = {
+                        let focus_handle = editor.focus_handle(cx);
+                        move |cx: &mut WindowContext| {
+                            Tooltip::for_action_in(
+                                "Discard Hunk",
+                                &RevertSelectedHunks,
+                                &focus_handle,
+                                cx,
+                            )
+                        }
+                    };
 
                     h_flex()
                         .id(cx.block_id)
-                        .block_mouse_down()
-                        .h(cx.line_height())
+                        .pr_5()
                         .w_full()
-                        .border_t_1()
-                        .border_color(border_color)
-                        .bg(bg_color)
-                        .child(
-                            div()
-                                .id("gutter-strip")
-                                .w(EditorElement::diff_hunk_strip_width(cx.line_height()))
-                                .h_full()
-                                .bg(gutter_color)
-                                .cursor(CursorStyle::PointingHand)
-                                .on_click({
-                                    let editor = editor.clone();
-                                    let hunk = hunk.clone();
-                                    move |_event, cx| {
-                                        editor.update(cx, |editor, cx| {
-                                            editor.toggle_hovered_hunk(&hunk, cx);
-                                        });
-                                    }
-                                }),
-                        )
+                        .justify_end()
                         .child(
                             h_flex()
-                                .px_6()
-                                .size_full()
-                                .justify_end()
-                                .child(
-                                    h_flex()
-                                        .gap_1()
-                                        .when(!is_branch_buffer, |row| {
-                                            row.child(
-                                                IconButton::new("next-hunk", IconName::ArrowDown)
-                                                    .shape(IconButtonShape::Square)
-                                                    .icon_size(IconSize::Small)
-                                                    .tooltip({
-                                                        let focus_handle = editor.focus_handle(cx);
-                                                        move |cx| {
-                                                            Tooltip::for_action_in(
-                                                                "Next Hunk",
-                                                                &GoToHunk,
-                                                                &focus_handle,
-                                                                cx,
-                                                            )
-                                                        }
-                                                    })
-                                                    .on_click({
-                                                        let editor = editor.clone();
-                                                        let hunk = hunk.clone();
-                                                        move |_event, cx| {
-                                                            editor.update(cx, |editor, cx| {
-                                                                editor.go_to_subsequent_hunk(
-                                                                    hunk.multi_buffer_range.end,
-                                                                    cx,
-                                                                );
-                                                            });
-                                                        }
-                                                    }),
-                                            )
-                                            .child(
-                                                IconButton::new("prev-hunk", IconName::ArrowUp)
-                                                    .shape(IconButtonShape::Square)
-                                                    .icon_size(IconSize::Small)
-                                                    .tooltip({
-                                                        let focus_handle = editor.focus_handle(cx);
-                                                        move |cx| {
-                                                            Tooltip::for_action_in(
-                                                                "Previous Hunk",
-                                                                &GoToPrevHunk,
-                                                                &focus_handle,
-                                                                cx,
-                                                            )
-                                                        }
-                                                    })
-                                                    .on_click({
-                                                        let editor = editor.clone();
-                                                        let hunk = hunk.clone();
-                                                        move |_event, cx| {
-                                                            editor.update(cx, |editor, cx| {
-                                                                editor.go_to_preceding_hunk(
-                                                                    hunk.multi_buffer_range.start,
-                                                                    cx,
-                                                                );
-                                                            });
-                                                        }
-                                                    }),
-                                            )
-                                        })
-                                        .child(
-                                            IconButton::new("discard", IconName::Undo)
+                                .h(cx.line_height())
+                                .gap_1()
+                                .px_1()
+                                .pb_1()
+                                .border_x_1()
+                                .border_b_1()
+                                .border_color(cx.theme().colors().border_variant)
+                                .rounded_b_lg()
+                                .bg(cx.theme().colors().editor_background)
+                                .shadow(smallvec::smallvec![gpui::BoxShadow {
+                                    color: gpui::hsla(0.0, 0.0, 0.0, 0.1),
+                                    blur_radius: px(1.0),
+                                    spread_radius: px(1.0),
+                                    offset: gpui::point(px(0.), px(1.0)),
+                                }])
+                                .when(!is_branch_buffer, |row| {
+                                    row.child(
+                                        IconButton::new("next-hunk", IconName::ArrowDown)
+                                            .shape(IconButtonShape::Square)
+                                            .icon_size(IconSize::Small)
+                                            .tooltip({
+                                                let focus_handle = editor.focus_handle(cx);
+                                                move |cx| {
+                                                    Tooltip::for_action_in(
+                                                        "Next Hunk",
+                                                        &GoToHunk,
+                                                        &focus_handle.clone(),
+                                                        cx,
+                                                    )
+                                                }
+                                            })
+                                            .on_click({
+                                                let editor = editor.clone();
+                                                let hunk = hunk.clone();
+                                                move |_event, cx| {
+                                                    editor.update(cx, |editor, cx| {
+                                                        editor.go_to_subsequent_hunk(
+                                                            hunk.multi_buffer_range.end,
+                                                            cx,
+                                                        );
+                                                    });
+                                                }
+                                            }),
+                                    )
+                                    .child(
+                                        IconButton::new("prev-hunk", IconName::ArrowUp)
+                                            .shape(IconButtonShape::Square)
+                                            .icon_size(IconSize::Small)
+                                            .tooltip({
+                                                let focus_handle = editor.focus_handle(cx);
+                                                move |cx| {
+                                                    Tooltip::for_action_in(
+                                                        "Previous Hunk",
+                                                        &GoToPrevHunk,
+                                                        &focus_handle,
+                                                        cx,
+                                                    )
+                                                }
+                                            })
+                                            .on_click({
+                                                let editor = editor.clone();
+                                                let hunk = hunk.clone();
+                                                move |_event, cx| {
+                                                    editor.update(cx, |editor, cx| {
+                                                        editor.go_to_preceding_hunk(
+                                                            hunk.multi_buffer_range.start,
+                                                            cx,
+                                                        );
+                                                    });
+                                                }
+                                            }),
+                                    )
+                                })
+                                .child(if is_branch_buffer {
+                                    if is_hunk_selected {
+                                        Button::new("discard", "Discard")
+                                            .style(ButtonStyle::Tinted(TintColor::Negative))
+                                            .label_size(LabelSize::Small)
+                                            .key_binding(discard_key_binding)
+                                            .on_click(handle_discard_click.clone())
+                                            .into_any_element()
+                                    } else {
+                                        IconButton::new("discard", IconName::Close)
+                                            .style(ButtonStyle::Tinted(TintColor::Negative))
+                                            .shape(IconButtonShape::Square)
+                                            .icon_size(IconSize::XSmall)
+                                            .tooltip(discard_tooltip.clone())
+                                            .on_click(handle_discard_click.clone())
+                                            .into_any_element()
+                                    }
+                                } else {
+                                    if is_hunk_selected {
+                                        Button::new("undo", "Undo")
+                                            .style(ButtonStyle::Tinted(TintColor::Negative))
+                                            .label_size(LabelSize::Small)
+                                            .key_binding(discard_key_binding)
+                                            .on_click(handle_discard_click.clone())
+                                            .into_any_element()
+                                    } else {
+                                        IconButton::new("undo", IconName::Undo)
+                                            .shape(IconButtonShape::Square)
+                                            .icon_size(IconSize::Small)
+                                            .tooltip(discard_tooltip.clone())
+                                            .on_click(handle_discard_click.clone())
+                                            .into_any_element()
+                                    }
+                                })
+                                .when(is_branch_buffer, |this| {
+                                    this.child({
+                                        let button = Button::new("apply", "Apply")
+                                            .style(ButtonStyle::Tinted(TintColor::Positive))
+                                            .label_size(LabelSize::Small)
+                                            .key_binding(KeyBinding::for_action_in(
+                                                &ApplySelectedDiffHunks,
+                                                &focus_handle,
+                                                cx,
+                                            ))
+                                            .on_click(handle_apply_click.clone())
+                                            .into_any_element();
+                                        if is_hunk_selected {
+                                            button
+                                        } else {
+                                            IconButton::new("apply", IconName::Check)
+                                                .style(ButtonStyle::Tinted(TintColor::Positive))
                                                 .shape(IconButtonShape::Square)
-                                                .icon_size(IconSize::Small)
+                                                .icon_size(IconSize::XSmall)
                                                 .tooltip({
                                                     let focus_handle = editor.focus_handle(cx);
                                                     move |cx| {
                                                         Tooltip::for_action_in(
-                                                            "Discard Hunk",
-                                                            &RevertSelectedHunks,
+                                                            "Apply Hunk",
+                                                            &ApplySelectedDiffHunks,
                                                             &focus_handle,
                                                             cx,
                                                         )
                                                     }
                                                 })
-                                                .on_click({
-                                                    let editor = editor.clone();
-                                                    let hunk = hunk.clone();
-                                                    move |_event, cx| {
-                                                        let multi_buffer =
-                                                            editor.read(cx).buffer().clone();
-                                                        let multi_buffer_snapshot =
-                                                            multi_buffer.read(cx).snapshot(cx);
-                                                        let mut revert_changes = HashMap::default();
-                                                        if let Some(hunk) =
-                                                            crate::hunk_diff::to_diff_hunk(
-                                                                &hunk,
-                                                                &multi_buffer_snapshot,
-                                                            )
-                                                        {
-                                                            Editor::prepare_revert_change(
-                                                                &mut revert_changes,
-                                                                &multi_buffer,
-                                                                &hunk,
-                                                                cx,
-                                                            );
-                                                        }
-                                                        if !revert_changes.is_empty() {
-                                                            editor.update(cx, |editor, cx| {
-                                                                editor.revert(revert_changes, cx)
-                                                            });
-                                                        }
-                                                    }
-                                                }),
-                                        )
-                                        .map(|this| {
-                                            if is_branch_buffer {
-                                                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_diff_hunks_in_range(
-                                                                            hunk.multi_buffer_range
-                                                                                .clone(),
-                                                                            cx,
-                                                                        );
-                                                                });
-                                                            }
-                                                        }),
-                                                )
-                                            } else {
-                                                this.child({
-                                                    let focus = editor.focus_handle(cx);
-                                                    PopoverMenu::new("hunk-controls-dropdown")
-                                                        .trigger(
-                                                            IconButton::new(
-                                                                "toggle_editor_selections_icon",
-                                                                IconName::EllipsisVertical,
-                                                            )
-                                                            .shape(IconButtonShape::Square)
-                                                            .icon_size(IconSize::Small)
-                                                            .style(ButtonStyle::Subtle)
-                                                            .selected(
-                                                                hunk_controls_menu_handle
-                                                                    .is_deployed(),
-                                                            )
-                                                            .when(
-                                                                !hunk_controls_menu_handle
-                                                                    .is_deployed(),
-                                                                |this| {
-                                                                    this.tooltip(|cx| {
-                                                                        Tooltip::text(
-                                                                            "Hunk Controls",
-                                                                            cx,
-                                                                        )
-                                                                    })
-                                                                },
-                                                            ),
-                                                        )
-                                                        .anchor(AnchorCorner::TopRight)
-                                                        .with_handle(hunk_controls_menu_handle)
-                                                        .menu(move |cx| {
-                                                            let focus = focus.clone();
-                                                            let menu = ContextMenu::build(
-                                                                cx,
-                                                                move |menu, _| {
-                                                                    menu.context(focus.clone())
-                                                                        .action(
-                                                                            "Discard All Hunks",
-                                                                            RevertFile
-                                                                                .boxed_clone(),
-                                                                        )
-                                                                },
-                                                            );
-                                                            Some(menu)
-                                                        })
-                                                })
-                                            }
-                                        }),
-                                )
+                                                .on_click(handle_apply_click.clone())
+                                                .into_any_element()
+                                        }
+                                    })
+                                })
                                 .when(!is_branch_buffer, |div| {
                                     div.child(
                                         IconButton::new("collapse", IconName::Close)
@@ -707,7 +710,7 @@ impl Editor {
             placement: BlockPlacement::Above(hunk.multi_buffer_range.start),
             height,
             style: BlockStyle::Flex,
-            priority: 0,
+            priority: 1,
             render: Arc::new(move |cx| {
                 let width = EditorElement::diff_hunk_strip_width(cx.line_height());
                 let gutter_dimensions = editor.read(cx.context).gutter_dimensions;

crates/editor/src/proposed_changes_editor.rs 🔗

@@ -5,10 +5,11 @@ use gpui::{AppContext, EventEmitter, FocusableView, Model, Render, Subscription,
 use language::{Buffer, BufferEvent, Capability};
 use multi_buffer::{ExcerptRange, MultiBuffer};
 use project::Project;
+use settings::Settings;
 use smol::stream::StreamExt;
 use std::{any::TypeId, ops::Range, rc::Rc, time::Duration};
 use text::ToOffset;
-use ui::{prelude::*, ButtonLike, KeyBinding};
+use ui::{prelude::*, KeyBinding};
 use workspace::{
     searchable::SearchableItemHandle, Item, ItemHandle as _, ToolbarItemEvent, ToolbarItemLocation,
     ToolbarItemView, Workspace,
@@ -34,7 +35,11 @@ struct BufferEntry {
     _subscription: Subscription,
 }
 
-pub struct ProposedChangesEditorToolbar {
+pub struct ProposedChangesToolbarControls {
+    current_editor: Option<View<ProposedChangesEditor>>,
+}
+
+pub struct ProposedChangesToolbar {
     current_editor: Option<View<ProposedChangesEditor>>,
 }
 
@@ -228,6 +233,10 @@ impl ProposedChangesEditor {
             _ => (),
         }
     }
+
+    fn all_changes_accepted(&self) -> bool {
+        false // In the future, we plan to compute this based on the current state of patches.
+    }
 }
 
 impl Render for ProposedChangesEditor {
@@ -251,7 +260,11 @@ impl Item for ProposedChangesEditor {
     type Event = EditorEvent;
 
     fn tab_icon(&self, _cx: &ui::WindowContext) -> Option<Icon> {
-        Some(Icon::new(IconName::Diff))
+        if self.all_changes_accepted() {
+            Some(Icon::new(IconName::Check).color(Color::Success))
+        } else {
+            Some(Icon::new(IconName::ZedAssistant))
+        }
     }
 
     fn tab_content_text(&self, _cx: &WindowContext) -> Option<SharedString> {
@@ -317,7 +330,7 @@ impl Item for ProposedChangesEditor {
     }
 }
 
-impl ProposedChangesEditorToolbar {
+impl ProposedChangesToolbarControls {
     pub fn new() -> Self {
         Self {
             current_editor: None,
@@ -333,28 +346,97 @@ impl ProposedChangesEditorToolbar {
     }
 }
 
-impl Render for ProposedChangesEditorToolbar {
+impl Render for ProposedChangesToolbarControls {
     fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
-        let button_like = ButtonLike::new("apply-changes").child(Label::new("Apply All"));
+        if let Some(editor) = &self.current_editor {
+            let focus_handle = editor.focus_handle(cx);
+            let action = &ApplyAllDiffHunks;
+            let keybinding = KeyBinding::for_action_in(action, &focus_handle, cx);
 
-        match &self.current_editor {
-            Some(editor) => {
-                let focus_handle = editor.focus_handle(cx);
-                let keybinding = KeyBinding::for_action_in(&ApplyAllDiffHunks, &focus_handle, cx)
-                    .map(|binding| binding.into_any_element());
+            let editor = editor.read(cx);
 
-                button_like.children(keybinding).on_click({
-                    move |_event, cx| focus_handle.dispatch_action(&ApplyAllDiffHunks, cx)
-                })
-            }
-            None => button_like.disabled(true),
+            let apply_all_button = if editor.all_changes_accepted() {
+                None
+            } else {
+                Some(
+                    Button::new("apply-changes", "Apply All")
+                        .style(ButtonStyle::Filled)
+                        .key_binding(keybinding)
+                        .on_click(move |_event, cx| focus_handle.dispatch_action(action, cx)),
+                )
+            };
+
+            h_flex()
+                .gap_1()
+                .children([apply_all_button].into_iter().flatten())
+                .into_any_element()
+        } else {
+            gpui::Empty.into_any_element()
+        }
+    }
+}
+
+impl EventEmitter<ToolbarItemEvent> for ProposedChangesToolbarControls {}
+
+impl ToolbarItemView for ProposedChangesToolbarControls {
+    fn set_active_pane_item(
+        &mut self,
+        active_pane_item: Option<&dyn workspace::ItemHandle>,
+        _cx: &mut ViewContext<Self>,
+    ) -> workspace::ToolbarItemLocation {
+        self.current_editor =
+            active_pane_item.and_then(|item| item.downcast::<ProposedChangesEditor>());
+        self.get_toolbar_item_location()
+    }
+}
+
+impl ProposedChangesToolbar {
+    pub fn new() -> Self {
+        Self {
+            current_editor: None,
+        }
+    }
+
+    fn get_toolbar_item_location(&self) -> ToolbarItemLocation {
+        if self.current_editor.is_some() {
+            ToolbarItemLocation::PrimaryLeft
+        } else {
+            ToolbarItemLocation::Hidden
+        }
+    }
+}
+
+impl Render for ProposedChangesToolbar {
+    fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
+        if let Some(editor) = &self.current_editor {
+            let editor = editor.read(cx);
+            let all_changes_accepted = editor.all_changes_accepted();
+            let icon = if all_changes_accepted {
+                Icon::new(IconName::Check).color(Color::Success)
+            } else {
+                Icon::new(IconName::ZedAssistant)
+            };
+
+            h_flex()
+                .gap_2p5()
+                .font(theme::ThemeSettings::get_global(cx).buffer_font.clone())
+                .child(icon.size(IconSize::Small))
+                .child(
+                    Label::new(editor.title.clone())
+                        .color(Color::Muted)
+                        .single_line()
+                        .strikethrough(all_changes_accepted),
+                )
+                .into_any_element()
+        } else {
+            gpui::Empty.into_any_element()
         }
     }
 }
 
-impl EventEmitter<ToolbarItemEvent> for ProposedChangesEditorToolbar {}
+impl EventEmitter<ToolbarItemEvent> for ProposedChangesToolbar {}
 
-impl ToolbarItemView for ProposedChangesEditorToolbar {
+impl ToolbarItemView for ProposedChangesToolbar {
     fn set_active_pane_item(
         &mut self,
         active_pane_item: Option<&dyn workspace::ItemHandle>,

crates/zed/src/zed.rs 🔗

@@ -17,8 +17,8 @@ use breadcrumbs::Breadcrumbs;
 use client::{zed_urls, ZED_URL_SCHEME};
 use collections::VecDeque;
 use command_palette_hooks::CommandPaletteFilter;
-use editor::ProposedChangesEditorToolbar;
 use editor::{scroll::Autoscroll, Editor, MultiBuffer};
+use editor::{ProposedChangesToolbar, ProposedChangesToolbarControls};
 use feature_flags::FeatureFlagAppExt;
 use futures::{channel::mpsc, select_biased, StreamExt};
 use gpui::{
@@ -644,8 +644,10 @@ fn initialize_pane(workspace: &Workspace, pane: &View<Pane>, cx: &mut ViewContex
             let buffer_search_bar = cx.new_view(search::BufferSearchBar::new);
             toolbar.add_item(buffer_search_bar.clone(), cx);
 
-            let proposed_change_bar = cx.new_view(|_| ProposedChangesEditorToolbar::new());
-            toolbar.add_item(proposed_change_bar, cx);
+            let proposed_changes_bar = cx.new_view(|_| ProposedChangesToolbar::new());
+            toolbar.add_item(proposed_changes_bar, cx);
+            let proposed_changes_controls = cx.new_view(|_| ProposedChangesToolbarControls::new());
+            toolbar.add_item(proposed_changes_controls, cx);
             let quick_action_bar =
                 cx.new_view(|cx| QuickActionBar::new(buffer_search_bar, workspace, cx));
             toolbar.add_item(quick_action_bar, cx);