ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

revset_graph: group commits topologically

Browse source 
The original idea was similar to Mercurial's "topo" sorting, but it was bad
at handling merge-heavy history. In order to render merges of topic branches
nicely, we need to prioritize branches at merge point, not at fork point.
OTOH, we do also want to place unmerged branches as close to the fork point
as possible. This commit implements the former requirement, and the latter
will be addressed by the next commit.

I think this is similar to Git's sorting logic described in the following blog
post. In our case, the in-degree walk can be dumb since topological order is
guaranteed by the index. We keep HashSet<CommitId> instead of an in-degree
integer value, which will be used in the next commit to resolve new heads as
late as possible.

https://github.blog/2022-08-30-gits-database-internals-ii-commit-history-queries/#topological-sorting

Compared to Sapling's beautify_graph(), this is lazy, and can roughly preserve
the index (or chronological) order. I tried beautify_graph() with prioritizing
the @ commit, but the result seemed too aggressively reordered. Perhaps, for
more complex history, beautify_graph() would produce a better result. For my
wip branches (~30 branches, a couple of commits per branch), this works pretty
well.

#242
This commit is contained in:
Yuya Nishihara 2023-07-16 19:47:46 +09:00
parent 6fcb98c0c4
commit fb33620f9e
14 changed files with 1328 additions and 109 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### New features
* `jj log` output is now topologically grouped.
[#242](https://github.com/martinvonz/jj/issues/242)
### Fixed bugs
## [0.8.0] - 2023-07-09

1216
lib/src/revset_graph.rs
View file

File diff suppressed because it is too large Load diff

12
src/commands/mod.rs
View file

@ -43,7 +43,9 @@ use jj_lib::repo_path::RepoPath;
use jj_lib::revset::{
RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt,
};
use jj_lib::revset_graph::{ReverseRevsetGraphIterator, RevsetGraphEdge, RevsetGraphEdgeType};
use jj_lib::revset_graph::{
ReverseRevsetGraphIterator, RevsetGraphEdgeType, TopoGroupedRevsetGraphIterator,
};
use jj_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser};
use jj_lib::settings::UserSettings;
use jj_lib::tree::{merge_trees, Tree};
@ -1587,11 +1589,11 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
if !args.no_graph {
let mut graph = get_graphlog(command.settings(), formatter.raw());
let default_node_symbol = graph.default_node_symbol().to_owned();
let iter: Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)>> = if args.reversed
{
Box::new(ReverseRevsetGraphIterator::new(revset.iter_graph()))
let forward_iter = TopoGroupedRevsetGraphIterator::new(revset.iter_graph());
let iter: Box<dyn Iterator<Item = _>> = if args.reversed {
Box::new(ReverseRevsetGraphIterator::new(Box::new(forward_iter)))
} else {
revset.iter_graph()
Box::new(forward_iter)
};
for (commit_id, edges) in iter {
let mut graphlog_edges = vec![];

2
tests/test_abandon_command.rs
View file

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

10
tests/test_chmod_command.rs
View file

@ -59,8 +59,8 @@ fn test_chmod_regular_conflict() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ x
│ ◉ n
◉ │ x
├─╯
◉ base
◉
@ -149,13 +149,13 @@ 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
├─╯ │
│ ◉ dir
├───╯
│ ◉ file
├─╯
◉ base
◉
"###);

50
tests/test_duplicate_command.rs
View file

@ -43,8 +43,8 @@ fn test_duplicate() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 17a00fc21654 c
├─╮
◉ │ d370aee184ba b
│ ◉ 2443ea76b0b1 a
◉ │ d370aee184ba b
├─╯
◉ 000000000000
"###);
@ -62,10 +62,10 @@ fn test_duplicate() {
◉ 2f6dc5a1ffc2 a
│ @ 17a00fc21654 c
│ ├─╮
│ ◉ │ d370aee184ba b
├─╯ │
│ ◉ 2443ea76b0b1 a
│ │ ◉ 2443ea76b0b1 a
├───╯
│ ◉ d370aee184ba b
├─╯
◉ 000000000000
"###);
@ -79,8 +79,8 @@ fn test_duplicate() {
├─╮
│ │ @ 17a00fc21654 c
╭─┬─╯
◉ │ d370aee184ba b
│ ◉ 2443ea76b0b1 a
◉ │ d370aee184ba b
├─╯
◉ 000000000000
"###);
@ -101,9 +101,9 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 921dde6e55c0 e
├─╮
│ ◉ 1394f625cbbd b
◉ │ ebd06dba20ec d
◉ │ c0cb3a0b73e7 c
│ ◉ 1394f625cbbd b
├─╯
◉ 2443ea76b0b1 a
◉ 000000000000
@ -120,11 +120,11 @@ fn test_duplicate_many() {
◉ │ 3b74d9691015 b
│ │ @ 921dde6e55c0 e
│ ╭─┤
│ ◉ │ ebd06dba20ec d
│ ◉ │ c0cb3a0b73e7 c
├─╯ │
│ ◉ 1394f625cbbd b
│ │ ◉ 1394f625cbbd b
├───╯
│ ◉ ebd06dba20ec d
│ ◉ c0cb3a0b73e7 c
├─╯
◉ 2443ea76b0b1 a
◉ 000000000000
"###);
@ -139,11 +139,11 @@ fn test_duplicate_many() {
◉ 0276d3d7c24d b
│ @ 921dde6e55c0 e
│ ├─╮
│ ◉ │ ebd06dba20ec d
│ ◉ │ c0cb3a0b73e7 c
├─╯ │
│ ◉ 1394f625cbbd b
│ │ ◉ 1394f625cbbd b
├───╯
│ ◉ ebd06dba20ec d
│ ◉ c0cb3a0b73e7 c
├─╯
◉ 2443ea76b0b1 a
◉ 000000000000
"###);
@ -159,16 +159,16 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ 0f7430f2727a e
├─╮
◉ │ 2181781b4f81 d
│ ◉ fa167d18a83a b
◉ │ 2181781b4f81 d
│ │ @ 921dde6e55c0 e
│ │ ├─╮
│ │ ◉ │ ebd06dba20ec d
├───╯ │
◉ │ │ c0cb3a0b73e7 c
├─╯ │
│ ◉ 1394f625cbbd b
├─────╯
│ │ │ ◉ 1394f625cbbd b
│ ├───╯
│ │ ◉ ebd06dba20ec d
├───╯
◉ │ c0cb3a0b73e7 c
├─╯
◉ 2443ea76b0b1 a
◉ 000000000000
"###);
@ -178,9 +178,9 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 921dde6e55c0 e
├─╮
│ ◉ 1394f625cbbd b
◉ │ ebd06dba20ec d
◉ │ c0cb3a0b73e7 c
│ ◉ 1394f625cbbd b
├─╯
◉ 2443ea76b0b1 a
◉ 000000000000
@ -198,10 +198,10 @@ fn test_duplicate_many() {
│ │ ◉ c6f7f8c4512e a
│ │ │ @ 921dde6e55c0 e
│ ╭───┤
│ ◉ │ │ 1394f625cbbd b
│ │ │ ◉ ebd06dba20ec d
├─────╯
◉ │ │ c0cb3a0b73e7 c
│ ◉ │ 1394f625cbbd b
├─╯ │
◉ │ 2443ea76b0b1 a
├───╯
@ -221,16 +221,16 @@ fn test_duplicate_many() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ ee8fe64ed254 e
├─╮
│ ◉ e13ac0adabdf b
◉ │ 2f2442db08eb d
◉ │ df53fa589286 c
│ ◉ e13ac0adabdf b
├─╯
◉ 0fe67a05989e a
│ @ 921dde6e55c0 e
│ ├─╮
│ │ ◉ 1394f625cbbd b
│ ◉ │ ebd06dba20ec d
│ ◉ │ c0cb3a0b73e7 c
│ │ ◉ 1394f625cbbd b
│ ├─╯
│ ◉ 2443ea76b0b1 a
├─╯

2
tests/test_git_colocated.rs
View file

@ -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
"###);

80
tests/test_new_command.rs
View file

@ -61,8 +61,8 @@ fn test_new_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 0c4e5b9b68ae0cbe7ce3c61042619513d09005bf
├─╮
◉ │ f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
│ ◉ 38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
◉ │ f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
├─╯
◉ 0000000000000000000000000000000000000000
"###);
@ -77,8 +77,8 @@ fn test_new_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 200ed1a14c8acf09783dafefe5bebf2ff58f12fd
├─╮
◉ │ f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
│ ◉ 38e8e2f6c92ffb954961fc391b515ff551b41636 add file1
◉ │ f399209d9dda06e8a25a0c8e9a0cde9f421ff35d add file2
├─╯
◉ 0000000000000000000000000000000000000000
"###);
@ -115,8 +115,8 @@ fn test_new_insert_after() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B
@ -137,14 +137,14 @@ fn test_new_insert_after() {
◉ C
│ ◉ F
╭─┤
│ ◉ E
@ │ G
├───╮
│ ◉ │ E
◉ │ │ D
├─╯ │
│ ◉ B
│ ◉ A
├───╯
│ │ ◉ B
│ │ ◉ A
│ ├─╯
◉ │ D
├─╯
◉ root
"###);
@ -158,15 +158,15 @@ fn test_new_insert_after() {
◉ C
│ ◉ F
╭─┤
│ ◉ E
◉ │ G
├───╮
@ │ │ H
│ ◉ │ E
◉ │ │ D
├─╯ │
│ ◉ B
│ ◉ A
├───╯
│ │ ◉ B
│ │ ◉ A
│ ├─╯
@ │ H
◉ │ D
├─╯
◉ root
"###);
}
@ -180,8 +180,8 @@ fn test_new_insert_after_children() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B
@ -203,16 +203,16 @@ fn test_new_insert_after_children() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ G
├─╮
│ │ ◉ F
│ │ ├─╮
│ │ ◉ │ E
│ │ │ ◉ D
│ │ ├─╯
◉ │ │ C
◉ │ │ B
├─╯ │
◉ │ A
◉ │ C
◉ │ B
├─╯
◉ A
│ ◉ F
│ ├─╮
│ │ ◉ D
├───╯
│ ◉ E
├─╯
◉ root
"###);
}
@ -226,8 +226,8 @@ fn test_new_insert_before() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B
@ -251,12 +251,12 @@ fn test_new_insert_before() {
├─╯
@ G
├─┬─╮
◉ │ │ E
│ │ ◉ B
│ │ ◉ A
│ ◉ │ D
├─╯ │
│ ◉ B
│ ◉ A
├───╯
│ ├─╯
◉ │ E
├─╯
◉ root
"###);
}
@ -270,8 +270,8 @@ fn test_new_insert_before_root_successors() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B
@ -290,13 +290,13 @@ fn test_new_insert_before_root_successors() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
◉ F
├─╮
│ ◉ E
◉ │ D
│ │ ◉ C
│ │ ◉ B
◉ │ │ D
│ │ ◉ A
├───╯
@ │ G
│ ◉ E
├─╯
◉ root
"###);
@ -313,8 +313,8 @@ fn test_new_insert_before_no_loop() {
insta::assert_snapshot!(stdout, @r###"
@ 7705d353bf5d F
├─╮
◉ │ 41a89ffcbba2 E
│ ◉ c9257eff5bf9 D
◉ │ 41a89ffcbba2 E
├─╯
│ ◉ ec18c57d72d8 C
│ ◉ 6041917ceeb5 B
@ -339,8 +339,8 @@ fn test_new_insert_before_no_root_merge() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B
@ -359,12 +359,12 @@ fn test_new_insert_before_no_root_merge() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
◉ F
├─╮
│ ◉ E
◉ │ D
│ │ ◉ C
◉ │ │ D
│ │ ◉ B
├───╯
@ │ G
│ ◉ E
◉ │ A
├─╯
◉ root
@ -380,8 +380,8 @@ fn test_new_insert_before_root() {
insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
@ F
├─╮
◉ │ E
│ ◉ D
◉ │ E
├─╯
│ ◉ C
│ ◉ B

28
tests/test_rebase_command.rs
View file

@ -248,8 +248,8 @@ fn test_rebase_single_revision() {
@ d
◉ c
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉
"###);
@ -290,10 +290,10 @@ fn test_rebase_single_revision() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ d
├─╮
│ │ ◉ c
◉ │ │ b
├───╯
│ ◉ a
◉ │ b
├─╯
│ ◉ c
├─╯
◉
"###);
@ -313,9 +313,9 @@ fn test_rebase_single_revision_merge_parent() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ d
├─╮
│ ◉ a
◉ │ c
◉ │ b
│ ◉ a
├─╯
◉
"###);
@ -333,9 +333,9 @@ fn test_rebase_single_revision_merge_parent() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ d
├─╮
◉ │ b
│ │ ◉ c
│ ├─╯
◉ │ b
│ ◉ a
├─╯
◉
@ -366,8 +366,8 @@ fn test_rebase_multiple_destinations() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ a
├─╮
@ │ c
│ ◉ b
@ │ c
├─╯
◉
"###);
@ -388,8 +388,8 @@ fn test_rebase_multiple_destinations() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ a
├─╮
@ │ c
│ ◉ b
@ │ c
├─╯
◉
"###);
@ -417,8 +417,8 @@ fn test_rebase_multiple_destinations() {
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ a
◉ b
│ @ c
◉ │ b
├─╯
◉
"###);
@ -439,8 +439,8 @@ fn test_rebase_multiple_destinations() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ a
├─╮
@ │ c
│ ◉ b
@ │ c
├─╯
◉
"###);
@ -467,8 +467,8 @@ fn test_rebase_with_descendants() {
@ d
◉ c
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉
"###);
@ -500,8 +500,8 @@ fn test_rebase_with_descendants() {
@ d
│ ◉ c
├─╯
◉ a
│ ◉ b
◉ │ a
├─╯
◉
"###);
@ -512,8 +512,8 @@ fn test_rebase_with_descendants() {
@ d
◉ c
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉
"###);
@ -529,8 +529,8 @@ fn test_rebase_with_descendants() {
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ c
◉ b
│ @ d
◉ │ b
├─╯
◉ a
◉

20
tests/test_resolve_command.rs
View file

@ -56,8 +56,8 @@ fn test_resolution() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉
@ -307,8 +307,8 @@ fn test_normal_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉
@ -348,8 +348,8 @@ fn test_baseless_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉
@ -416,8 +416,8 @@ fn test_edit_delete_conflict_input_files() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉
@ -459,8 +459,8 @@ fn test_file_vs_dir() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉
@ -506,11 +506,11 @@ fn test_description_with_dir_and_deletion() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─┬─╮
◉ │ │ del
│ │ ◉ edit
│ ◉ │ dir
├─╯ │
│ ◉ edit
├───╯
│ ├─╯
◉ │ del
├─╯
◉ base
◉
"###);
@ -586,8 +586,8 @@ fn test_multiple_conflicts() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉

2
tests/test_restore_command.rs
View file

@ -123,8 +123,8 @@ fn test_restore_conflicted_merge() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ conflict
├─╮
◉ │ b
│ ◉ a
◉ │ b
├─╯
◉ base
◉

4
tests/test_squash_command.rs
View file

@ -91,8 +91,8 @@ fn test_squash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c7a11b36d333 e
├─╮
◉ │ 5658521e0f8b d
│ ◉ 90fe0a96fc90 c
◉ │ 5658521e0f8b d
├─╯
◉ fa5efbdf533c b
◉ 90aeefd03044 a
@ -115,8 +115,8 @@ fn test_squash() {
@ 959145c11426
◉ 80960125bb96 e
├─╮
◉ │ 5658521e0f8b d
│ ◉ 90fe0a96fc90 c
◉ │ 5658521e0f8b d
├─╯
◉ fa5efbdf533c b
◉ 90aeefd03044 a

4
tests/test_templater.rs
View file

@ -68,10 +68,10 @@ fn test_templater_branches() {
insta::assert_snapshot!(output, @r###"
◉ b1bb3766d584 branch3??
│ @ a5b4d15489cc branch2* new-branch
│ │ ◉ 21c33875443e branch1*
├───╯
│ ◉ 8476341eb395 branch2@origin
├─╯
│ ◉ 21c33875443e branch1*
├─╯
◉ 000000000000
"###);
}

4
tests/test_unsquash_command.rs
View file

@ -90,8 +90,8 @@ fn test_unsquash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 1f8f152ff48e e
├─╮
◉ │ 5658521e0f8b d
│ ◉ 90fe0a96fc90 c
◉ │ 5658521e0f8b d
├─╯
◉ fa5efbdf533c b
◉ 90aeefd03044 a
@ -114,8 +114,8 @@ fn test_unsquash() {
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 3217340cb761
├─╮
◉ │ 5658521e0f8b d e??
│ ◉ 90fe0a96fc90 c e??
◉ │ 5658521e0f8b d e??
├─╯
◉ fa5efbdf533c b
◉ 90aeefd03044 a