fix commits_ahead computation in Target.

Previously it was dependent on counting remote commits, but these
are dependent on the respective local tracking branch not being caught
up, but that really is up to the user and shouldn't affect the count.

Author: Byron

crates/but-graph/src/projection/workspace.rs

@@ -97,28 +97,65 @@ pub struct Target {
 impl Target {
     /// Return `None` if `ref_name` wasn't found as segment in `graph`.
     /// This can happen if a reference is configured, but not actually present as reference.
-    fn from_ref_name(ref_name: &gix::refs::FullName, graph: &Graph) -> Option<Self> {
+    /// Note that `commits_ahead` isn't set yet, see [`Self::compute_and_set_commits_ahead()`].
+    fn from_ref_name_without_commits_ahead(
+        ref_name: &gix::refs::FullName,
+        graph: &Graph,
+    ) -> Option<Self> {
         let target_segment = graph.inner.node_indices().find_map(|n| {
             let s = &graph[n];
             (s.ref_name.as_ref() == Some(ref_name)).then_some(s)
         })?;
         Some(Target {
             ref_name: ref_name.to_owned(),
             segment_index: target_segment.id,
-            commits_ahead: {
-                // Find all remote commits but stop traversing when there is segments without remotes.
-                let mut count = 0;
-                graph.visit_all_segments_until(target_segment.id, Direction::Outgoing, |s| {
-                    let remote_commits = s.commits.iter().filter(|c| c.flags.is_remote()).count();
-                    count += remote_commits;
-                    remote_commits != s.commits.len()
-                });
-                count
-            },
+            commits_ahead: 0,
+        })
+    }
+
+    fn compute_and_set_commits_ahead(
+        &mut self,
+        graph: &Graph,
+        lower_bound_segment: Option<SegmentIndex>,
+    ) {
+        let lower_bound = lower_bound_segment.map(|sidx| (sidx, graph[sidx].generation));
+        self.commits_ahead = 0;
+        Self::visit_upstream_commits(graph, self.segment_index, lower_bound, |s| {
+            self.commits_ahead += s.commits.len();
         })
     }
 }
 
+/// Utilities
+impl Target {
+    /// Visit all segments whose commits would be considered 'upstream', or part of the target branch
+    /// whose tip is identified with `target_segment`. The `lower_bound_segment_and_generation` is another way
+    /// to stop the traversal.
+    pub fn visit_upstream_commits(
+        graph: &Graph,
+        target_segment: SegmentIndex,
+        lower_bound_segment_and_generation: Option<(SegmentIndex, usize)>,
+        mut visit: impl FnMut(&Segment),
+    ) {
+        graph.visit_all_segments_until(target_segment, Direction::Outgoing, |s| {
+            let prune = true;
+            if lower_bound_segment_and_generation.is_some_and(
+                |(lower_bound, lower_bound_generation)| {
+                    s.id == lower_bound || s.generation > lower_bound_generation
+                },
+            ) || s
+                .commits
+                .iter()
+                .any(|c| c.flags.contains(CommitFlags::InWorkspace))
+            {
+                return prune;
+            }
+            visit(s);
+            !prune
+        });
+    }
+}
+
 impl Graph {
     /// Analyse the current graph starting at its [entrypoint](Self::lookup_entrypoint()).
     ///
@@ -196,9 +233,9 @@ impl Graph {
             id: ws_tip_segment.id,
             kind,
             stacks: vec![],
-            target: metadata
-                .as_ref()
-                .and_then(|md| Target::from_ref_name(md.target_ref.as_ref()?, self)),
+            target: metadata.as_ref().and_then(|md| {
+                Target::from_ref_name_without_commits_ahead(md.target_ref.as_ref()?, self)
+            }),
             extra_target: self.extra_target,
             metadata,
             lower_bound_segment_id: None,
@@ -352,6 +389,9 @@ impl Graph {
             );
         }
 
+        if let Some(target) = ws.target.as_mut() {
+            target.compute_and_set_commits_ahead(self, ws.lower_bound_segment_id);
+        }
         ws.mark_remote_reachability()?;
         Ok(ws)
     }
@@ -834,6 +874,8 @@ impl std::fmt::Debug for Workspace<'_> {
             .field("id", &self.id.index())
             .field("stacks", &self.stacks)
             .field("metadata", &self.metadata)
+            .field("target", &self.target)
+            .field("extra_target", &self.extra_target)
             .finish()
     }
 }

crates/but-workspace/src/changeset.rs

@@ -16,7 +16,6 @@ use bstr::BString;
 use but_core::{ChangeState, commit::TreeKind};
 use gix::diff::tree::recorder::Change;
 use gix::{ObjectId, Repository, object::tree::EntryKind, prelude::ObjectIdExt};
-use itertools::Itertools;
 use std::ops::Deref;
 use std::{
     borrow::Borrow,
@@ -59,10 +58,7 @@ impl RefInfo {
             .target
             .as_ref()
             .map(|t| t.segment_index)
-            .into_iter()
-            .chain(self.extra_target)
-            .sorted_by_key(|sidx| graph[*sidx].generation)
-            .next();
+            .or(self.extra_target);
         let mut upstream_commits = Vec::new();
         let Some(target_tip) = topmost_target_sidx else {
             // Without any notion of 'target' we can't do anything here.

crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/exhaustive_with_squash_merges.rs

@@ -757,7 +757,7 @@ Ok(
                     "refs/remotes/origin/main",
                 ),
                 segment_index: NodeIndex(1),
-                commits_ahead: 3,
+                commits_ahead: 5,
             },
         ),
         extra_target: None,
@@ -880,7 +880,7 @@ Ok(
                     "refs/remotes/origin/main",
                 ),
                 segment_index: NodeIndex(1),
-                commits_ahead: 2,
+                commits_ahead: 7,
             },
         ),
         extra_target: None,

crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/integrate_with_merges.rs

@@ -277,7 +277,7 @@ fn remote_diverged_merge() -> anyhow::Result<()> {
                         "refs/remotes/origin/main",
                     ),
                     segment_index: NodeIndex(1),
-                    commits_ahead: 0,
+                    commits_ahead: 2,
                 },
             ),
             extra_target: Some(
@@ -416,7 +416,7 @@ fn remote_behind_merge_no_ff() -> anyhow::Result<()> {
                         "refs/remotes/origin/main",
                     ),
                     segment_index: NodeIndex(1),
-                    commits_ahead: 0,
+                    commits_ahead: 1,
                 },
             ),
             extra_target: Some(
@@ -552,7 +552,7 @@ fn remote_ahead_merge_ff() -> anyhow::Result<()> {
                         "refs/remotes/origin/main",
                     ),
                     segment_index: NodeIndex(1),
-                    commits_ahead: 0,
+                    commits_ahead: 1,
                 },
             ),
             extra_target: Some(

crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/integrate_with_rebase.rs

@@ -62,7 +62,7 @@ fn two_commits_rebased_onto_target() -> anyhow::Result<()> {
                         "refs/remotes/origin/main",
                     ),
                     segment_index: NodeIndex(1),
-                    commits_ahead: 0,
+                    commits_ahead: 5,
                 },
             ),
             extra_target: None,
@@ -137,7 +137,7 @@ fn two_commits_rebased_onto_target_with_changeset_check() -> anyhow::Result<()>
                         "refs/remotes/origin/main",
                     ),
                     segment_index: NodeIndex(1),
-                    commits_ahead: 0,
+                    commits_ahead: 5,
                 },
             ),
             extra_target: None,