git: Migrate GitPanel UI components to use repo.pending_ops_by_path

Jakub Konka created

Change summary

crates/git_ui/src/git_panel.rs             | 230 ++++++++---------------
crates/project/src/git_store.rs            |   2 
crates/project/src/git_store/pending_op.rs |  41 +++
3 files changed, 116 insertions(+), 157 deletions(-)

Detailed changes

crates/git_ui/src/git_panel.rs 🔗

@@ -47,7 +47,7 @@ use panel::{
 };
 use project::{
     Fs, Project, ProjectPath,
-    git_store::{GitStoreEvent, Repository, RepositoryEvent, RepositoryId},
+    git_store::{GitStoreEvent, Repository, RepositoryEvent, RepositoryId, pending_op},
 };
 use serde::{Deserialize, Serialize};
 use settings::{Settings, SettingsStore, StatusStyle};
@@ -271,27 +271,6 @@ impl GitStatusEntry {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-enum TargetStatus {
-    Staged,
-    Unstaged,
-    Reverted,
-    Unchanged,
-}
-
-struct PendingOperation {
-    finished: bool,
-    target_status: TargetStatus,
-    entries: Vec<GitStatusEntry>,
-    op_id: usize,
-}
-
-impl PendingOperation {
-    fn contains_path(&self, path: &RepoPath) -> bool {
-        self.entries.iter().any(|p| &p.repo_path == path)
-    }
-}
-
 pub struct GitPanel {
     pub(crate) active_repository: Option<Entity<Repository>>,
     pub(crate) commit_editor: Entity<Editor>,
@@ -307,7 +286,6 @@ pub struct GitPanel {
     new_count: usize,
     entry_count: usize,
     new_staged_count: usize,
-    pending: Vec<PendingOperation>,
     pending_commit: Option<Task<()>>,
     amend_pending: bool,
     original_commit_message: Option<String>,
@@ -427,7 +405,7 @@ impl GitPanel {
                 move |this, _git_store, event, window, cx| match event {
                     GitStoreEvent::ActiveRepositoryChanged(_) => {
                         this.active_repository = this.project.read(cx).active_repository(cx);
-                        this.schedule_update(true, window, cx);
+                        this.schedule_update(window, cx);
                     }
                     GitStoreEvent::RepositoryUpdated(
                         _,
@@ -436,7 +414,7 @@ impl GitPanel {
                         | RepositoryEvent::MergeHeadsChanged,
                         true,
                     ) => {
-                        this.schedule_update(true, window, cx);
+                        this.schedule_update(window, cx);
                     }
                     GitStoreEvent::RepositoryUpdated(
                         _,
@@ -445,7 +423,7 @@ impl GitPanel {
                     )
                     | GitStoreEvent::RepositoryAdded
                     | GitStoreEvent::RepositoryRemoved(_) => {
-                        this.schedule_update(false, window, cx);
+                        this.schedule_update(window, cx);
                     }
                     GitStoreEvent::IndexWriteError(error) => {
                         this.workspace
@@ -472,7 +450,6 @@ impl GitPanel {
                 fs,
                 new_count: 0,
                 new_staged_count: 0,
-                pending: Vec::new(),
                 pending_commit: None,
                 amend_pending: false,
                 original_commit_message: None,
@@ -501,7 +478,7 @@ impl GitPanel {
                 _settings_subscription,
             };
 
-            this.schedule_update(false, window, cx);
+            this.schedule_update(window, cx);
             this
         })
     }
@@ -1013,13 +990,6 @@ impl GitPanel {
             return;
         };
 
-        let op_id = self.pending.iter().map(|p| p.op_id).max().unwrap_or(0) + 1;
-        self.pending.push(PendingOperation {
-            op_id,
-            target_status: TargetStatus::Reverted,
-            entries: entries.clone(),
-            finished: false,
-        });
         self.update_visible_entries(window, cx);
         let task = cx.spawn(async move |_, cx| {
             let tasks: Vec<_> = workspace.update(cx, |workspace, cx| {
@@ -1071,21 +1041,10 @@ impl GitPanel {
             let result = task.await;
 
             this.update_in(cx, |this, window, cx| {
-                for pending in this.pending.iter_mut() {
-                    if pending.op_id == op_id {
-                        pending.finished = true;
-                        if result.is_err() {
-                            pending.target_status = TargetStatus::Unchanged;
-                            this.update_visible_entries(window, cx);
-                        }
-                        break;
-                    }
+                if let Err(err) = result {
+                    this.update_visible_entries(window, cx);
+                    this.show_error_toast("checkout", err, cx);
                 }
-                result
-                    .map_err(|e| {
-                        this.show_error_toast("checkout", e, cx);
-                    })
-                    .ok();
             })
             .ok();
         })
@@ -1247,8 +1206,13 @@ impl GitPanel {
         let (stage, repo_paths) = match entry {
             GitListEntry::Status(status_entry) => {
                 let repo_paths = vec![status_entry.clone()];
-                let stage = if let Some(status) = self.entry_staging(&status_entry) {
-                    !status.is_fully_staged()
+                let stage = if active_repository
+                    .read(cx)
+                    .pending_ops_for_path(&status_entry.repo_path)
+                    .map(|ops| ops.staging())
+                    .unwrap_or(false)
+                {
+                    false
                 } else if status_entry.status.staging().is_fully_staged() {
                     if let Some(op) = self.bulk_staging.clone()
                         && op.anchor == status_entry.repo_path
@@ -1291,26 +1255,8 @@ impl GitPanel {
         let Some(active_repository) = self.active_repository.clone() else {
             return;
         };
-        let op_id = self.pending.iter().map(|p| p.op_id).max().unwrap_or(0) + 1;
-        self.pending.push(PendingOperation {
-            op_id,
-            target_status: if stage {
-                TargetStatus::Staged
-            } else {
-                TargetStatus::Unstaged
-            },
-            entries: entries.clone(),
-            finished: false,
-        });
 
         let repository = active_repository.read(cx);
-        for entry in &entries {
-            println!(
-                "{:?} -> {:?}",
-                &entry.repo_path,
-                repository.snapshot().pending_ops_for_path(&entry.repo_path)
-            );
-        }
         self.update_counts(repository);
         cx.notify();
 
@@ -1339,11 +1285,6 @@ impl GitPanel {
                     .await;
 
                 this.update(cx, |this, cx| {
-                    for pending in this.pending.iter_mut() {
-                        if pending.op_id == op_id {
-                            pending.finished = true
-                        }
-                    }
                     result
                         .map_err(|e| {
                             this.show_error_toast(if stage { "add" } else { "reset" }, e, cx);
@@ -2580,12 +2521,7 @@ impl GitPanel {
         message.push('\n');
     }
 
-    fn schedule_update(
-        &mut self,
-        clear_pending: bool,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
+    fn schedule_update(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         let handle = cx.entity().downgrade();
         self.reopen_commit_buffer(window, cx);
         self.update_visible_entries_task = cx.spawn_in(window, async move |_, cx| {
@@ -2593,9 +2529,6 @@ impl GitPanel {
             if let Some(git_panel) = handle.upgrade() {
                 git_panel
                     .update_in(cx, |git_panel, window, cx| {
-                        if clear_pending {
-                            git_panel.clear_pending();
-                        }
                         git_panel.update_visible_entries(window, cx);
                     })
                     .ok();
@@ -2644,10 +2577,6 @@ impl GitPanel {
         .detach_and_log_err(cx);
     }
 
-    fn clear_pending(&mut self) {
-        self.pending.retain(|v| !v.finished)
-    }
-
     fn update_visible_entries(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         let path_style = self.project.read(cx).path_style(cx);
         let bulk_staging = self.bulk_staging.take();
@@ -2690,11 +2619,11 @@ impl GitPanel {
             let is_new = entry.status.is_created();
             let staging = entry.status.staging();
 
-            if self.pending.iter().any(|pending| {
-                pending.target_status == TargetStatus::Reverted
-                    && !pending.finished
-                    && pending.contains_path(&entry.repo_path)
-            }) {
+            if let Some(pending) = repo.pending_ops_for_path(&entry.repo_path)
+                && pending.ops.iter().any(|op| {
+                    op.git_status == pending_op::GitStatus::Reverted && op.finished_or_skipped()
+                })
+            {
                 continue;
             }
 
@@ -2735,30 +2664,35 @@ impl GitPanel {
             }
         }
 
-        let mut pending_staged_count = 0;
-        let mut last_pending_staged = None;
-        let mut pending_status_for_single_staged = None;
-        for pending in self.pending.iter() {
-            if pending.target_status == TargetStatus::Staged {
-                pending_staged_count += pending.entries.len();
-                last_pending_staged = pending.entries.first().cloned();
-            }
-            if let Some(single_staged) = &single_staged_entry
-                && pending.contains_path(&single_staged.repo_path)
+        if conflict_entries.is_empty() {
+            if staged_count == 1
+                && let Some(entry) = single_staged_entry.as_ref()
             {
-                pending_status_for_single_staged = Some(pending.target_status);
-            }
-        }
-
-        if conflict_entries.is_empty() && staged_count == 1 && pending_staged_count == 0 {
-            match pending_status_for_single_staged {
-                Some(TargetStatus::Staged) | None => {
-                    self.single_staged_entry = single_staged_entry;
+                if let Some(ops) = repo.pending_ops_for_path(&entry.repo_path) {
+                    if ops.staged() {
+                        self.single_staged_entry = single_staged_entry;
+                    }
                 }
-                _ => {}
+            } else if repo
+                .pending_ops_by_path
+                .summary()
+                .item_summary
+                .staging_count
+                == 1
+            {
+                self.single_staged_entry = repo.pending_ops_by_path.iter().find_map(|ops| {
+                    if ops.staging() {
+                        repo.status_for_path(&ops.repo_path)
+                            .map(|status| GitStatusEntry {
+                                repo_path: ops.repo_path.clone(),
+                                status: status.status,
+                                staging: StageStatus::Staged,
+                            })
+                    } else {
+                        None
+                    }
+                });
             }
-        } else if conflict_entries.is_empty() && pending_staged_count == 1 {
-            self.single_staged_entry = last_pending_staged;
         }
 
         if conflict_entries.is_empty() && changed_entries.len() == 1 {
@@ -2807,7 +2741,10 @@ impl GitPanel {
             && let Some(index) = bulk_staging_anchor_new_index
             && let Some(entry) = self.entries.get(index)
             && let Some(entry) = entry.status_entry()
-            && self.entry_staging(entry).unwrap_or(entry.staging) == StageStatus::Staged
+            && repo
+                .pending_ops_for_path(&entry.repo_path)
+                .map(|ops| ops.staging() || ops.staged())
+                .unwrap_or(false)
         {
             self.bulk_staging = bulk_staging;
         }
@@ -2853,51 +2790,29 @@ impl GitPanel {
                 continue;
             };
             self.entry_count += 1;
+            let is_staging_or_staged = repo
+                .pending_ops_for_path(&status_entry.repo_path)
+                .map(|ops| ops.staging() || ops.staged())
+                .unwrap_or(false);
             if repo.had_conflict_on_last_merge_head_change(&status_entry.repo_path) {
                 self.conflicted_count += 1;
-                if self
-                    .entry_staging(status_entry)
-                    .unwrap_or(status_entry.staging)
-                    .has_staged()
-                {
+                if is_staging_or_staged {
                     self.conflicted_staged_count += 1;
                 }
             } else if status_entry.status.is_created() {
                 self.new_count += 1;
-                if self
-                    .entry_staging(status_entry)
-                    .unwrap_or(status_entry.staging)
-                    .has_staged()
-                {
+                if is_staging_or_staged {
                     self.new_staged_count += 1;
                 }
             } else {
                 self.tracked_count += 1;
-                if self
-                    .entry_staging(status_entry)
-                    .unwrap_or(status_entry.staging)
-                    .has_staged()
-                {
+                if is_staging_or_staged {
                     self.tracked_staged_count += 1;
                 }
             }
         }
     }
 
-    fn entry_staging(&self, entry: &GitStatusEntry) -> Option<StageStatus> {
-        for pending in self.pending.iter().rev() {
-            if pending.contains_path(&entry.repo_path) {
-                match pending.target_status {
-                    TargetStatus::Staged => return Some(StageStatus::Staged),
-                    TargetStatus::Unstaged => return Some(StageStatus::Unstaged),
-                    TargetStatus::Reverted => continue,
-                    TargetStatus::Unchanged => continue,
-                }
-            }
-        }
-        None
-    }
-
     pub(crate) fn has_staged_changes(&self) -> bool {
         self.tracked_staged_count > 0
             || self.new_staged_count > 0
@@ -3735,10 +3650,12 @@ impl GitPanel {
         let ix = self.entry_by_path(&repo_path, cx)?;
         let entry = self.entries.get(ix)?;
 
-        let status = entry.status_entry()?;
-        let entry_staging = self.entry_staging(status).unwrap_or(status.staging);
+        let is_staging_or_staged = repo
+            .pending_ops_for_path(&repo_path)
+            .map(|ops| ops.staging() || ops.staged())
+            .unwrap_or(false);
 
-        let checkbox = Checkbox::new("stage-file", entry_staging.as_bool().into())
+        let checkbox = Checkbox::new("stage-file", is_staging_or_staged.into())
             .disabled(!self.has_write_access(cx))
             .fill()
             .elevation(ElevationIndex::Surface)
@@ -4030,8 +3947,17 @@ impl GitPanel {
         let checkbox_id: ElementId =
             ElementId::Name(format!("entry_{}_{}_checkbox", display_name, ix).into());
 
-        let entry_staging = self.entry_staging(entry).unwrap_or(entry.staging);
-        let mut is_staged: ToggleState = entry_staging.as_bool().into();
+        let active_repo = self
+            .project
+            .read(cx)
+            .active_repository(cx)
+            .expect("active repository must be set");
+        let repo = active_repo.read(cx);
+        let is_staging_or_staged = repo
+            .pending_ops_for_path(&entry.repo_path)
+            .map(|ops| ops.staging() || ops.staged())
+            .unwrap_or(false);
+        let mut is_staged: ToggleState = is_staging_or_staged.into();
         if self.show_placeholders && !self.has_staged_changes() && !entry.status.is_created() {
             is_staged = ToggleState::Selected;
         }
@@ -4150,9 +4076,11 @@ impl GitPanel {
                                 }
                             })
                             .tooltip(move |_window, cx| {
-                                let is_staged = entry_staging.is_fully_staged();
-
-                                let action = if is_staged { "Unstage" } else { "Stage" };
+                                let action = if is_staging_or_staged {
+                                    "Unstage"
+                                } else {
+                                    "Stage"
+                                };
                                 let tooltip_name = action.to_string();
 
                                 Tooltip::for_action(tooltip_name, &ToggleStaged, cx)

crates/project/src/git_store.rs 🔗

@@ -5240,7 +5240,7 @@ impl Repository {
         cx.spawn(async move |this, cx| {
             let job_status = match f(this.clone(), cx).await {
                 Ok(()) => pending_op::JobStatus::Finished,
-                Err(err) if err.is::<Canceled>() => pending_op::JobStatus::Canceled,
+                Err(err) if err.is::<Canceled>() => pending_op::JobStatus::Skipped,
                 Err(err) => return Err(err),
             };
 

crates/project/src/git_store/pending_op.rs 🔗

@@ -1,4 +1,3 @@
-use futures::channel::oneshot::Canceled;
 use git::repository::RepoPath;
 use std::ops::Add;
 use sum_tree::{ContextLessSummary, Item, KeyedItem};
@@ -16,7 +15,7 @@ pub enum GitStatus {
 pub enum JobStatus {
     Started,
     Finished,
-    Canceled,
+    Skipped,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -35,6 +34,8 @@ pub struct PendingOp {
 #[derive(Clone, Debug)]
 pub struct PendingOpsSummary {
     pub max_id: PendingOpId,
+    pub staged_count: usize,
+    pub staging_count: usize,
 }
 
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
@@ -48,6 +49,8 @@ impl Item for PendingOps {
             max_path: self.repo_path.0.clone(),
             item_summary: PendingOpsSummary {
                 max_id: self.ops.last().map(|op| op.id).unwrap_or_default(),
+                staged_count: self.staged() as usize,
+                staging_count: self.staging() as usize,
             },
         }
     }
@@ -57,11 +60,15 @@ impl ContextLessSummary for PendingOpsSummary {
     fn zero() -> Self {
         Self {
             max_id: PendingOpId::default(),
+            staged_count: 0,
+            staging_count: 0,
         }
     }
 
     fn add_summary(&mut self, summary: &Self) {
         self.max_id = summary.max_id;
+        self.staged_count += summary.staged_count;
+        self.staging_count += summary.staging_count;
     }
 }
 
@@ -96,10 +103,34 @@ impl PendingOps {
     pub fn op_by_id_mut(&mut self, id: PendingOpId) -> Option<&mut PendingOp> {
         self.ops.iter_mut().find(|op| op.id == id)
     }
+
+    /// File is staged if the last job is finished and has status Staged.
+    pub fn staged(&self) -> bool {
+        if let Some(last) = self.ops.last() {
+            if last.git_status == GitStatus::Staged && last.finished() {
+                return true;
+            }
+        }
+        false
+    }
+
+    /// File is staged if the last job is not finished and has status Staged.
+    pub fn staging(&self) -> bool {
+        if let Some(last) = self.ops.last() {
+            if last.git_status == GitStatus::Staged && !last.finished() {
+                return true;
+            }
+        }
+        false
+    }
 }
 
-impl From<Canceled> for JobStatus {
-    fn from(_err: Canceled) -> Self {
-        Self::Canceled
+impl PendingOp {
+    pub fn finished(&self) -> bool {
+        self.job_status == JobStatus::Finished
+    }
+
+    pub fn finished_or_skipped(&self) -> bool {
+        self.job_status == JobStatus::Finished || self.job_status == JobStatus::Skipped
     }
 }