fix(vcs): link filtered Git Log commits past the commits a filter hides (#1094)

With a User or text filter, the Git Log graph opened a lane for every commit whose parent the filter hid; the lanes never closed and filled the graph with arrows, since git keeps the real parents for these filters.

The log now links each shown commit to its nearest shown ancestors, dashed where the link passes hidden commits (the date range and No Merges count as hidden there too); a page's last commits are linked by walking on past the page. Collapse Linear Branches keeps those dashed links.

Fixes #872

crates/ide-vcs/src/history.rs

@@ -116,6 +116,11 @@ pub(crate) fn log(r: &Repo, q: &LogQuery) -> IdeResult<Vec<Commit>> {
     // Walks in (roughly) commit time order can stop once they are well past `since`.
     let time_ordered = range.is_none() && (q.order == LogOrder::Date || path_walk);
     let mut older_run = 0usize;
+    // A user or text filter hides commits within the history: the shown ones are linked to their
+    // nearest shown ancestors instead (dashed past hidden ones), as IDEA's filtered graph does.
+    // A path walk already links commits that way.
+    let mut links = (!path_walk && (!authors.is_empty() || text.is_some())).then(FilteredLinks::default);
+    let mut extra = 0usize;
 
     let mut out = Vec::new();
     for item in walk {
@@ -123,8 +128,17 @@ pub(crate) fn log(r: &Repo, q: &LogQuery) -> IdeResult<Vec<Commit>> {
         if q.first_parent {
             parents.truncate(1);
         }
+        if out.len() >= limit {
+            // Full: walk on only to link the last commits to their shown ancestors.
+            let Some(l) = links.as_ref() else { break };
+            extra += 1;
+            if l.is_empty() || extra > LINK_LIMIT {
+                break;
+            }
+        }
         let commit = repo.find_commit(id).map_err(gx)?;
         let commit_time = commit.time().map(|t| t.seconds).unwrap_or(0);
+        let mut shown = true;
         if shape.is_active() {
             let merge = commit.parent_ids().nth(1).is_some();
             if shape.since.is_some_and(|s| commit_time < s) {
@@ -132,33 +146,41 @@ pub(crate) fn log(r: &Repo, q: &LogQuery) -> IdeResult<Vec<Commit>> {
                 if time_ordered && older_run >= OLDER_RUN_LIMIT {
                     break;
                 }
-                continue;
+                shown = false;
+            } else {
+                older_run = 0;
+                if !shape.shows(merge, commit_time) {
+                    shown = false;
+                } else if links.is_none() {
+                    parents = shape.rewrite_parents(&repo, &parents)?;
+                }
             }
-            older_run = 0;
-            if !shape.shows(merge, commit_time) {
-                continue;
-            }
-            parents = shape.rewrite_parents(&repo, &parents)?;
         }
         let sig = commit.author().map_err(gx)?;
         let name = to_string(sig.name.trim().as_bstr());
         let email = to_string(sig.email.trim().as_bstr());
-        if !authors.is_empty() {
+        if shown && !authors.is_empty() {
             let (name, email) = (name.to_lowercase(), email.to_lowercase());
-            if !authors.iter().any(|a| name.contains(a.as_str()) || email.contains(a.as_str())) {
-                continue;
-            }
+            shown = authors.iter().any(|a| name.contains(a.as_str()) || email.contains(a.as_str()));
         }
         let message = to_string(commit.message_raw_sloppy());
-        if let Some(filter) = text.as_ref()
-            && !filter.matches(&message, &id)
-        {
+        if shown && let Some(filter) = text.as_ref() {
+            shown = filter.matches(&message, &id);
+        }
+        let listed = shown && to_skip == 0 && out.len() < limit;
+        if let Some(l) = links.as_mut() {
+            l.visit(id, &parents, shown, listed.then_some(out.len()));
+        }
+        if !shown {
             continue;
         }
         if to_skip > 0 {
             to_skip -= 1;
             continue;
         }
+        if !listed {
+            continue;
+        }
         let time_ms = sig.time().map(|t| t.seconds as f64 * 1000.0).unwrap_or(0.0);
         let summary = message.lines().find(|l| !l.trim().is_empty()).unwrap_or("").trim().to_string();
         let hex = id.to_string();
@@ -173,14 +195,80 @@ pub(crate) fn log(r: &Repo, q: &LogQuery) -> IdeResult<Vec<Commit>> {
             message: message.trim_end().to_string(),
             refs: decorations.get(&id).cloned().unwrap_or_default(),
             commit_time_ms: commit_time as f64 * 1000.0,
+            dashed: Vec::new(),
         });
-        if out.len() >= limit {
+        if out.len() >= limit && links.is_none() {
             break;
         }
     }
+    if let Some(l) = links {
+        l.apply(&mut out);
+    }
     Ok(out)
 }
 
+/// Commits walked past a full page to link its last commits to their shown ancestors.
+const LINK_LIMIT: usize = 20_000;
+
+/// The links of a filtered log: each listed commit's nearest shown ancestors. The walk visits
+/// children before parents, so a listed commit waits on its parents; a hidden commit passes
+/// the waits on to its own parents, and a shown one takes them.
+#[derive(Default)]
+struct FilteredLinks {
+    /// Commit → listed commits (by index) waiting on it, and whether the way passed a hidden one.
+    waiting: HashMap<ObjectId, Vec<(usize, bool)>>,
+    /// Per listed commit: its shown ancestors in the order found, and whether each link is dashed.
+    found: HashMap<usize, Vec<(ObjectId, bool)>>,
+}
+
+impl FilteredLinks {
+    fn is_empty(&self) -> bool {
+        self.waiting.is_empty()
+    }
+
+    fn visit(&mut self, id: ObjectId, parents: &[ObjectId], shown: bool, listed: Option<usize>) {
+        let waits = self.waiting.remove(&id).unwrap_or_default();
+        if shown {
+            for (child, dashed) in waits {
+                let links = self.found.entry(child).or_default();
+                match links.iter_mut().find(|(p, _)| *p == id) {
+                    Some(link) => link.1 &= dashed,
+                    None => links.push((id, dashed)),
+                }
+            }
+            if let Some(index) = listed {
+                for p in parents {
+                    Self::wait(&mut self.waiting, *p, index, false);
+                }
+            }
+        } else {
+            for p in parents {
+                for (child, _) in &waits {
+                    Self::wait(&mut self.waiting, *p, *child, true);
+                }
+            }
+        }
+    }
+
+    fn wait(waiting: &mut HashMap<ObjectId, Vec<(usize, bool)>>, on: ObjectId, child: usize, dashed: bool) {
+        let list = waiting.entry(on).or_default();
+        match list.iter_mut().find(|(c, _)| *c == child) {
+            Some(w) => w.1 &= dashed,
+            None => list.push((child, dashed)),
+        }
+    }
+
+    /// Replaces the listed commits' parents by their shown ancestors (none found: none).
+    fn apply(mut self, out: &mut [Commit]) {
+        for (index, commit) in out.iter_mut().enumerate() {
+            let links = self.found.remove(&index).unwrap_or_default();
+            commit.parents = links.iter().map(|(id, _)| id.to_string()).collect();
+            commit.dashed =
+                if links.iter().any(|(_, d)| *d) { links.iter().map(|(_, d)| *d).collect() } else { Vec::new() };
+        }
+    }
+}
+
 /// Consecutive commits older than `since` after which a time-ordered walk stops.
 const OLDER_RUN_LIMIT: usize = 64;
 

crates/ide-vcs/src/lib.rs

@@ -92,6 +92,11 @@ pub struct Commit {
     /// Committer time (Log › View Options › Commit Timestamp).
     #[serde(default)]
     pub commit_time_ms: f64,
+    /// Per parent, under a user or text filter: the link passes commits the filter hides
+    /// (`parents` then name the nearest shown ancestors). Empty when no parent is rewritten.
+    #[serde(default)]
+    #[specta(optional)]
+    pub dashed: Vec<bool>,
 }
 
 /// Order of the Git Log (Graph Options › Sort).

crates/ide-vcs/tests/log_shape.rs

@@ -178,3 +178,43 @@ fn remotes_and_worktrees_are_managed() {
     repo.remove_worktree(&wt, false).unwrap();
     assert_eq!(repo.worktrees().unwrap().len(), 1);
 }
+
+#[test]
+fn a_user_filter_links_shown_commits_past_hidden_ones() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"], None);
+    let by = |who: &str, file: &str, msg: &str, time: i64| {
+        std::fs::write(dir.join(file), msg).unwrap();
+        git(dir, &["add", "-A"], None);
+        let email = format!("user.email={who}@example.com");
+        git(dir, &["-c", &email, "commit", "-q", "-m", msg], Some(time));
+        git(dir, &["rev-parse", "HEAD"], None)
+    };
+    let m1 = by("me", "a", "m1", 1_000);
+    let o1 = by("other", "b", "o1", 2_000);
+    let m2 = by("me", "c", "m2", 3_000);
+    let m3 = by("me", "d", "m3", 4_000);
+    let _o2 = by("other", "e", "o2", 5_000);
+    let repo = Repo::discover(dir).unwrap().unwrap();
+
+    let log = repo.log(&LogQuery { author: Some("me@".into()), ..Default::default() }).unwrap();
+    let links: Vec<(&str, Vec<&str>, Vec<bool>)> = log
+        .iter()
+        .map(|c| (c.summary.as_str(), c.parents.iter().map(String::as_str).collect(), c.dashed.clone()))
+        .collect();
+    assert_eq!(
+        links,
+        [("m3", vec![m2.as_str()], vec![]), ("m2", vec![m1.as_str()], vec![true]), ("m1", vec![], vec![]),],
+        "m2's parent o1 is hidden: m2 links to m1, dashed"
+    );
+    assert!(!log.iter().any(|c| c.parents.contains(&o1)));
+
+    // A page ends at m2: walking on still links it to m1.
+    let page = repo.log(&LogQuery { author: Some("me@".into()), limit: 2, ..Default::default() }).unwrap();
+    assert_eq!(page.len(), 2);
+    assert_eq!(page[1].parents, std::slice::from_ref(&m1));
+    let next = repo.log(&LogQuery { author: Some("me@".into()), skip: 2, limit: 2, ..Default::default() }).unwrap();
+    assert_eq!(next.iter().map(|c| c.id.clone()).collect::<Vec<_>>(), std::slice::from_ref(&m1));
+    let _ = m3;
+}

src/bindings.ts

@@ -2036,6 +2036,11 @@ export type Commit = {
 	refs: string[],
 	/**  Committer time (Log › View Options › Commit Timestamp). */
 	commitTimeMs?: number | null,
+	/**
+	 *  Per parent, under a user or text filter: the link passes commits the filter hides
+	 *  (`parents` then name the nearest shown ancestors). Empty when no parent is rewritten.
+	 */
+	dashed?: boolean[],
 };
 
 /**  Commit Options (IDEA's commit options popup). */

src/features/vcs/__tests__/logOptions.test.ts

@@ -44,6 +44,18 @@ describe("Collapse Linear Branches", () => {
     expect(rows[2]!.up.some((s) => s.dashed)).toBe(true);
   });
 
+  it("keeps the dashed links of a filtered log", () => {
+    // A user filter already linked y past hidden commits.
+    const commits = [
+      c("z", ["y"], ["main"]),
+      { ...c("y", ["x"], ["tag: v2"]), dashed: [true] },
+      c("x", [], ["tag: v1"]),
+    ];
+    const { graph } = collapseLinear(commits, new Set());
+    expect(graph.find((g) => g.id === "y")).toEqual({ id: "y", parents: ["x"], dashed: [true] });
+    expect(computeGraph(commits).rows[1]!.down.some((s) => s.dashed)).toBe(true);
+  });
+
   it("keeps referenced and kept commits", () => {
     const commits = [c("d", ["c"]), c("c", ["b"], ["tag: v1"]), c("b", ["a"]), c("a")];
     expect(collapseLinear(commits, new Set()).commits.map((x) => x.id)).toEqual(["d", "c", "a"]);

src/features/vcs/graph.ts

@@ -133,10 +133,9 @@ export function computeGraph(
  * no references) so that only branch points, merges, tips and referenced commits stay,
  * linked by dashed edges. Commits in `keep` (HEAD) stay visible.
  */
-export function collapseLinear<C extends { id: string; parents: readonly string[]; refs: readonly string[] }>(
-  commits: readonly C[],
-  keep: ReadonlySet<string>,
-): { commits: C[]; graph: GraphCommit[] } {
+export function collapseLinear<
+  C extends { id: string; parents: readonly string[]; refs: readonly string[]; dashed?: readonly boolean[] },
+>(commits: readonly C[], keep: ReadonlySet<string>): { commits: C[]; graph: GraphCommit[] } {
   const byId = new Map(commits.map((c) => [c.id, c]));
   const children = new Map<string, number>();
   for (const c of commits) for (const p of c.parents) children.set(p, (children.get(p) ?? 0) + 1);
@@ -149,9 +148,10 @@ export function collapseLinear<C extends { id: string; parents: readonly string[
     if (hidden(c)) continue;
     const dashed: boolean[] = [];
     const parents: string[] = [];
-    for (const p of c.parents) {
+    for (const [i, p] of c.parents.entries()) {
       let cur = p;
-      let skipped = false;
+      // A filter may already link past hidden commits.
+      let skipped = c.dashed?.[i] ?? false;
       for (let next = byId.get(cur), guard = 0; next && guard < commits.length && hidden(next); guard++) {
         cur = next.parents[0]!;
         next = byId.get(cur);