From 2d5c4cede1c20fbe209d414ce52aa34da7bb8013 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Tue, 5 May 2026 17:07:52 -0400 Subject: [PATCH] git_graph: Keep shared parents on the leftmost incoming lane (#55818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes git graph lane selection when multiple active lanes point to the same parent commit, matching Git’s `--graph` behavior by keeping the commit on the leftmost incoming lane instead of the first discovered lane. This should make it much easier to track long lived branches ### Before image ### After Screenshot 2026-05-05 at 4 07 28 PM Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --------- Co-authored-by: Remco Smits --- crates/git_graph/src/git_graph.rs | 72 ++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/crates/git_graph/src/git_graph.rs b/crates/git_graph/src/git_graph.rs index 73ad9293e1..ac9a01deb9 100644 --- a/crates/git_graph/src/git_graph.rs +++ b/crates/git_graph/src/git_graph.rs @@ -640,7 +640,7 @@ impl GraphData { let commit_lane = self .parent_to_lanes .get(&commit.sha) - .and_then(|lanes| lanes.first().copied()); + .and_then(|lanes| lanes.iter().min().copied()); let commit_lane = commit_lane.unwrap_or_else(|| self.first_empty_lane_idx()); @@ -4049,6 +4049,74 @@ mod tests { Ok(()) } + fn verify_keep_shared_parents_on_leftmost_lane(graph: &GraphData) -> Result<()> { + let mut active_lane_parents: Vec> = Vec::new(); + let mut parent_to_lanes: HashMap> = HashMap::default(); + + for (row, entry) in graph.commits.iter().enumerate() { + let pending_lanes = parent_to_lanes.remove(&entry.data.sha).unwrap_or_default(); + + if pending_lanes.len() > 1 + && let Some(expected_lane) = pending_lanes.iter().copied().min() + && entry.lane != expected_lane + { + bail!( + "commit {:?} at row {} uses lane {}, but shared parent should use leftmost pending lane {} from {:?}", + entry.data.sha, + row, + entry.lane, + expected_lane, + pending_lanes + ); + } + + for lane in pending_lanes { + let Some(active_lane_parent) = active_lane_parents.get_mut(lane) else { + bail!( + "commit {:?} at row {} was pending on missing lane {}", + entry.data.sha, + row, + lane + ); + }; + + if *active_lane_parent != Some(entry.data.sha) { + bail!( + "commit {:?} at row {} was pending on lane {}, but that lane points to {:?}", + entry.data.sha, + row, + lane, + active_lane_parent + ); + } + + *active_lane_parent = None; + } + + for (parent_index, parent) in entry.data.parents.iter().enumerate() { + let lane = if parent_index == 0 { + entry.lane + } else if let Some(empty_lane) = + active_lane_parents.iter().position(Option::is_none) + { + empty_lane + } else { + active_lane_parents.push(None); + active_lane_parents.len() - 1 + }; + + if lane >= active_lane_parents.len() { + active_lane_parents.resize(lane + 1, None); + } + + active_lane_parents[lane] = Some(*parent); + parent_to_lanes.entry(*parent).or_default().push(lane); + } + } + + Ok(()) + } + fn verify_coverage(graph: &GraphData) -> Result<()> { let mut expected_edges: HashSet<(Oid, Oid)> = HashSet::default(); for entry in &graph.commits { @@ -4197,6 +4265,8 @@ mod tests { verify_column_correctness(graph, &oid_to_row).context("column correctness")?; verify_segment_continuity(graph).context("segment continuity")?; verify_merge_line_optimality(graph, &oid_to_row).context("merge line optimality")?; + verify_keep_shared_parents_on_leftmost_lane(graph) + .context("keep shared parents on leftmost lane")?; verify_coverage(graph).context("coverage")?; verify_line_overlaps(graph).context("line overlaps")?; Ok(())