Merge pull request #10449 from Byron/fix

Ensure that the target branch isn't showing up as workspace segment.

Author: Byron

crates/but-graph/src/init/post.rs

@@ -410,6 +410,61 @@ impl Graph {
             return Ok(());
         };
 
+        // If the target branch made it into a stack segment which can also be named differently by preferably a ref with metadata,
+        // then do that instead. Let's prevent it from showing up as its own node as this leads to undesirable workspaces.
+        if let Some((target_segment_id, local_tracking_segment_of_target)) =
+            ws_target.as_ref().and_then(|t| {
+                self[t.segment_index]
+                    .sibling_segment_id
+                    .map(|sidx| (t.segment_index, sidx))
+            })
+            && let Some(segment_id) = ws_stacks.iter().find_map(|s| {
+                s.segments.iter().find_map(|s| {
+                    if s.id != local_tracking_segment_of_target {
+                        return None;
+                    }
+                    let first_commit = s.commits.first()?;
+                    (!first_commit.refs.is_empty()).then_some(s.id)
+                })
+            })
+        {
+            let s = &mut self[segment_id];
+            let c = s
+                .commits
+                .first_mut()
+                .expect("segment was chosen because it has at least one commit");
+            let mut candidates = c.refs.clone();
+            candidates.sort_by(|a, b| {
+                meta.branch_opt(a.as_ref())
+                    .ok()
+                    .map(|md| md.is_some())
+                    .cmp(&meta.branch_opt(b.as_ref()).ok().map(|md| md.is_some()))
+                    .then_with(|| a.cmp(b))
+            });
+            let candidate = candidates
+                .pop()
+                .expect("at least one ref or we wouldn't be here");
+            let current_name = s.ref_name.take();
+            let index_to_replace = c
+                .refs
+                .iter()
+                .position(|rn| rn == &candidate)
+                .expect("candidate is from a clone of 'refs', so must be contained");
+            // effectively swap the names
+            s.metadata = meta
+                .branch_opt(candidate.as_ref())?
+                .map(SegmentMetadata::Branch);
+            s.ref_name = Some(candidate);
+            if let Some(rn) = current_name {
+                c.refs[index_to_replace] = rn;
+            }
+
+            // Make sure we dissolve the sibling relationship for correctness, the node is now not related
+            // to the target branch anymore.
+            s.sibling_segment_id = None;
+            self[target_segment_id].sibling_segment_id = None;
+        }
+
         // Setup independent stacks, first by looking at potential bases.
         let candidates = self.candidates_for_independent_branches_in_workspace(
             ws_sidx,

crates/but-graph/tests/fixtures/scenarios.sh

@@ -1100,5 +1100,14 @@ EOF
       create_workspace_commit_once A
     )
   )
+
+  git init local-target-ahead-and-on-stack-tip
+  (cd local-target-ahead-and-on-stack-tip
+    commit init
+    setup_target_to_match_main
+    commit A
+    git checkout -b A
+    create_workspace_commit_once A
+  )
 )
 

crates/but-graph/tests/graph/init/with_workspace.rs

@@ -1497,6 +1497,59 @@ fn stacked_rebased_remotes() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+fn target_with_remote_on_stack_tip() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/local-target-ahead-and-on-stack-tip")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    * dd0cca8 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    * e255adc (main, A) A
+    * fafd9d0 (origin/main) init
+    ");
+    add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &[]);
+
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    insta::assert_snapshot!(graph_tree(&graph), @r"
+    └── 👉📕►►►:0[0]:gitbutler/workspace
+        └── ·dd0cca8 (⌂|🏘|1)
+            └── 📙►:2[1]:A
+                └── ·e255adc (⌂|🏘|11) ►main
+                    └── ►:1[2]:origin/main
+                        └── ·fafd9d0 (⌂|🏘|✓|11)
+    ");
+
+    // The main branch is not present, as it's the target.
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main on fafd9d0
+    └── ≡📙:2:A on fafd9d0
+        └── 📙:2:A
+            └── ·e255adc (🏘️) ►main
+    ");
+
+    // But mention it if it's in the workspace. It should retain order.
+    add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &["main"]);
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main on fafd9d0
+    └── ≡📙:2:A on fafd9d0
+        ├── 📙:2:A
+        └── 📙:3:main <> origin/main →:1:⇡1
+            └── ·e255adc (🏘️)
+    ");
+
+    // But mention it if it's in the workspace. It should retain order - inverting the order is fine.
+    add_stack_with_segments(&mut meta, 1, "main", StackState::InWorkspace, &["A"]);
+    let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?;
+    // TODO: fix order, main should be first.
+    insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r"
+    📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main on fafd9d0
+    └── ≡📙:2:A on fafd9d0
+        ├── 📙:2:A
+        └── 📙:3:main <> origin/main →:1:⇡1
+            └── ·e255adc (🏘️)
+    ");
+    Ok(())
+}
+
 #[test]
 fn disambiguate_by_remote() -> anyhow::Result<()> {
     let (repo, mut meta) = read_only_in_memory_scenario("ws/disambiguate-by-remote")?;