Merge pull request #10606 from Byron/next

v3 apply/unapply: MVP

Author: Byron

crates/but-core/src/ref_metadata.rs

@@ -60,20 +60,21 @@ impl Workspace {
 
     /// Insert `branch` as new stack if it's not yet contained in the workspace and if `order` is not `None` or push
     /// it to the end of the stack list.
-    /// If a new stack is created, it's considered to be *in* the workspace. If there is an existing stack, it will
-    /// also be put *in* the workspace as a side effect.
+    /// If a new stack is created, it's considered to be *in* the workspace. If there is an existing stack, it
+    /// may also be marked as *outside the workspace*, we do not change this.
     /// Note that `order` is only relevant at insertion time, not if the branch already exists.
-    /// Returns `true` if the ref was newly added, or `false` if it already existed.
+    /// Returns `(stack_id, segment_idx)` of the stack that was either newly created, or already present.
+    /// Note that `segment_idx` may be non-0 if `branch` already existed as segment, and the caller has to
+    /// deal with this.
     pub fn add_or_insert_new_stack_if_not_present(
         &mut self,
         branch: &FullNameRef,
         order: Option<usize>,
-    ) -> bool {
-        if let Some((stack_idx, _)) =
+    ) -> (usize, usize) {
+        if let Some(owners) =
             self.find_owner_indexes_by_name(branch, StackKind::AppliedAndUnapplied)
         {
-            self.stacks[stack_idx].in_workspace = true;
-            return false;
+            return owners;
         };
 
         let stack = WorkspaceStack {
@@ -84,15 +85,18 @@ impl Workspace {
                 archived: false,
             }],
         };
-        match order.map(|idx| idx.min(self.stacks.len())) {
+        let stack_idx = match order.map(|idx| idx.min(self.stacks.len())) {
             None => {
+                let idx = self.stacks.len();
                 self.stacks.push(stack);
+                idx
             }
             Some(existing_index) => {
                 self.stacks.insert(existing_index, stack);
+                existing_index
             }
-        }
-        true
+        };
+        (stack_idx, 0)
     }
 
     /// Insert `branch` as new segment if it's not yet contained in the workspace,

crates/but-core/tests/core/ref_metadata.rs

@@ -8,19 +8,31 @@ mod workspace {
         assert_eq!(ws.stacks.len(), 0);
 
         let a_ref = r("refs/heads/A");
-        assert!(ws.add_or_insert_new_stack_if_not_present(a_ref, Some(100)));
-        assert!(!ws.add_or_insert_new_stack_if_not_present(a_ref, Some(200)));
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(100)),
+            (0, 0)
+        );
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(a_ref, Some(200)),
+            (0, 0)
+        );
         assert_eq!(ws.stacks.len(), 1);
 
         let b_ref = r("refs/heads/B");
-        assert!(ws.add_or_insert_new_stack_if_not_present(b_ref, Some(0)));
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(b_ref, Some(0)),
+            (0, 0)
+        );
         assert_eq!(
             ws.stack_names(AppliedAndUnapplied).collect::<Vec<_>>(),
             [b_ref, a_ref]
         );
 
         let c_ref = r("refs/heads/C");
-        assert!(ws.add_or_insert_new_stack_if_not_present(c_ref, None));
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(c_ref, None),
+            (2, 0)
+        );
         assert_eq!(
             ws.stack_names(AppliedAndUnapplied).collect::<Vec<_>>(),
             [b_ref, a_ref, c_ref]
@@ -55,7 +67,10 @@ mod workspace {
             None,
             "anchor doesn't exist"
         );
-        assert!(ws.add_or_insert_new_stack_if_not_present(a_ref, None));
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(a_ref, None),
+            (0, 0)
+        );
         assert_eq!(
             ws.insert_new_segment_above_anchor_if_not_present(b_ref, a_ref),
             Some(true),
@@ -73,6 +88,12 @@ mod workspace {
             Some(true)
         );
 
+        assert_eq!(
+            ws.add_or_insert_new_stack_if_not_present(a_ref, None),
+            (0, 2),
+            "adding a new stack can 'fail' if the segment is already present, but not as stack tip"
+        );
+
         insta::assert_snapshot!(but_testsupport::sanitize_uuids_and_timestamps(format!("{ws:#?}")), @r#"
         Workspace {
             ref_info: RefInfo { created_at: None, updated_at: None },

crates/but-graph/src/ref_metadata_legacy.rs

@@ -220,13 +220,20 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
     }
 
     fn branch(&self, ref_name: &FullNameRef) -> anyhow::Result<Self::Handle<Branch>> {
-        let Some((stack, branch)) = self.data().branches.values().find_map(|stack| {
-            stack.heads.iter().find_map(|branch| {
-                full_branch_name(branch.name.as_str()).and_then(|full_name| {
-                    (full_name.as_ref() == ref_name).then_some((stack, branch))
+        let Some((stack, branch)) = self
+            .data()
+            .branches
+            .values()
+            // There shouldn't be duplication, but let's be sure it's deterministic if it is.
+            .sorted_by_key(|s| s.order)
+            .find_map(|stack| {
+                stack.heads.iter().find_map(|branch| {
+                    full_branch_name(branch.name.as_str()).and_then(|full_name| {
+                        (full_name.as_ref() == ref_name).then_some((stack, branch))
+                    })
                 })
             })
-        }) else {
+        else {
             return Ok(VBTomlMetadataHandle {
                 is_default: true,
                 ref_name: ref_name.to_owned(),
@@ -373,6 +380,12 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
             vb_stack.heads.reverse()
         }
 
+        for (stack_idx, stack) in value.stacks.iter().enumerate() {
+            if let Some(vb_stack) = self.data_mut().branches.get_mut(&stack.id) {
+                vb_stack.order = stack_idx;
+            }
+        }
+
         let stacks_to_delete: Vec<_> = self
             .data()
             .branches