Skip to content

Commit

Permalink
Fetch descendants more correctly.
Browse files Browse the repository at this point in the history 
See context in [this discussion](#3935 (comment))

Fixes #3947
  • Loading branch information
@essiene
essiene committed Sep 15, 2024
1 parent 56dbbb8 commit 6121166
Show file tree
Hide file tree
Showing 2 changed files with 317 additions and 9 deletions.
26 changes: 17 additions & 9 deletions cli/src/movement_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,25 @@ impl Direction {
start_revset: &Rc<RevsetExpression>,
args: &MovementArgsInternal,
) -> Result<Rc<RevsetExpression>, CommandError> {
let target_revset = match (self, args.conflict) {
(Direction::Next, true) => start_revset
let target_revset = match (self, args.should_edit, args.conflict) {
(Direction::Next, true, false) => start_revset.descendants_at(args.offset),
(Direction::Next, true, true) => start_revset
.descendants_at(args.offset)
.descendants()
.filtered(RevsetFilterPredicate::HasConflict)
.roots(),
(Direction::Next, false, false) => start_revset
.children()
.minus(working_revset)
.descendants_at(args.offset - 1),
(Direction::Next, false, true) => start_revset
.children()
.minus(working_revset)
.descendants_at(args.offset - 1)
.descendants()
.filtered(RevsetFilterPredicate::HasConflict)
.roots()
.minus(working_revset),
(Direction::Next, false) => start_revset
.descendants_at(args.offset)
.minus(working_revset),
(Direction::Prev, true) =>
.roots(),
(Direction::Prev, _, true) =>
// If people desire to move to the root conflict, replace the `heads()` below
// with `roots(). But let's wait for feedback.
{
Expand All @@ -141,7 +149,7 @@ impl Direction {
.filtered(RevsetFilterPredicate::HasConflict)
.heads()
}
(Direction::Prev, false) => start_revset.ancestors_at(args.offset),
(Direction::Prev, _, false) => start_revset.ancestors_at(args.offset),
};

Ok(target_revset)
Expand Down
300 changes: 300 additions & 0 deletions cli/tests/test_next_prev_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,306 @@ fn test_movement_edit_mode_false() {
"###);
}

#[test]
fn test_movement_target_revset() {
let test_env = TestEnvironment::default();

test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let conflict_path = repo_path.join("conflict.txt");
let good_path = repo_path.join("good.txt");

test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);

std::fs::write(&conflict_path, "second").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);

std::fs::write(&conflict_path, "third").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);

std::fs::write(&conflict_path, "fourth").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fourth"]);

std::fs::write(&conflict_path, "fifth").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fifth"]);

std::fs::write(&conflict_path, "sixth").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "sixth"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "seventh"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "eight"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "ninth"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "tenth"]);

// Create a conflict
test_env.jj_cmd_ok(&repo_path, &["edit", "description(fourth)"]);
std::fs::write(&conflict_path, "modified fourth").unwrap();

// Fix the conflict
test_env.jj_cmd_ok(&repo_path, &["edit", "description(seventh)"]);
std::fs::remove_file(&conflict_path).unwrap();
std::fs::write(&good_path, "good").unwrap();

test_env.jj_cmd_ok(&repo_path, &["edit", "description(tenth)"]);

// Confirm the final state of the tree.
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.add_config(r#"ui.movement.edit = false"#);

test_env.jj_cmd_ok(&repo_path, &["new", "description(second)"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ nkmrtpmomlro
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
│ × royxmykxtrkr conflict sixth
│ × mzvwutvlkqwt conflict fifth
│ ○ zsuskulnrvyr fourth
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ xznxytknoqwo
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
│ × royxmykxtrkr conflict sixth
│ × mzvwutvlkqwt conflict fifth
│ ○ zsuskulnrvyr fourth
├─╯
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ nmzmmopxokps
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
│ × royxmykxtrkr conflict sixth
│ × mzvwutvlkqwt conflict fifth
│ ○ zsuskulnrvyr fourth
│ ○ kkmpptxzrspx third
├─╯
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ wvuyspvkupzz conflict
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
│ × royxmykxtrkr conflict sixth
├─╯
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ pzsxstztnpkv conflict
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
├─╯
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev", "3"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ uuuvxpvwspwr
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
│ ○ yqosqzytrlsw seventh
│ × royxmykxtrkr conflict sixth
│ × mzvwutvlkqwt conflict fifth
│ ○ zsuskulnrvyr fourth
├─╯
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "4"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ oupztwtkortx
│ ○ znkkpsqqskkl tenth
│ ○ yostqsxwqrlt ninth
│ ○ vruxwmqvtpmx eight
├─╯
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.add_config(r#"ui.movement.edit = true"#);

test_env.jj_cmd_ok(&repo_path, &["edit", "description(second)"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
@ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
@ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
@ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
@ mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
@ royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["prev", "3"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
@ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
× mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
@ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);

test_env.jj_cmd_ok(&repo_path, &["next", "4"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
○ znkkpsqqskkl tenth
○ yostqsxwqrlt ninth
○ vruxwmqvtpmx eight
○ yqosqzytrlsw seventh
× royxmykxtrkr conflict sixth
@ mzvwutvlkqwt conflict fifth
○ zsuskulnrvyr fourth
○ kkmpptxzrspx third
○ rlvkpnrzqnoo second
○ qpvuntsmwlqt first
◆ zzzzzzzzzzzz
"###);
}

fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
let template = r#"separate(" ", change_id.short(), local_bookmarks, if(conflict, "conflict"), description)"#;
test_env.jj_cmd_success(cwd, &["log", "-T", template])
Expand Down

0 comments on commit 6121166

Please sign in to comment.