mirror of
https://github.com/martinvonz/jj.git
synced 2024-09-11 13:56:29 +03:00
Update error message when no movement targets are found.
If movement commands don't find a target commit, they fail. However, it's usually not intuitive why they fail because in non-edit mode the start commit is the parent of the working commit. Adding the start commit change hash to the error message makes it easier for the user to figure out what is going on. Also, specifying 'No **other** descendant|ancestor...' helps make it clear what `jj` is really looking for. Part of #3947
This commit is contained in:
parent
1a73a47d93
commit
6ecc2dac81
@ -16,12 +16,12 @@ use std::io::Write;
|
||||
use std::rc::Rc;
|
||||
|
||||
use itertools::Itertools;
|
||||
use jj_lib::backend::CommitId;
|
||||
use jj_lib::backend::{BackendError, CommitId};
|
||||
use jj_lib::commit::Commit;
|
||||
use jj_lib::repo::Repo;
|
||||
use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};
|
||||
|
||||
use crate::cli_util::WorkspaceCommandHelper;
|
||||
use crate::cli_util::{short_change_hash, WorkspaceCommandHelper};
|
||||
use crate::command_error::{user_error, CommandError};
|
||||
use crate::ui::{MovementEditMode, Ui};
|
||||
|
||||
@ -44,29 +44,33 @@ impl Direction {
|
||||
}
|
||||
}
|
||||
|
||||
fn next_node_type(&self) -> String {
|
||||
fn missing_target_label(&self, edit: bool) -> String {
|
||||
match self {
|
||||
Direction::Next => "descendant".to_string(),
|
||||
Direction::Next => {
|
||||
if edit {
|
||||
// in edit mode, start_revset is the WC, so
|
||||
// we only look for direct descendants.
|
||||
"descendant".to_string()
|
||||
} else {
|
||||
// in none edit mode, start_revset is the parent
|
||||
// of WC, so we look for other descendants of
|
||||
// start_revset.
|
||||
"other descendant".to_string()
|
||||
}
|
||||
}
|
||||
// The WC can never be an ancestor of the start_revset since
|
||||
// start_revset is either itself or it's parent.
|
||||
Direction::Prev => "ancestor".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn get_target_revset(
|
||||
&self,
|
||||
working_commit_id: &CommitId,
|
||||
edit: bool,
|
||||
working_revset: &Rc<RevsetExpression>,
|
||||
start_revset: &Rc<RevsetExpression>,
|
||||
has_conflict: bool,
|
||||
change_offset: u64,
|
||||
) -> Result<Rc<RevsetExpression>, 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 edit {
|
||||
wc_revset.clone()
|
||||
} else {
|
||||
wc_revset.parents()
|
||||
};
|
||||
|
||||
let target_revset = match self {
|
||||
Direction::Next => if has_conflict {
|
||||
start_revset
|
||||
@ -77,7 +81,7 @@ impl Direction {
|
||||
} else {
|
||||
start_revset.descendants_at(change_offset)
|
||||
}
|
||||
.minus(&wc_revset),
|
||||
.minus(working_revset),
|
||||
|
||||
Direction::Prev => {
|
||||
if has_conflict {
|
||||
@ -115,8 +119,27 @@ pub fn get_target_commit(
|
||||
has_conflict: bool,
|
||||
change_offset: u64,
|
||||
) -> Result<Commit, 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 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::<Result<Vec<_>, BackendError>>()?
|
||||
.pop()
|
||||
.unwrap();
|
||||
|
||||
let target_revset =
|
||||
direction.get_target_revset(working_commit_id, edit, has_conflict, change_offset)?;
|
||||
direction.get_target_revset(&wc_revset, &start_revset, has_conflict, change_offset)?;
|
||||
|
||||
let targets: Vec<Commit> = target_revset
|
||||
.evaluate_programmatic(workspace_command.repo().as_ref())?
|
||||
.iter()
|
||||
@ -128,11 +151,12 @@ pub fn get_target_commit(
|
||||
[] => {
|
||||
// We found no descendant.
|
||||
return Err(user_error(format!(
|
||||
"No {} found {} commit{} {}",
|
||||
direction.next_node_type(),
|
||||
"No {} found {} commit{} {} from {}",
|
||||
direction.missing_target_label(edit),
|
||||
change_offset,
|
||||
if change_offset > 1 { "s" } else { "" },
|
||||
direction.direction(),
|
||||
short_change_hash(start_commit.change_id()),
|
||||
)));
|
||||
}
|
||||
commits => choose_commit(ui, workspace_command, &direction, commits)?,
|
||||
|
@ -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 commits forward
|
||||
Error: No descendant found 3 commits 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 commits back
|
||||
Error: No ancestor found 5 commits back from zsuskulnrvyr
|
||||
"###);
|
||||
}
|
||||
|
||||
@ -860,12 +860,12 @@ fn test_next_conflict_head() {
|
||||
"###);
|
||||
let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--conflict"]);
|
||||
insta::assert_snapshot!(stderr, @r###"
|
||||
Error: No descendant found 1 commit forward
|
||||
Error: No other descendant found 1 commit 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 forward
|
||||
Error: No descendant found 1 commit forward from rlvkpnrzqnoo
|
||||
"###);
|
||||
}
|
||||
|
||||
@ -1003,6 +1003,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###"
|
||||
@ -1030,6 +1035,11 @@ fn test_movement_edit_mode_always() {
|
||||
○ qpvuntsmwlqt first
|
||||
◆ zzzzzzzzzzzz
|
||||
"###);
|
||||
|
||||
let stderr = test_env.jj_cmd_failure(&repo_path, &["next"]);
|
||||
insta::assert_snapshot!(stderr, @r###"
|
||||
Error: No descendant found 1 commit forward from kkmpptxzrspx
|
||||
"###);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1078,15 +1088,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 commits 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
|
||||
@ -1094,19 +1109,9 @@ fn test_movement_edit_mode_never() {
|
||||
◆ zzzzzzzzzzzz
|
||||
"###);
|
||||
|
||||
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
|
||||
insta::assert_snapshot!(stdout, @"");
|
||||
let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "5"]);
|
||||
insta::assert_snapshot!(stderr, @r###"
|
||||
Working copy now at: kmkuslsw 8c4d85ef (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
|
||||
Error: No other descendant found 5 commits forward from rlvkpnrzqnoo
|
||||
"###);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user