op_walk: include operation ids in multiple match error

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<OpHeadResolutionError> for CommandError {
 impl From<OpsetEvaluationError> 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<String> {
     }
 }
 
+fn opset_resolution_error_hint(err: &OpsetResolutionError) -> Option<String> {
+    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<String> {
     // Only for the bottom error, which is usually the root cause
     let bottom_err = iter::successors(Some(err), |e| e.origin()).last().unwrap();

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
     "###);
 }
 

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<OperationId>,
+    },
     /// 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)

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 { .. }
         ))
     );
 }