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 16, 2024
1 parent 56dbbb8 commit 4231b87
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 16 deletions.
32 changes: 16 additions & 16 deletions cli/src/movement_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,27 +121,27 @@ 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 nth = match (self, args.should_edit) {
(Direction::Next, true) => start_revset.descendants_at(args.offset),
(Direction::Next, false) => start_revset
.children()
.minus(working_revset)
.descendants_at(args.offset - 1),
(Direction::Prev, _) => start_revset.ancestors_at(args.offset),
};

let target_revset = match (self, args.conflict) {
(_, false) => nth,
(Direction::Next, true) => nth
.descendants()
.filtered(RevsetFilterPredicate::HasConflict)
.roots()
.minus(working_revset),
(Direction::Next, false) => start_revset
.descendants_at(args.offset)
.minus(working_revset),
(Direction::Prev, true) =>
.roots(),
// If people desire to move to the root conflict, replace the `heads()` below
// with `roots(). But let's wait for feedback.
{
start_revset
.parents()
.ancestors()
.filtered(RevsetFilterPredicate::HasConflict)
.heads()
}
(Direction::Prev, false) => start_revset.ancestors_at(args.offset),
(Direction::Prev, true) => nth
.ancestors()
.filtered(RevsetFilterPredicate::HasConflict)
.heads(),
};

Ok(target_revset)
Expand Down
149 changes: 149 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,155 @@ 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
"###);

// next, edit=false, conflict=false, offset is implied (1)
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
"###);

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

// next, edit=false, conflict=true, offset = ignored, wc conflict
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
"###);

// next, edit=false, conflict=false, offset is explicit
test_env.jj_cmd_ok(&repo_path, &["new", "description(second)"]);
test_env.jj_cmd_ok(&repo_path, &["next", "6"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ xlzxqlslvuyv
│ ○ 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 4231b87

Please sign in to comment.