Commit All Mode (#24293)

Conrad Irwin created

- **Base diffs on uncommitted changes**
- **Show added files in project diff view**
- **Fix git panel optimism**
- **boop**
- **Co-Authored-By: Cole <cole@zed.dev>**
- **Fix commit (all) buttons state**
- **WIP**
- **WIP: commit all mode**

Closes #ISSUE

Release Notes:

- N/A

Change summary

assets/icons/circle.svg            |   1 
crates/git_ui/src/git_panel.rs     | 199 ++++++++++++++++---------------
crates/ui/src/components/icon.rs   |   1 
crates/ui/src/components/toggle.rs |  41 ++++--
crates/ui/src/traits/toggleable.rs |  18 +-
5 files changed, 142 insertions(+), 118 deletions(-)

Detailed changes

assets/icons/circle.svg 🔗

@@ -0,0 +1 @@
+<svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"><circle cx="7.25" cy="7.25" r="3" fill="currentColor"></circle></svg>

crates/git_ui/src/git_panel.rs 🔗

@@ -75,15 +75,15 @@ struct SerializedGitPanel {
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 enum Section {
-    Changed,
-    Created,
+    Tracked,
+    New,
 }
 
 impl Section {
     pub fn contains(&self, status: FileStatus) -> bool {
         match self {
-            Section::Changed => !status.is_created(),
-            Section::Created => status.is_created(),
+            Section::Tracked => !status.is_created(),
+            Section::New => status.is_created(),
         }
     }
 }
@@ -99,8 +99,8 @@ impl GitHeaderEntry {
     }
     pub fn title(&self) -> &'static str {
         match self.header {
-            Section::Changed => "Changed",
-            Section::Created => "New",
+            Section::Tracked => "Changed",
+            Section::New => "New",
         }
     }
 }
