diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index fe08b914f..1c3f9a593 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -24,7 +24,7 @@ use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemote use jj_lib::gitignore::GitIgnoreError; use jj_lib::op_heads_store::OpHeadResolutionError; use jj_lib::op_store::OpStoreError; -use jj_lib::op_walk::OpsetEvaluationError; +use jj_lib::op_walk::{OpsetEvaluationError, OpsetResolutionError}; use jj_lib::repo::{CheckOutCommitError, EditCommitError, RepoLoaderError, RewriteRootCommit}; use jj_lib::repo_path::{RepoPathBuf, UiPathParseError}; use jj_lib::revset::{ @@ -36,6 +36,7 @@ use jj_lib::working_copy::{ResetError, SnapshotError, WorkingCopyStateError}; use jj_lib::workspace::WorkspaceInitError; use thiserror::Error; +use crate::cli_util::short_operation_hash; use crate::diff_util::DiffRenderError; use crate::formatter::{FormatRecorder, Formatter}; use crate::merge_tools::{ConflictResolveError, DiffEditError, MergeToolConfigError}; @@ -291,7 +292,12 @@ impl From for CommandError { impl From for CommandError { fn from(err: OpsetEvaluationError) -> Self { match err { - OpsetEvaluationError::OpsetResolution(err) => user_error(err), + OpsetEvaluationError::OpsetResolution(err) => { + let hint = opset_resolution_error_hint(&err); + let mut cmd_err = user_error(err); + cmd_err.extend_hints(hint); + cmd_err + } OpsetEvaluationError::OpHeadResolution(err) => err.into(), OpsetEvaluationError::OpStore(err) => err.into(), } @@ -583,6 +589,22 @@ fn fileset_parse_error_hint(err: &FilesetParseError) -> Option { } } +fn opset_resolution_error_hint(err: &OpsetResolutionError) -> Option { + match err { + OpsetResolutionError::MultipleOperations { + expr: _, + candidates, + } => Some(format!( + "Try specifying one of the operations by ID: {}", + candidates.iter().map(short_operation_hash).join(", ") + )), + OpsetResolutionError::EmptyOperations(_) + | OpsetResolutionError::InvalidIdPrefix(_) + | OpsetResolutionError::NoSuchOperation(_) + | OpsetResolutionError::AmbiguousIdPrefix(_) => None, + } +} + fn revset_parse_error_hint(err: &RevsetParseError) -> Option { // Only for the bottom error, which is usually the root cause let bottom_err = iter::successors(Some(err), |e| e.origin()).last().unwrap(); diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index fb957b864..05c81dce1 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -90,6 +90,7 @@ fn test_op_log() { ); insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "@-"]), @r###" Error: The "@" expression resolved to more than one operation + Hint: Try specifying one of the operations by ID: 5f690688f7d7, cfb67eb2b65c "###); } diff --git a/lib/src/op_walk.rs b/lib/src/op_walk.rs index 14eee4fc6..b33e19101 100644 --- a/lib/src/op_walk.rs +++ b/lib/src/op_walk.rs @@ -50,8 +50,13 @@ pub enum OpsetResolutionError { // TODO: Maybe empty/multiple operations should be allowed, and rejected by // caller as needed. /// Expression resolved to multiple operations. - #[error(r#"The "{0}" expression resolved to more than one operation"#)] - MultipleOperations(String), + #[error(r#"The "{expr}" expression resolved to more than one operation"#)] + MultipleOperations { + /// Source expression. + expr: String, + /// Matched operation ids. + candidates: Vec, + }, /// Expression resolved to no operations. #[error(r#"The "{0}" expression resolved to no operations"#)] EmptyOperations(String), @@ -74,8 +79,12 @@ pub fn resolve_op_for_load( let op_store = repo_loader.op_store(); let op_heads_store = repo_loader.op_heads_store().as_ref(); let get_current_op = || { - op_heads_store::resolve_op_heads(op_heads_store, op_store, |_| { - Err(OpsetResolutionError::MultipleOperations("@".to_owned()).into()) + op_heads_store::resolve_op_heads(op_heads_store, op_store, |op_heads| { + Err(OpsetResolutionError::MultipleOperations { + expr: "@".to_owned(), + candidates: op_heads.iter().map(|op| op.id().clone()).collect(), + } + .into()) }) }; let get_head_ops = || get_current_head_ops(op_store, op_heads_store); @@ -127,7 +136,10 @@ fn resolve_single_op( operation = match neighbor_ops.len() { 0 => Err(OpsetResolutionError::EmptyOperations(op_str.to_owned()))?, 1 => neighbor_ops.pop().unwrap(), - _ => Err(OpsetResolutionError::MultipleOperations(op_str.to_owned()))?, + _ => Err(OpsetResolutionError::MultipleOperations { + expr: op_str.to_owned(), + candidates: neighbor_ops.iter().map(|op| op.id().clone()).collect(), + })?, }; } Ok(operation) diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index d7bb81b74..be3040ab6 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -580,14 +580,14 @@ fn test_resolve_op_parents_children() { assert_matches!( op_walk::resolve_op_with_repo(&repo, &format!("{op5_id_hex}-")), Err(OpsetEvaluationError::OpsetResolution( - OpsetResolutionError::MultipleOperations(_) + OpsetResolutionError::MultipleOperations { .. } )) ); let op2_id_hex = operations[2].id().hex(); assert_matches!( op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}+")), Err(OpsetEvaluationError::OpsetResolution( - OpsetResolutionError::MultipleOperations(_) + OpsetResolutionError::MultipleOperations { .. } )) ); }