graphlog: queue linear history per branch, emit at fork point (PoC) (…

…WIP)

May be buggy. Not yet ready for production use. I'm just dogfooding to see
if I like this more than Sapling's beautify_graph(), which reorders commits
more aggressively.

jj-vcs#242

src/commands/mod.rs

@@ -19,12 +19,12 @@ mod debug;
 mod git;
 mod operation;
 
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{hash_map, BTreeMap, HashMap, HashSet, VecDeque};
 use std::fmt::Debug;
 use std::io::{BufRead, Read, Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
-use std::{fs, io};
+use std::{fs, io, mem};
 
 use clap::builder::NonEmptyStringValueParser;
 use clap::parser::ValueSource;
@@ -1539,6 +1539,181 @@ fn cmd_status(
     Ok(())
 }
 
+// TODO: rewrite doc comments, move example to test
+/// Graph iterator adapter to group topological branches.
+///
+/// Basic idea is to build DFS-ish stack until fork point, and do that lazily.
+///
+/// ```
+/// J
+/// |   I
+/// | H |
+/// | G |
+/// F |\|
+/// | | E
+/// | D |
+/// C |/
+/// | B
+/// |/
+/// A
+/// ```
+///
+/// ```
+/// _: {J: []}
+/// J: {F: [J]}
+/// I: {F: [J], E: [I]}
+/// H: {F: [J], E: [I], G: [H]}
+/// G: {F: [J], E: [I], G: [H], D: [G*], E: [G*]}  # G is merge point
+///  : {F: [J], E: [G*, I], G: [H], D: [G*]}
+/// F: {C: [F, J], E: [G*, I], G: [H], D: [G*]}
+/// E: {C: [F, J], B: [E, G*, I], G: [H], D: [G*]}
+/// D: {C: [F, J], B: [E, G*, I], G: [H], B: [D, G*]}
+///  : {C: [F, J], B: [D, G*, E, G*, I], G: [H]}
+/// C: {A: [C, F, J], B: [D, G*, E, G*, I], G: [H]}
+/// B: {A: [C, F, J], A: [B, D, G*, E, G*, I], G: [H]}
+///  : {A: [B, D, G*, E, G*, I, C, F, J], G: [H]}
+/// A: {-: [A, B, D, G*, E, G*, I, C, F, J], G: [H]}
+/// => J, F, C, I, H, G, E, D, B, A
+/// ```
+///
+/// ```
+/// J
+/// F
+/// C
+/// | I
+/// | | H
+/// | | G
+/// | |/|
+/// | E |
+/// | | D
+/// | |/
+/// | B
+/// |/
+/// A
+/// ```
+// TODO: move to separate module?
+#[derive(Clone, Debug)]
+struct TopoGroupedGraphIterator<I> {
+    input_iter: I,
+    emitting_root_id: Option<CommitId>,
+    // root_id: [descendants] for each linear sub graph
+    linearized: HashMap<Option<CommitId>, VecDeque<TopoGroupedGraphWorkItem>>,
+}
+
+#[derive(Clone, Debug)]
+enum TopoGroupedGraphWorkItem {
+    Data((CommitId, Vec<RevsetGraphEdge>)),
+    Ref(CommitId),
+}
+
+impl<I> TopoGroupedGraphIterator<I>
+where
+    I: Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)>,
+{
+    pub fn new(input_iter: I) -> Self {
+        TopoGroupedGraphIterator {
+            input_iter,
+            emitting_root_id: None,
+            linearized: HashMap::from([(None, VecDeque::new())]),
+        }
+    }
+
+    fn enqueue(&mut self) {
+        fn to_subroot_id(edge: &RevsetGraphEdge) -> Option<CommitId> {
+            match edge.edge_type {
+                RevsetGraphEdgeType::Missing => None,
+                RevsetGraphEdgeType::Direct | RevsetGraphEdgeType::Indirect => {
+                    Some(edge.target.clone())
+                }
+            }
+        }
+
+        while self
+            .linearized
+            .get(&self.emitting_root_id)
+            .unwrap()
+            .is_empty()
+        {
+            let (curr_id, edges) = if let Some((id, edges)) = self.input_iter.next() {
+                (id, edges)
+            } else {
+                return;
+            };
+            let curr_subroot_id = Some(curr_id.clone());
+            if edges.len() <= 1 {
+                // Not a merge, queue up linear sub graph.
+                let next_subroot_id = edges.first().and_then(to_subroot_id);
+                let was_curr_emitting = self.emitting_root_id == curr_subroot_id;
+                if self.emitting_root_id.is_none() || was_curr_emitting {
+                    self.emitting_root_id = next_subroot_id.clone();
+                }
+                let mut queue = self.linearized.remove(&curr_subroot_id).unwrap_or_default();
+                queue.push_back(TopoGroupedGraphWorkItem::Data((curr_id, edges)));
+                match self.linearized.entry(next_subroot_id) {
+                    hash_map::Entry::Occupied(mut entry) => {
+                        // At fork point, append or prepend sub graph, so each sub graph will be
+                        // emitted contiguously.
+                        let other = entry.get_mut();
+                        if was_curr_emitting {
+                            mem::swap(other, &mut queue);
+                        }
+                        other.extend(queue);
+                    }
+                    hash_map::Entry::Vacant(entry) => {
+                        entry.insert(queue);
+                    }
+                }
+            } else {
+                // Track each ancestor separately, and link to the current sub graph.
+                for edge in &edges {
+                    let next_subroot_id = to_subroot_id(edge);
+                    if self.emitting_root_id.is_none() || self.emitting_root_id == curr_subroot_id {
+                        self.emitting_root_id = next_subroot_id.clone();
+                    }
+                    let queue = self.linearized.entry(next_subroot_id).or_default();
+                    queue.push_back(TopoGroupedGraphWorkItem::Ref(curr_id.clone()));
+                }
+                // Current node and its descendants will be folded into one of the ancestors,
+                // but we don't know which one will be emitted first.
+                let queue = self.linearized.entry(curr_subroot_id).or_default();
+                queue.push_back(TopoGroupedGraphWorkItem::Data((curr_id, edges)));
+            }
+        }
+    }
+}
+
+impl<I> Iterator for TopoGroupedGraphIterator<I>
+where
+    I: Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)>,
+{
+    type Item = (CommitId, Vec<RevsetGraphEdge>);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        loop {
+            self.enqueue();
+            let queue = self.linearized.get_mut(&self.emitting_root_id).unwrap();
+            let item = if let Some(item) = queue.pop_front() {
+                item
+            } else {
+                assert_eq!(self.linearized.len(), 1);
+                return None;
+            };
+            match item {
+                TopoGroupedGraphWorkItem::Data(next) => return Some(next),
+                TopoGroupedGraphWorkItem::Ref(id) => {
+                    if let Some(mut other) = self.linearized.remove(&Some(id)) {
+                        let queue = self.linearized.get_mut(&self.emitting_root_id).unwrap();
+                        mem::swap(queue, &mut other);
+                        queue.append(&mut other);
+                    } else {
+                        // Would have been emitted by another branch
+                    }
+                }
+            }
+        }
+    }
+}
+
 fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
 
