diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs index 30aa9cc7c1..c29fc6edce 100644 --- a/cli/src/movement_util.rs +++ b/cli/src/movement_util.rs @@ -16,12 +16,14 @@ use std::io::Write; use std::rc::Rc; use itertools::Itertools; -use jj_lib::backend::CommitId; +use jj_lib::backend::{ChangeId, CommitId}; use jj_lib::commit::Commit; use jj_lib::repo::Repo; use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; -use crate::cli_util::{short_commit_hash, CommandHelper, WorkspaceCommandHelper}; +use crate::cli_util::{ + short_change_hash, short_commit_hash, CommandHelper, WorkspaceCommandHelper, +}; use crate::command_error::{user_error, CommandError}; use crate::ui::Ui; @@ -54,27 +56,47 @@ impl Direction { } } - fn target_not_found_message(&self, change_offset: u64) -> String { + fn target_not_found_message( + &self, + edit: bool, + change_offset: u64, + change_ids: &[ChangeId], + ) -> String { + let from = change_ids.iter().map(short_change_hash).join(", "); match self { - Direction::Next => format!("No descendant found {} commit(s) forward", change_offset), - Direction::Prev => format!("No ancestor found {} commit(s) back", change_offset), + Direction::Next => { + if edit { + // in edit mode, start_revset is the WC, so + // we only look for direct descendants. + format!( + "No descendant found {} commit(s) forward from {}", + change_offset, from + ) + } else { + // in non-edit mode, start_revset is the parent + // of WC, so we look for other descendants of + // start_revset. + format!( + "No other descendant found {} commit(s) forward from {}", + change_offset, from + ) + } + } + // The WC can never be an ancestor of the start_revset since + // start_revset is either itself or it's parent. + Direction::Prev => format!( + "No ancestor found {} commit(s) back from {}", + change_offset, from + ), } } fn get_target_revset( &self, - working_commit_id: &CommitId, + working_revset: &Rc, + start_revset: &Rc, args: &MovementArgsInternal, ) -> Result, CommandError> { - let wc_revset = RevsetExpression::commit(working_commit_id.clone()); - // If we're editing, start at the working-copy commit. Otherwise, start from - // its direct parent(s). - let start_revset = if args.should_edit { - wc_revset.clone() - } else { - wc_revset.parents() - }; - let target_revset = match self { Direction::Next => if args.conflict { start_revset @@ -85,7 +107,7 @@ impl Direction { } else { start_revset.descendants_at(args.offset) } - .minus(&wc_revset), + .minus(working_revset), Direction::Prev => { if args.conflict { @@ -113,7 +135,25 @@ fn get_target_commit( working_commit_id: &CommitId, args: &MovementArgsInternal, ) -> Result { - let target_revset = direction.get_target_revset(working_commit_id, args)?; + let wc_revset = RevsetExpression::commit(working_commit_id.clone()); + // If we're editing, start at the working-copy commit. Otherwise, start from + // its direct parent(s). + let start_revset = if args.should_edit { + wc_revset.clone() + } else { + wc_revset.parents() + }; + + let start_commits: Vec = start_revset + .clone() + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .map_ok(|c| c.change_id().clone()) + .try_collect()?; + + let target_revset = direction.get_target_revset(&wc_revset, &start_revset, args)?; + let targets: Vec = target_revset .evaluate_programmatic(workspace_command.repo().as_ref())? .iter() @@ -124,7 +164,11 @@ fn get_target_commit( [target] => target, [] => { // We found no ancestor/descendant. - return Err(user_error(direction.target_not_found_message(args.offset))); + return Err(user_error(direction.target_not_found_message( + args.should_edit, + args.offset, + &start_commits, + ))); } commits => choose_commit(ui, workspace_command, direction, commits)?, }; diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs index 1dba660613..7f0c5ead01 100644 --- a/cli/tests/test_next_prev_commands.rs +++ b/cli/tests/test_next_prev_commands.rs @@ -201,7 +201,7 @@ fn test_next_exceeding_history() { let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "3"]); // `jj next` beyond existing history fails. insta::assert_snapshot!(stderr, @r###" - Error: No descendant found 3 commit(s) forward + Error: No other descendant found 3 commit(s) forward from qpvuntsmwlqt "###); } @@ -542,10 +542,12 @@ fn test_prev_prompts_on_multiple_parents() { // Create a merge commit, which has two parents. test_env.jj_cmd_ok(&repo_path, &["new", "all:@--+"]); test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "merge"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "merge+1"]); // Check that the graph looks the way we expect. insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ vruxwmqvtpmx + @ yostqsxwqrlt + ○ vruxwmqvtpmx merge+1 ○ yqosqzytrlsw merge ├─┬─╮ │ │ ○ qpvuntsmwlqt first @@ -557,7 +559,7 @@ fn test_prev_prompts_on_multiple_parents() { "###); // Move @ backwards. - let (stdout, stderr) = test_env.jj_cmd_stdin_ok(&repo_path, &["prev"], "3\n"); + let (stdout, stderr) = test_env.jj_cmd_stdin_ok(&repo_path, &["prev", "2"], "3\n"); insta::assert_snapshot!(stdout,@r###" ambiguous prev commit, choose one to target: 1: mzvwutvl bc4f4fe3 (empty) third @@ -567,9 +569,42 @@ fn test_prev_prompts_on_multiple_parents() { enter the index of the commit you want to target: "###); insta::assert_snapshot!(stderr,@r###" - Working copy now at: znkkpsqq 07b409e8 (empty) (no description set) + Working copy now at: kpqxywon ddac00b0 (empty) (no description set) Parent commit : qpvuntsm fa15625b (empty) first "###); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ kpqxywonksrl + │ ○ vruxwmqvtpmx merge+1 + │ ○ yqosqzytrlsw merge + ╭─┼─╮ + ○ │ │ qpvuntsmwlqt first + │ │ ○ kkmpptxzrspx second + ├───╯ + │ ○ mzvwutvlkqwt third + ├─╯ + ◆ zzzzzzzzzzzz + "###); + + _ = test_env.jj_cmd_ok(&repo_path, &["next"]); + _ = test_env.jj_cmd_ok(&repo_path, &["edit", "@-"]); + + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ○ vruxwmqvtpmx merge+1 + @ yqosqzytrlsw merge + ├─┬─╮ + │ │ ○ qpvuntsmwlqt first + │ ○ │ kkmpptxzrspx second + │ ├─╯ + ○ │ mzvwutvlkqwt third + ├─╯ + ◆ zzzzzzzzzzzz + "###); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--no-edit"]); + insta::assert_snapshot!(stderr,@r###" + Error: No other descendant found 1 commit(s) forward from mzvwutvlkqwt, kkmpptxzrspx, qpvuntsmwlqt + "###) } #[test] @@ -592,7 +627,7 @@ fn test_prev_beyond_root_fails() { // @- is at "fourth", and there is no parent 5 commits behind it. let stderr = test_env.jj_cmd_failure(&repo_path, &["prev", "5"]); insta::assert_snapshot!(stderr,@r###" - Error: No ancestor found 5 commit(s) back + Error: No ancestor found 5 commit(s) back from zsuskulnrvyr "###); } @@ -854,14 +889,15 @@ fn test_next_conflict_head() { @ rlvkpnrzqnoo conflict ◆ zzzzzzzzzzzz "###); + let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--conflict"]); insta::assert_snapshot!(stderr, @r###" - Error: No descendant found 1 commit(s) forward + Error: No other descendant found 1 commit(s) forward from zzzzzzzzzzzz "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--conflict", "--edit"]); insta::assert_snapshot!(stderr, @r###" - Error: No descendant found 1 commit(s) forward + Error: No descendant found 1 commit(s) forward from rlvkpnrzqnoo "###); } @@ -921,6 +957,11 @@ fn test_movement_edit_mode_true() { ◆ zzzzzzzzzzzz "###); + let stderr = test_env.jj_cmd_failure(&repo_path, &["prev"]); + insta::assert_snapshot!(stderr, @r###" + Error: The root commit 000000000000 is immutable + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" @@ -952,12 +993,12 @@ fn test_movement_edit_mode_true() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--no-edit"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: nkmrtpmo 67a001a6 (empty) (no description set) + Working copy now at: uyznsvlq 7ad57fb8 (empty) (no description set) Parent commit : qpvuntsm fa15625b (empty) first "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ nkmrtpmomlro + @ uyznsvlquzzm │ ○ kkmpptxzrspx third │ ○ rlvkpnrzqnoo second ├─╯ @@ -968,18 +1009,23 @@ fn test_movement_edit_mode_true() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--no-edit"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: xznxytkn 22287813 (empty) (no description set) + Working copy now at: xtnwkqum 7ac7a1c4 (empty) (no description set) Parent commit : rlvkpnrz 9ed53a4a (empty) second "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ xznxytknoqwo + @ xtnwkqumpolk │ ○ kkmpptxzrspx third ├─╯ ○ rlvkpnrzqnoo second ○ qpvuntsmwlqt first ◆ zzzzzzzzzzzz "###); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["next"]); + insta::assert_snapshot!(stderr, @r###" + Error: No descendant found 1 commit(s) forward from xtnwkqumpolk + "###); } #[test] @@ -1028,15 +1074,20 @@ fn test_movement_edit_mode_false() { ◆ zzzzzzzzzzzz "###); + let stderr = test_env.jj_cmd_failure(&repo_path, &["prev", "3"]); + insta::assert_snapshot!(stderr, @r###" + Error: No ancestor found 3 commit(s) back from qpvuntsmwlqt + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: znkkpsqq a8419fd6 (empty) (no description set) + Working copy now at: kpqxywon d06750fb (empty) (no description set) Parent commit : rlvkpnrz 9ed53a4a (empty) second "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ znkkpsqqskkl + @ kpqxywonksrl │ ○ kkmpptxzrspx third ├─╯ ○ rlvkpnrzqnoo second @@ -1047,18 +1098,10 @@ fn test_movement_edit_mode_false() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next", "--no-edit"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: kmkuslsw 8c4d85ef (empty) (no description set) + Working copy now at: wqnwkozp 10fa181f (empty) (no description set) Parent commit : kkmpptxz 30056b0c (empty) third "###); - insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ kmkuslswpqwq - ○ kkmpptxzrspx third - ○ rlvkpnrzqnoo second - ○ qpvuntsmwlqt first - ◆ zzzzzzzzzzzz - "###); - let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["prev", "--edit", "2"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###"