From 91504cae02175fc29587585f6d4e8bb4cb514417 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sun, 14 Jul 2024 09:43:36 -0500 Subject: [PATCH] obslog: reverse order of predecessors in topo traversal Currently, when there is a commit with two predecessors, the graph splits into two branches, and all of the predecessors on the first branch are printed before all of the predecessors on the second branch. This causes the graph to grow wider with each squashed commit, since the second branch must always get indented one level farther each time a commit is squashed. I have some commits where the graph is indented more than 10 levels due to squashing more than 10 times, making it very difficult to read. Reversing the order and printing the second branch before the first branch prevents this unnecessary indentation and makes the graph easier to read. This does not change the order of the edges in the graph (i.e. the first predecessor is still the first edge and the second predecessor is still the second edge in the graph). --- cli/src/commands/obslog.rs | 16 ++++++++- cli/tests/test_obslog_command.rs | 60 ++++++++++++++++---------------- cli/tests/test_squash_command.rs | 8 ++--- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cli/src/commands/obslog.rs b/cli/src/commands/obslog.rs index 7a34c735f..bfd5c71b7 100644 --- a/cli/src/commands/obslog.rs +++ b/cli/src/commands/obslog.rs @@ -113,7 +113,21 @@ pub(crate) fn cmd_obslog( let mut commits = topo_order_reverse_ok( vec![Ok(start_commit)], |commit: &Commit| commit.id().clone(), - |commit: &Commit| commit.predecessors().collect_vec(), + |commit: &Commit| { + let mut predecessors = commit.predecessors().collect_vec(); + // Predecessors don't need to follow any defined order. However in + // practice, if there are multiple predecessors, then usually the + // first predecessor is the previous version of the same change, and + // the other predecessors are commits that were squashed into it. If + // multiple commits are squashed at once, then they are usually + // recorded in chronological order. We want to show squashed commits + // in reverse chronological order, and we also want to show squashed + // commits before the squash destination (since the destination's + // subgraph may contain earlier squashed commits as well), so we + // visit the predecessors in reverse order. + predecessors.reverse(); + predecessors + }, )?; if args.deprecated_limit.is_some() { writeln!( diff --git a/cli/tests/test_obslog_command.rs b/cli/tests/test_obslog_command.rs index b2264d6ab..42a9ffc90 100644 --- a/cli/tests/test_obslog_command.rs +++ b/cli/tests/test_obslog_command.rs @@ -252,36 +252,36 @@ fn test_obslog_squash() { │ │ 1 1: foo │ │ 2 2: bar │ │ 3: baz - ◉ │ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:10 e3c2a446 - ├───╮ squashed 1 - │ │ │ Modified regular file file1: - │ │ │ 1 1: foo - │ │ │ 2: bar - ◉ │ │ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:09 766420db - │ │ │ first - │ │ │ Added regular file file1: - │ │ │ 1: foo - ◉ │ │ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:08 fa15625b - │ │ │ (empty) first - ◉ │ │ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:07 230dd059 - │ │ (empty) (no description set) - │ ◉ kkmpptxz hidden test.user@example.com 2001-02-03 08:05:10 46acd22a - │ │ second - │ │ Modified regular file file1: - │ │ 1 1: foo - │ │ 2: bar - │ ◉ kkmpptxz hidden test.user@example.com 2001-02-03 08:05:09 cba41deb - │ (empty) second - ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:12 7015a42c - │ third - │ Modified regular file file1: - │ 1 1: foo - │ 2 2: bar - │ 3: baz - ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:11 66645763 - │ (empty) third - ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:10 1c7afcb4 - (empty) (no description set) + │ ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:12 7015a42c + │ │ third + │ │ Modified regular file file1: + │ │ 1 1: foo + │ │ 2 2: bar + │ │ 3: baz + │ ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:11 66645763 + │ │ (empty) third + │ ◉ zsuskuln hidden test.user@example.com 2001-02-03 08:05:10 1c7afcb4 + │ (empty) (no description set) + ◉ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:10 e3c2a446 + ├─╮ squashed 1 + │ │ Modified regular file file1: + │ │ 1 1: foo + │ │ 2: bar + │ ◉ kkmpptxz hidden test.user@example.com 2001-02-03 08:05:10 46acd22a + │ │ second + │ │ Modified regular file file1: + │ │ 1 1: foo + │ │ 2: bar + │ ◉ kkmpptxz hidden test.user@example.com 2001-02-03 08:05:09 cba41deb + │ (empty) second + ◉ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:09 766420db + │ first + │ Added regular file file1: + │ 1: foo + ◉ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:08 fa15625b + │ (empty) first + ◉ qpvuntsm hidden test.user@example.com 2001-02-03 08:05:07 230dd059 + (empty) (no description set) "###); } diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index 57d2ab658..319c509de 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -972,10 +972,10 @@ fn test_squash_from_multiple_partial_no_op() { insta::assert_snapshot!(stdout, @r###" @ e178068add8c d ├─╮ - ◉ │ b37ca1ee3306 d - ◉ │ 1d9eb34614c9 d - ◉ b73077b08c59 b - ◉ a786561e909f b + │ ◉ b73077b08c59 b + │ ◉ a786561e909f b + ◉ b37ca1ee3306 d + ◉ 1d9eb34614c9 d "###); // If no source commits match the paths, then the whole operation is a no-op