Revert "supergit: work on implementing branching iterators"

This commit reverts the following commits:

  * b2681a59d5dfae8afed0a6a8d46210133667532c.
  * a35411dc74c436b8c31878304e8d5447862e1dfe.

As already mentioned in the commit message of a2513a, this design
decision turned out to be a bad one: instead of having the iterator
abstraction branch internally, an API is required to control the flow
of iterators externally.  This way users can opt-into complexity,
instead of having to opt-out.

I opted to revert the commits, instead of trying to untangle all the
changes made in these two commits, to avoid breaking any of the code.

Signed-off-by: Katharina Fey <kookie@spacekookie.de>

1 files changed, 10 insertions, 209 deletions

diff --git a/apps/servers/octopus/supergit/src/branch.rs b/apps/servers/octopus/supergit/src/branch.rs
index 7d62941bdb51..48f66ac47112 100644
--- a/apps/servers/octopus/supergit/src/branch.rs
+++ b/apps/servers/octopus/supergit/src/branch.rs
@@ -1,15 +1,8 @@
 //! Type system for working with git branches
 
 use crate::{Commit, HashId};
-use atomptr::AtomPtr;
 use git2::Repository;
-use std::{
-    mem,
-    sync::{
-        atomic::{AtomicUsize, Ordering},
-        Arc,
-    },
-};
+use std::{mem, sync::Arc};
 
 /// Abstraction for a branch history slice
 ///