@@ -1595,6 +1770,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
             } else {
                 revset.iter_graph()
             };
+            let iter = TopoGroupedGraphIterator::new(iter);
             for (commit_id, edges) in iter {
                 let mut graphlog_edges = vec![];
                 // TODO: Should we update RevsetGraphIterator to yield this flag instead of all

tests/test_abandon_command.rs

@@ -84,11 +84,11 @@ fn test_rebase_branch_with_merge() {
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @
+    │ ◉  b
+    ├─╯
+    ◉  a e??
     │ ◉  d e??
     │ ◉  c
-    │ │ ◉  b
-    ├───╯
-    ◉ │  a e??
     ├─╯
     ◉
     "###);

tests/test_chmod_command.rs

@@ -149,10 +149,10 @@ fn test_chmod_file_dir_deletion_conflicts() {
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @    file_deletion
     ├─╮
+    ◉ │  deletion
     │ │ ◉  file_dir
     │ ╭─┤
     │ │ ◉  dir
-    ◉ │ │  deletion
     ├───╯
     │ ◉  file
     ├─╯

tests/test_duplicate_command.rs

@@ -160,12 +160,12 @@ fn test_duplicate_many() {
     ◉    0f7430f2727a   e
     ├─╮
     ◉ │  2181781b4f81   d
-    │ ◉  fa167d18a83a   b
     │ │ @    921dde6e55c0   e
     │ │ ├─╮
     │ │ ◉ │  ebd06dba20ec   d
     ├───╯ │
     ◉ │   │  c0cb3a0b73e7   c
+    │ ◉   │  fa167d18a83a   b
     ├─╯   │
     │     ◉  1394f625cbbd   b
     ├─────╯
@@ -195,16 +195,16 @@ fn test_duplicate_many() {
     ◉    9bd4389f5d47   e
     ├─╮
     ◉ │  d94e4c55a68b   d
-    │ │ ◉  c6f7f8c4512e   a
-    │ │ │ @  921dde6e55c0   e
-    │ ╭───┤
-    │ │ │ ◉  ebd06dba20ec   d
-    ├─────╯
-    ◉ │ │  c0cb3a0b73e7   c
-    │ ◉ │  1394f625cbbd   b
-    ├─╯ │
-    ◉   │  2443ea76b0b1   a
+    │ │ @  921dde6e55c0   e
+    │ ╭─┤
+    │ │ ◉  ebd06dba20ec   d
     ├───╯
+    ◉ │  c0cb3a0b73e7   c
+    │ ◉  1394f625cbbd   b
+    ├─╯
+    ◉  2443ea76b0b1   a
+    │ ◉  c6f7f8c4512e   a
+    ├─╯
     ◉  000000000000
     "###);
 

tests/test_git_colocated.rs

@@ -340,8 +340,8 @@ fn test_git_colocated_external_checkout() {
     // be abandoned. (#1042)
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  0521ce3b8c4e29aab79f3c750e2845dcbc4c3874
+    ◉  a86754f975f953fa25da4265764adc0c62e9ce6b master HEAD@git A
     │ ◉  66f4d1806ae41bd604f69155dece64062a0056cf B
-    ◉ │  a86754f975f953fa25da4265764adc0c62e9ce6b master HEAD@git A
     ├─╯
     ◉  0000000000000000000000000000000000000000
     "###);

tests/test_new_command.rs

@@ -139,8 +139,8 @@ fn test_new_insert_after() {
     ╭─┤
     @ │    G
     ├───╮
-    │ ◉ │  E
     ◉ │ │  D
+    │ ◉ │  E
     ├─╯ │
     │   ◉  B
     │   ◉  A
@@ -161,8 +161,8 @@ fn test_new_insert_after() {
     ◉ │    G
     ├───╮
     @ │ │  H
-    │ ◉ │  E
     ◉ │ │  D
+    │ ◉ │  E
     ├─╯ │
     │   ◉  B
     │   ◉  A
@@ -203,15 +203,15 @@ fn test_new_insert_after_children() {
     insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
     @    G
     ├─╮
-    │ │ ◉    F
-    │ │ ├─╮
-    │ │ ◉ │  E
-    │ │ │ ◉  D
-    │ │ ├─╯
-    ◉ │ │  C
-    ◉ │ │  B
+    ◉ │  C
+    ◉ │  B
+    ├─╯
+    ◉  A
+    │ ◉    F
+    │ ├─╮
+    │ ◉ │  E
     ├─╯ │
-    ◉   │  A
+    │   ◉  D
     ├───╯
     ◉  root
     "###);
@@ -290,9 +290,9 @@ fn test_new_insert_before_root_successors() {
     insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
     ◉    F
     ├─╮
+    ◉ │  D
     │ │ ◉  C
     │ │ ◉  B
-    ◉ │ │  D
     │ │ ◉  A
     ├───╯
     @ │  G
@@ -359,13 +359,13 @@ fn test_new_insert_before_no_root_merge() {
     insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
     ◉    F
     ├─╮
+    ◉ │  D
     │ │ ◉  C
-    ◉ │ │  D
     │ │ ◉  B
     ├───╯
     @ │  G
-    │ ◉  E
     ◉ │  A
+    │ ◉  E
     ├─╯
     ◉  root
     "###);