diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs index 6cf6d8939b..bfb7c0a015 100644 --- a/cli/src/movement_util.rs +++ b/cli/src/movement_util.rs @@ -17,12 +17,14 @@ use std::rc::Rc; use config::Config; use itertools::Itertools; -use jj_lib::backend::CommitId; +use jj_lib::backend::{BackendError, 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::{config_error_with_message, user_error, CommandError}; use crate::ui::Ui; @@ -55,27 +57,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_id: &ChangeId, + ) -> String { + let change_hash = short_change_hash(change_id); 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, change_hash + ) + } 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, change_hash + ) + } + } + // 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, change_hash + ), } } 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 @@ -86,7 +108,7 @@ impl Direction { } else { start_revset.descendants_at(args.offset) } - .minus(&wc_revset), + .minus(working_revset), Direction::Prev => { if args.conflict { @@ -114,7 +136,26 @@ 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_commit: Commit = start_revset + .clone() + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .collect::, BackendError>>()? + .pop() + .unwrap(); + + 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() @@ -125,7 +166,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_commit.change_id(), + ))); } 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 0e63eaa545..2cfedddafc 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 descendant found 3 commit(s) forward from rlvkpnrzqnoo "###); } @@ -594,7 +594,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 "###); } @@ -858,14 +858,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 "###); } @@ -963,7 +964,7 @@ fn test_movement_edit_mode_auto() { // next --no-edit will fail on a linear tree. let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--no-edit"]); insta::assert_snapshot!(stderr, @r###" - Error: No descendant found 1 commit(s) forward + Error: No other descendant found 1 commit(s) forward from qpvuntsmwlqt "###); // to make it pass, first create a new commit @@ -1056,6 +1057,11 @@ fn test_movement_edit_mode_always() { ◆ 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###" @@ -1087,12 +1093,12 @@ fn test_movement_edit_mode_always() { 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 ├─╯ @@ -1103,18 +1109,23 @@ fn test_movement_edit_mode_always() { 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] @@ -1163,15 +1174,20 @@ fn test_movement_edit_mode_never() { ◆ 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 @@ -1182,18 +1198,10 @@ fn test_movement_edit_mode_never() { 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###"