@@ -133,8 +126,6 @@ impl Branch {
 /// handles.  This means that without explicitly branching, this
 /// iterator is first-parent.
 pub struct BranchIter {
-    mode: AtomPtr<IterMode>,
-    splits: IterData,
     repo: Arc<Repository>,
     curr: Option<HashId>,
     limit: SegLimit,
@@ -144,8 +135,6 @@ impl BranchIter {
     /// Create a new branch segment iterator
     fn new(repo: Arc<Repository>, last: HashId, limit: SegLimit) -> Self {
         Self {
-            mode: AtomPtr::new(IterMode::FirstParent),
-            splits: IterData::new(),
             repo,
             curr: Some(last),
             limit,
@@ -184,75 +173,12 @@ impl BranchIter {
         }
     }
 
-    /// Determine which commit should be looked at next
+    /// Get the current commit
     ///
-    /// FirstParent and DepthFirst iterators will take the next
-    /// first_parent if available.  DepthFirst iterators will fall
-    /// back to the last split point, if no `first_parent()` exists.
-    /// BreathFirst iterators will always prefer a new branch over
-    /// first_parent, and jump back to the last split if there are no
-    /// parents.
-    fn determine_next(&mut self, current: Commit) -> Commit {
-        let mode = &**self.mode.get_ref();
-        self.curr = match mode {
-            IterMode::FirstParent => match current.first_parent() {
-                Some(p1) => Some(p1.id),
-                None => None,
-            },
-            // DepthFirst iterates normally until we hit the end of
-            // the branch, then we "jump back" to an earlier commit.
-            IterMode::DepthFirst => match current.first_parent() {
-                Some(p1) => Some(p1.id),
-                None => {
-                    // Get the last split point.  If there are no
-                    // more, terminate the iterator.  If there are,
-                    // increment brnum and keep going.  If brnum is
-                    // higher than the parent count, call this
-                    // function again to get the next split point (and
-                    // reset brnum)
-                    self.splits.next().map(|id| {
-                        let brnum = self.splits.incr_brnum();
-
-                        // ALSO: when brnum is LOWER than parent point,
-                        // re-insert split, to allow the commit to be
-                        // jumped to again!
-                        if brnum < current.parent_count() {
-                            self.splits.re_insert(id.clone());
-                        }
-
-                        let com = self.find_commit(&id).unwrap();
-                        if brnum > current.parent_count() {
-                            self.splits.reset_brnum();
-                            self.determine_next(com).id
-                        } else {
-                            id
-                        }
-                    })
-                }
-            },
-            // // When there is only one parent, chose that parent.  When
-            // // there is none, jump back to the last split point,
-            // // according to brnum.  If brnum is then greater than the
-            // // number of branches, reset brnum and re-call this
-            // // function.
-            // IterMode::BreadthFirst if current.parent_count() <= 1 => match current.first_parent() {
-            //     Some(p1) => Some(p1.id),
-            //     None => self.splits.next().map(|id| {
-            //         let brnum = self.splits.incr_brnum();
-            //     }),
-            // },
-
-            // // When there are is more than 1 parent, set this commit
-            // // as a split point, and chose the last parent commit as
-            // // the next commit to walk.  This shifts the active branch
-            // // over.  Important: we set brnum to the _highest_ branch,
-            // // and iterate inwards.
-            // IterMode::BreadthFirst => {
-
-            // },
-            _ => todo!(),
-        };
-
+    /// This function looks either at the "curr" field, or takes the
+    /// ID from `cmd`, if it is set to `IterCmd::Jump(...)`, which
+    /// indicates that the previous commit was a merge, and we need to escape
+    fn set_next(&mut self, current: Commit) -> Commit {
         self.curr = match current.first_parent() {
             Some(p1) => Some(p1.id),
             None => None,
@@ -266,28 +192,9 @@ impl Iterator for BranchIter {
     type Item = BranchCommit;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let mode = &**self.mode.get_ref();
-
-        let id = match mode {
-            // When iterating first-parent OR when going depth first,
-            // while the current branch still has children, take the
-            // next child of the current branch.
-            IterMode::FirstParent => mem::replace(&mut self.curr, None),
-            IterMode::DepthFirst | IterMode::BreadthFirst if self.curr.is_some() => {
-                mem::replace(&mut self.curr, None)
-            }
-            // When going both BreadthFirst or DepthFirst, reaching
-            // the end of the current branch means getting the last
-            // split point.  The difference between these two
-            // strategies is in how the split points and "current
-            // point" are stored.
-            _ if self.curr.is_none() => self.splits.next(),
-            _ => unreachable!(), // can't be reached
-        };
-
-        // The chain of events
-        id.and_then(|id| self.find_commit(&id))
-            .map(|c| self.determine_next(c))
+        mem::replace(&mut self.curr, None)
+            .and_then(|id| self.find_commit(&id))
+            .map(|c| self.set_next(c))
             .and_then(|c| match self.limit {
                 SegLimit::None => Some(c),
                 SegLimit::Commit(ended, _) if ended => None,
@@ -308,27 +215,7 @@ impl Iterator for BranchIter {
             .map(|c| self.make_branch_commit(c))
     }
 }
-
-/// Specify the mode of a branch iterator
-///
-/// The default value is `FirstParent`, meaning that merged branches
-/// will create a new `Branch` handle that the user must iterate
-/// manually.
-///
-/// This is a reasonable default, but means that history searches for
-/// files become more tedious.  To improve this use-case, iterators
-/// can internally be set to change their iteration behaviour, meaning
-/// that returned commits are always `BranchCommit::Commit`, and can
-pub enum IterMode {
-    /// Default value, iterating only first-parent commits
-    FirstParent,
-    /// Iterate a branch to completion, before picking the next
-    DepthFirst,
-    /// Iterate branches as they come up
-    BreadthFirst,
-}
-
-/// Limiter applied to a branch segment
+/// Thelimit applied to a branch segment
 pub enum SegLimit {
     /// No limit, enumerating all children
     None,
@@ -373,89 +260,3 @@ impl BranchCommit {
         }
     }
 }
-
-/// Additional iterator data
-///
-/// This structure tracks split points on the iterator.  When
-/// traversing a branch, the iterator can reach branch points.  While
-/// in `IterMode::FirstParent`, these are irrelevant.  However: when
-/// iterating breadth or depth first these need to be tracked.
-///
-/// ## Depth first
-///
-/// - When a branch is encountered, append it to this data set.
-/// - When the end of the current branch is reached, get the next
-///   split point to resume from
-///
-/// ## Breadth first
-///
-/// - When a branch is encountered, add the previous commit to this set
-/// - When reaching the end of a branch, get the next split point to
-///   resume from
-///
-/// ---
-///
-/// In essense, the usage of this structure for BreadthFirst and
-/// DepthFirst are inverted!
-struct IterData {
-    /// Split points on the iterator
-    splits: AtomPtr<Vec<HashId>>,
-    /// Additional data for octopus merges
-    brnum: AtomicUsize,
-}
-
-impl IterData {
-    fn new() -> Self {
-        Self {
-            splits: AtomPtr::new(vec![]),
-            brnum: AtomicUsize::new(0),
-        }
-    }
-
-    /// Check if the split set is empty
-    fn empty(&self) -> bool {
-        self.splits.get_ref().len() == 0
-    }
-
-    fn set_brnum(&self, num: usize) {
-        self.brnum.swap(num, Ordering::Relaxed);
-    }
-
-    fn incr_brnum(&self) -> usize {
-        self.brnum.fetch_add(1, Ordering::Relaxed) + 1
-    }
-
-    fn decr_brnum(&self) -> usize {
-        self.brnum.fetch_sub(1, Ordering::Relaxed) - 1
-    }
-
-    fn reset_brnum(&self) {
-        self.set_brnum(0);
-    }
-
-    fn append(&self, id: HashId) {
-        let mut vec = (**self.splits.get_ref()).clone();
-        let mut new = vec![id];
-        new.append(&mut vec);
-        self.splits.swap(new);
-    }
-
-    /// Insert a hashID to the front of the splits list
-    fn re_insert(&self, id: HashId) {
-        let mut vec = (**self.splits.get_ref()).clone();
-        vec.insert(0, id);
-        self.splits.swap(vec);
-    }
-
-    fn next(&self) -> Option<HashId> {
-        let mut vec = (**self.splits.get_ref()).clone();
-        let next = if vec.len() < 0 {
-            Some(vec.remove(0))
-        } else {
-            None
-        };
-
-        self.splits.swap(vec);
-        next
-    }
-}