@@ -112,9 +112,9 @@ enum GitListEntry {
 }
 
 impl GitListEntry {
-    fn status_entry(&self) -> Option<GitStatusEntry> {
+    fn status_entry(&self) -> Option<&GitStatusEntry> {
         match self {
-            GitListEntry::GitStatusEntry(entry) => Some(entry.clone()),
+            GitListEntry::GitStatusEntry(entry) => Some(entry),
             _ => None,
         }
     }
@@ -129,7 +129,7 @@ pub struct GitStatusEntry {
     pub(crate) is_staged: Option<bool>,
 }
 
-pub struct PendingOperation {
+struct PendingOperation {
     finished: bool,
     will_become_staged: bool,
     repo_paths: HashSet<RepoPath>,
@@ -158,8 +158,11 @@ pub struct GitPanel {
     pending: Vec<PendingOperation>,
     commit_task: Task<Result<()>>,
     commit_pending: bool,
-    can_commit: bool,
-    can_commit_all: bool,
+
+    tracked_staged_count: usize,
+    tracked_count: usize,
+    new_staged_count: usize,
+    new_count: usize,
 }
 
 fn commit_message_editor(
@@ -272,8 +275,10 @@ impl GitPanel {
                 commit_editor,
                 project,
                 workspace,
-                can_commit: false,
-                can_commit_all: false,
+                tracked_staged_count: 0,
+                tracked_count: 0,
+                new_staged_count: 0,
+                new_count: 0,
             };
             git_panel.schedule_update(false, window, cx);
             git_panel.show_scrollbar = git_panel.should_show_scrollbar(cx);
@@ -579,7 +584,7 @@ impl GitPanel {
                         section.contains(&status_entry)
                             && status_entry.is_staged != Some(goal_staged_state)
                     })
-                    .map(|status_entry| status_entry.repo_path)
+                    .map(|status_entry| status_entry.repo_path.clone())
                     .collect::<Vec<_>>();
 
                 (goal_staged_state, entries)
@@ -593,10 +598,12 @@ impl GitPanel {
             repo_paths: repo_paths.iter().cloned().collect(),
             finished: false,
         });
+        let repo_paths = repo_paths.clone();
+        let active_repository = active_repository.clone();
+        self.update_counts();
+        cx.notify();
 
         cx.spawn({
-            let repo_paths = repo_paths.clone();
-            let active_repository = active_repository.clone();
             |this, mut cx| async move {
                 let result = cx
                     .update(|cx| {
@@ -673,7 +680,8 @@ impl GitPanel {
         let Some(active_repository) = self.active_repository.clone() else {
             return;
         };
-        if !self.can_commit {
+        if !self.has_staged_changes() {
+            self.commit_tracked_changes(&Default::default(), name_and_email, window, cx);
             return;
         }
         let message = self.commit_editor.read(cx).text(cx);
@@ -716,7 +724,7 @@ impl GitPanel {
         let Some(active_repository) = self.active_repository.clone() else {
             return;
         };
-        if !self.can_commit_all {
+        if !self.has_staged_changes() || !self.has_tracked_changes() {
             return;
         }
 
@@ -731,10 +739,10 @@ impl GitPanel {
             .iter()
             .filter_map(|entry| entry.status_entry())
             .filter(|status_entry| {
-                Section::Changed.contains(status_entry.status)
+                Section::Tracked.contains(status_entry.status)
                     && !status_entry.is_staged.unwrap_or(false)
             })
-            .map(|status_entry| status_entry.repo_path)
+            .map(|status_entry| status_entry.repo_path.clone())
             .collect::<Vec<_>>();
 
         self.commit_task = cx.spawn_in(window, |git_panel, mut cx| async move {
@@ -911,10 +919,6 @@ impl GitPanel {
         let repo = repo.read(cx);
         let path_set = HashSet::from_iter(repo.status().map(|entry| entry.repo_path));
 
-        let mut has_changed_checked_boxes = false;
-        let mut has_changed = false;
-        let mut has_added_checked_boxes = false;
-
         // Second pass - create entries with proper depth calculation
         for entry in repo.status() {
             let (depth, difference) =
@@ -951,15 +955,8 @@ impl GitPanel {
             };
 
             if is_new {
-                if entry.is_staged != Some(false) {
-                    has_added_checked_boxes = true
-                }
                 new_entries.push(entry);
             } else {
-                has_changed = true;
-                if entry.is_staged != Some(false) {
-                    has_changed_checked_boxes = true
-                }
                 changed_entries.push(entry);
             }
         }
@@ -970,7 +967,7 @@ impl GitPanel {
 
         if changed_entries.len() > 0 {
             self.entries.push(GitListEntry::Header(GitHeaderEntry {
-                header: Section::Changed,
+                header: Section::Tracked,
             }));
             self.entries.extend(
                 changed_entries
@@ -980,7 +977,7 @@ impl GitPanel {
         }
         if new_entries.len() > 0 {
             self.entries.push(GitListEntry::Header(GitHeaderEntry {
-                header: Section::Created,
+                header: Section::New,
             }));
             self.entries
                 .extend(new_entries.into_iter().map(GitListEntry::GitStatusEntry));
@@ -988,39 +985,62 @@ impl GitPanel {
 
         for (ix, entry) in self.entries.iter().enumerate() {
             if let Some(status_entry) = entry.status_entry() {
-                self.entries_by_path.insert(status_entry.repo_path, ix);
+                self.entries_by_path
+                    .insert(status_entry.repo_path.clone(), ix);
             }
         }
-        self.can_commit = has_changed_checked_boxes || has_added_checked_boxes;
-        self.can_commit_all = has_changed || has_added_checked_boxes;
+        self.update_counts();
 
         self.select_first_entry_if_none(cx);
 
         cx.notify();
     }
 
-    fn header_state(&self, header_type: Section) -> ToggleState {
-        let mut count = 0;
-        let mut staged_count = 0;
-        'outer: for entry in &self.entries {
-            let Some(entry) = entry.status_entry() else {
+    fn update_counts(&mut self) {
+        self.new_count = 0;
+        self.tracked_count = 0;
+        self.new_staged_count = 0;
+        self.tracked_staged_count = 0;
+        for entry in &self.entries {
+            let Some(status_entry) = entry.status_entry() else {
                 continue;
             };
-            if entry.status.is_created() != (header_type == Section::Created) {
-                continue;
-            }
-            count += 1;
-            for pending in self.pending.iter().rev() {
-                if pending.repo_paths.contains(&entry.repo_path) {
-                    if pending.will_become_staged {
-                        staged_count += 1;
-                    }
-                    continue 'outer;
+            if status_entry.status.is_created() {
+                self.new_count += 1;
+                if self.entry_appears_staged(status_entry) != Some(false) {
+                    self.new_staged_count += 1;
+                }
+            } else {
+                self.tracked_count += 1;
+                if self.entry_appears_staged(status_entry) != Some(false) {
+                    self.tracked_staged_count += 1;
                 }
             }
-            staged_count += entry.status.is_staged().unwrap_or(false) as usize;
         }
+    }
 
+    fn entry_appears_staged(&self, entry: &GitStatusEntry) -> Option<bool> {
+        for pending in self.pending.iter().rev() {
+            if pending.repo_paths.contains(&entry.repo_path) {
+                return Some(pending.will_become_staged);
+            }
+        }
+        entry.is_staged
+    }
+
+    fn has_staged_changes(&self) -> bool {
+        self.tracked_staged_count > 0 || self.new_staged_count > 0
+    }
+
+    fn has_tracked_changes(&self) -> bool {
+        self.tracked_count > 0
+    }
+
+    fn header_state(&self, header_type: Section) -> ToggleState {
+        let (staged_count, count) = match header_type {
+            Section::New => (self.new_staged_count, self.new_count),
+            Section::Tracked => (self.tracked_staged_count, self.tracked_count),
+        };
         if staged_count == 0 {
             ToggleState::Unselected
         } else if count == staged_count {
@@ -1157,25 +1177,27 @@ impl GitPanel {
         cx: &Context<Self>,
     ) -> impl IntoElement {
         let editor = self.commit_editor.clone();
-        let can_commit = !self.commit_pending && self.can_commit && !editor.read(cx).is_empty(cx);
-        let can_commit_all =
-            !self.commit_pending && self.can_commit_all && !editor.read(cx).is_empty(cx);
+        let can_commit =
+            (self.has_staged_changes() || self.has_tracked_changes()) && !self.commit_pending;
         let editor_focus_handle = editor.read(cx).focus_handle(cx).clone();
 
         let focus_handle_1 = self.focus_handle(cx).clone();
-        let focus_handle_2 = self.focus_handle(cx).clone();
+        let tooltip = if self.has_staged_changes() {
+            "Commit staged changes"
+        } else {
+            "Commit changes to tracked files"
+        };
+        let title = if self.has_staged_changes() {
+            "Commit"
+        } else {
+            "Commit All"
+        };
 
-        let commit_staged_button = self
-            .panel_button("commit-staged-changes", "Commit")
+        let commit_button = self
+            .panel_button("commit-changes", title)
             .tooltip(move |window, cx| {
                 let focus_handle = focus_handle_1.clone();
-                Tooltip::for_action_in(
-                    "Commit all staged changes",
-                    &CommitChanges,
-                    &focus_handle,
-                    window,
-                    cx,
-                )
+                Tooltip::for_action_in(tooltip, &CommitChanges, &focus_handle, window, cx)
             })
             .disabled(!can_commit)
             .on_click({
@@ -1185,31 +1207,6 @@ impl GitPanel {
                 })
             });
 
-        let commit_all_button = self
-            .panel_button("commit-all-changes", "Commit All")
-            .tooltip(move |window, cx| {
-                let focus_handle = focus_handle_2.clone();
-                Tooltip::for_action_in(
-                    "Commit all changes, including unstaged changes",
-                    &CommitAllChanges,
-                    &focus_handle,
-                    window,
-                    cx,
-                )
-            })
-            .disabled(!can_commit_all)
-            .on_click({
-                let name_and_email = name_and_email.clone();
-                cx.listener(move |this, _: &ClickEvent, window, cx| {
-                    this.commit_tracked_changes(
-                        &CommitAllChanges,
-                        name_and_email.clone(),
-                        window,
-                        cx,
-                    )
-                })
-            });
-
         div().w_full().h(px(140.)).px_2().pt_1().pb_2().child(
             v_flex()
                 .id("commit-editor-container")
@@ -1229,8 +1226,7 @@ impl GitPanel {
                         .right_3()
                         .gap_1p5()
                         .child(div().gap_1().flex_grow())
-                        .child(commit_all_button)
-                        .child(commit_staged_button),
+                        .child(commit_button),
                 ),
         )
     }
@@ -1423,8 +1419,17 @@ impl GitPanel {
         _window: &Window,
         cx: &Context<Self>,
     ) -> AnyElement {
-        let checkbox = Checkbox::new(header.title(), self.header_state(header.header))
+        let header_state = if self.has_staged_changes() {
+            self.header_state(header.header)
+        } else {
+            match header.header {
+                Section::Tracked => ToggleState::Selected,
+                Section::New => ToggleState::Unselected,
+            }
+        };
+        let checkbox = Checkbox::new(header.title(), header_state)
             .disabled(!has_write_access)
+            .placeholder(!self.has_staged_changes())
             .fill()
             .elevation(ElevationIndex::Surface);
         let selected = self.selected_entry == Some(ix);
@@ -1507,14 +1512,19 @@ impl GitPanel {
 
         let id: ElementId = ElementId::Name(format!("entry_{}", display_name).into());
 
-        let is_staged = pending
+        let mut is_staged = pending
             .or_else(|| entry.is_staged)
             .map(ToggleState::from)
             .unwrap_or(ToggleState::Indeterminate);
 
+        if !self.has_staged_changes() && !entry.status.is_created() {
+            is_staged = ToggleState::Selected;
+        }
+
         let checkbox = Checkbox::new(id, is_staged)
             .disabled(!has_write_access)
             .fill()
+            .placeholder(!self.has_staged_changes())
             .elevation(ElevationIndex::Surface)
             .on_click({
                 let entry = entry.clone();
@@ -1532,6 +1542,7 @@ impl GitPanel {
             .id(("start-slot", ix))
             .gap(DynamicSpacing::Base04.rems(cx))
             .child(checkbox)
+            .tooltip(|window, cx| Tooltip::for_action("Stage File", &ToggleStaged, window, cx))
             .child(git_status_icon(status, cx))
             .on_mouse_down(MouseButton::Left, |_, _, cx| {
                 // prevent the list item active state triggering when toggling checkbox

crates/ui/src/components/toggle.rs 🔗

@@ -43,6 +43,7 @@ pub struct Checkbox {
     id: ElementId,
     toggle_state: ToggleState,
     disabled: bool,
+    placeholder: bool,
     on_click: Option<Box<dyn Fn(&ToggleState, &mut Window, &mut App) + 'static>>,
     filled: bool,
     style: ToggleStyle,
@@ -62,6 +63,7 @@ impl Checkbox {
             style: ToggleStyle::default(),
             tooltip: None,
             label: None,
+            placeholder: false,
         }
     }
 
@@ -71,6 +73,12 @@ impl Checkbox {
         self
     }
 
+    /// Sets the disabled state of the [`Checkbox`].
+    pub fn placeholder(mut self, placeholder: bool) -> Self {
+        self.placeholder = placeholder;
+        self
+    }
+
     /// Binds a handler to the [`Checkbox`] that will be called when clicked.
     pub fn on_click(
         mut self,
@@ -145,23 +153,26 @@ impl Checkbox {
 impl RenderOnce for Checkbox {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
         let group_id = format!("checkbox_group_{:?}", self.id);
+        let color = if self.disabled {
+            Color::Disabled
+        } else if self.placeholder {
+            Color::Placeholder
+        } else {
+            Color::Selected
+        };
         let icon = match self.toggle_state {
-            ToggleState::Selected => Some(Icon::new(IconName::Check).size(IconSize::Small).color(
-                if self.disabled {
-                    Color::Disabled
-                } else {
-                    Color::Selected
-                },
-            )),
-            ToggleState::Indeterminate => Some(
-                Icon::new(IconName::Dash)
+            ToggleState::Selected => Some(if self.placeholder {
+                Icon::new(IconName::Circle)
+                    .size(IconSize::XSmall)
+                    .color(color)
+            } else {
+                Icon::new(IconName::Check)
                     .size(IconSize::Small)
-                    .color(if self.disabled {
-                        Color::Disabled
-                    } else {
-                        Color::Selected
-                    }),
-            ),
+                    .color(color)
+            }),
+            ToggleState::Indeterminate => {
+                Some(Icon::new(IconName::Dash).size(IconSize::Small).color(color))
+            }
             ToggleState::Unselected => None,
         };
 

crates/ui/src/traits/toggleable.rs 🔗

@@ -58,12 +58,12 @@ impl From<bool> for ToggleState {
     }
 }
 
-impl From<Option<bool>> for ToggleState {
-    fn from(selected: Option<bool>) -> Self {
-        match selected {
-            Some(true) => Self::Selected,
-            Some(false) => Self::Unselected,
-            None => Self::Unselected,
-        }
-    }
-}
+// impl From<Option<bool>> for ToggleState {
+//     fn from(selected: Option<bool>) -> Self {
+//         match selected {
+//             Some(true) => Self::Selected,
+//             Some(false) => Self::Unselected,
+//             None => Self::Unselected,
+//         }
+//     }
+// }