From b21081f92f90e84a22365aaee7a78ec002b6041b Mon Sep 17 00:00:00 2001 From: Philip Metzger Date: Sun, 18 Feb 2024 18:57:55 +0100 Subject: [PATCH] next/prev: Implement `next/prev --conflict` This allows users to jump to the next conflict in the ancestors or children of the start commit. Continues work on #2126 Co-Authored-By: Noah Mayr --- CHANGELOG.md | 3 + cli/src/commands/next.rs | 27 ++++-- cli/src/commands/prev.rs | 19 +++-- cli/tests/cli-reference@.md.snap | 8 ++ cli/tests/test_next_prev_commands.rs | 120 +++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 920ca1ede67..9e0ef4e2715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 key](https://toml.io/en/v1.0.0#keys). Quote meta characters as needed. Example: `jj config get "revset-aliases.'trunk()'"` +* `jj prev` and `jj next` have gained a `--conflict` flag which moves you + to the next conflict in a child commit. + ### Deprecations - Attempting to alias a built-in command now gives a warning, rather than being silently ignored. diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs index 8ae074fdb76..31fc22fc5b1 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -17,7 +17,7 @@ use std::io::Write; use itertools::Itertools; use jj_lib::commit::Commit; use jj_lib::repo::Repo; -use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; +use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; use crate::cli_util::{short_commit_hash, CommandHelper, WorkspaceCommandHelper}; use crate::command_error::{user_error, CommandError}; @@ -65,6 +65,9 @@ pub(crate) struct NextArgs { /// edit`). #[arg(long, short)] edit: bool, + /// Jump to the next conflicted descendant. + #[arg(long, conflicts_with = "offset")] + conflict: bool, } pub fn choose_commit<'a>( @@ -117,21 +120,27 @@ pub(crate) fn cmd_next( let wc_revset = RevsetExpression::commit(current_wc_id.clone()); // If we're editing, start at the working-copy commit. Otherwise, start from // its direct parent(s). - let target_revset = if edit { - wc_revset.descendants_at(args.offset) + let start_revset = if edit { + wc_revset.clone() } else { - wc_revset - .parents() - .descendants_at(args.offset) - // In previous versions we subtracted `wc_revset.descendants()`. That's - // unnecessary now that --edit is implied if `@` has descendants. - .minus(&wc_revset) + wc_revset.parents() }; + + let target_revset = if args.conflict { + start_revset + .descendants() + .filtered(RevsetFilterPredicate::HasConflict) + .roots() + } else { + start_revset.descendants_at(args.offset).minus(&wc_revset) + }; + let targets: Vec = target_revset .evaluate_programmatic(workspace_command.repo().as_ref())? .iter() .commits(workspace_command.repo().store()) .try_collect()?; + let target = match targets.as_slice() { [target] => target, [] => { diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index acd0c100edc..fead40f286c 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -14,7 +14,7 @@ use itertools::Itertools; use jj_lib::repo::Repo; -use jj_lib::revset::{RevsetExpression, RevsetIteratorExt}; +use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; use crate::cli_util::{short_commit_hash, CommandHelper}; use crate::command_error::{user_error, CommandError}; @@ -59,6 +59,9 @@ pub(crate) struct PrevArgs { /// Edit the parent directly, instead of moving the working-copy commit. #[arg(long, short)] edit: bool, + /// Jump to the previous conflicted ancestor. + #[arg(long, conflicts_with = "offset")] + conflict: bool, } pub(crate) fn cmd_prev( @@ -79,11 +82,17 @@ pub(crate) fn cmd_prev( // If we're editing, start at the working-copy commit. Otherwise, start from // its direct parent(s). let target_revset = if edit { - RevsetExpression::commit(current_wc_id.clone()).ancestors_at(args.offset) - } else { RevsetExpression::commit(current_wc_id.clone()) - .parents() - .ancestors_at(args.offset) + } else { + RevsetExpression::commit(current_wc_id.clone()).parents() + }; + let target_revset = if args.conflict { + target_revset + .ancestors() + .filtered(RevsetFilterPredicate::HasConflict) + .roots() + } else { + target_revset.ancestors_at(args.offset) }; let targets: Vec<_> = target_revset .evaluate_programmatic(workspace_command.repo().as_ref())? diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index c74cbbfa50c..16c30c8c6f3 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1200,6 +1200,10 @@ implied. Possible values: `true`, `false` +* `--conflict` — Jump to the next conflicted descendant + + Possible values: `true`, `false` + @@ -1444,6 +1448,10 @@ implied. Possible values: `true`, `false` +* `--conflict` — Jump to the previous conflicted ancestor + + Possible values: `true`, `false` + diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs index 36d2582fced..a77d04cd49d 100644 --- a/cli/tests/test_next_prev_commands.rs +++ b/cli/tests/test_next_prev_commands.rs @@ -567,6 +567,126 @@ fn test_next_editing() { "###); } +#[test] +fn test_prev_conflict() { + // Make the first commit our new parent. + 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 file_path = repo_path.join("content.txt"); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + std::fs::write(&file_path, "second").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + // Create a conflict in the first commit, where we'll jump to. + test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]); + std::fs::write(&file_path, "first").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "description(third)"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fourth"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--conflict"]); + // We now should be a child of `third`. + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: vruxwmqv 5050288d (conflict) (empty) (no description set) + Parent commit : rlvkpnrz 7c39847a (conflict) second + There are unresolved conflicts at these paths: + content.txt 2-sided conflict + "###); +} + +#[test] +fn test_prev_conflict_editing() { + // Edit the third commit. + 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 file_path = repo_path.join("content.txt"); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + std::fs::write(&file_path, "second").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + // Create a conflict in the third commit, where we'll jump to. + test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]); + std::fs::write(&file_path, "first text").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "description(third)"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "fourth"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--conflict", "--edit"]); + // We now should be editing the third commit. + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kkmpptxz 26b1439f (conflict) third + Parent commit : rlvkpnrz 55b5d11a (empty) second + There are unresolved conflicts at these paths: + content.txt 2-sided conflict + "###); +} + +#[test] +fn test_next_conflict() { + // There is a conflict in the third commit, so after next it should be the new + // parent. + 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 file_path = repo_path.join("content.txt"); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + std::fs::write(&file_path, "second").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + // Create a conflict in the third commit. + test_env.jj_cmd_ok(&repo_path, &["edit", "description(first)"]); + std::fs::write(&file_path, "first").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "description(second)"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]); + test_env.jj_cmd_ok(&repo_path, &["new", "description(first)"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: vruxwmqv b69eca51 (conflict) (empty) (no description set) + Parent commit : rlvkpnrz fa43d820 (conflict) second + Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + content.txt 2-sided conflict + "###); + // --edit is implied when already editing a non-head commit + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: yostqsxw e4ce14ca (conflict) (empty) (no description set) + Parent commit : mzvwutvl d37ca916 (conflict) (empty) third + There are unresolved conflicts at these paths: + content.txt 2-sided conflict + "###); +} + +#[test] +fn test_next_conflict_editing() { + // There is a conflict in the third commit, so after next it should be our + // working copy. + 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 file_path = repo_path.join("content.txt"); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]); + std::fs::write(&file_path, "second").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]); + // Create a conflict in the third commit. + std::fs::write(&file_path, "first+1").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["edit", "description(second)"]); + std::fs::write(&file_path, "third").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "@+"]); + test_env.jj_cmd_ok(&repo_path, &["edit", "description(second)"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--conflict", "--edit"]); + // We now should be editing the third commit. + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Working copy now at: kkmpptxz 73ba5559 (conflict) (no description set) + Parent commit : rlvkpnrz 9779f9df second + Added 0 files, modified 1 files, removed 0 files + There are unresolved conflicts at these paths: + content.txt 2-sided conflict + "###); +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"separate(" ", change_id.short(), local_branches, description)"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])