From 04158c3744c6e70d5215f051d0c0f06c481ba874 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Sat, 4 May 2024 21:37:08 -0700
Subject: [PATCH] External merge tools: better error message for unsupported
 usage

I've heard of one instance of a person being confused by the error.

Previously, the error was:

```
Error: Failed to load tool configuration
Caused by: To use `diffedit3` as a merge tool, the config `merge-tools.diffedit3.merge-args` must be defined (see docs for details)

```

Now, it is:

```
Error: The tool `diffedit3` cannot be used as a merge tool with `jj resolve`.
Hint: To use `diffedit3` as a merge tool, the config `merge-tools.diffedit3.merge-args` must be defined (see docs for details)
```

TODO for future PR: allow setting `merge-tools.TOOL.edit-args = false` so that
attempting to use TOOL as a diff editor fails. This would be helpful, for
example, for the `vscode` tool.
---
 cli/src/command_error.rs        | 15 ++++++++++++++-
 cli/src/merge_tools/external.rs |  5 +++++
 cli/src/merge_tools/mod.rs      |  5 +----
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs
index a95506700..7fb674b3b 100644
--- a/cli/src/command_error.rs
+++ b/cli/src/command_error.rs
@@ -375,7 +375,20 @@ impl From<ConflictResolveError> for CommandError {
 
 impl From<MergeToolConfigError> for CommandError {
     fn from(err: MergeToolConfigError) -> Self {
-        user_error_with_message("Failed to load tool configuration", err)
+        match &err {
+            MergeToolConfigError::MergeArgsNotConfigured { tool_name } => {
+                let tool_name = tool_name.clone();
+                user_error_with_hint(
+                    err,
+                    format!(
+                        "To use `{tool_name}` as a merge tool, the config \
+                         `merge-tools.{tool_name}.merge-args` must be defined (see docs for \
+                         details)"
+                    ),
+                )
+            }
+            _ => user_error_with_message("Failed to load tool configuration", err),
+        }
     }
 }
 
diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs
index 1aac8f5f5..469847f70 100644
--- a/cli/src/merge_tools/external.rs
+++ b/cli/src/merge_tools/external.rs
@@ -55,6 +55,11 @@ impl Default for ExternalMergeTool {
     fn default() -> Self {
         Self {
             program: String::new(),
+            // TODO(ilyagr): There should be a way to explicitly specify that a
+            // certain tool (e.g. vscode as of this writing) cannot be used as a
+            // diff editor (or a diff tool). A possible TOML syntax would be
+            // `edit-args = false`, or `edit-args = []`, or `edit = { disabled =
+            // true }` to go with `edit = { args = [...] }`.
             diff_args: ["$left", "$right"].map(ToOwned::to_owned).to_vec(),
             edit_args: ["$left", "$right"].map(ToOwned::to_owned).to_vec(),
             merge_args: vec![],
diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs
index 95140cd13..8a45fa7a3 100644
--- a/cli/src/merge_tools/mod.rs
+++ b/cli/src/merge_tools/mod.rs
@@ -91,10 +91,7 @@ pub enum ConflictResolveError {
 pub enum MergeToolConfigError {
     #[error(transparent)]
     Config(#[from] ConfigError),
-    #[error(
-        "To use `{tool_name}` as a merge tool, the config `merge-tools.{tool_name}.merge-args` \
-         must be defined (see docs for details)"
-    )]
+    #[error("The tool `{tool_name}` cannot be used as a merge tool with `jj resolve`")]
     MergeArgsNotConfigured { tool_name: String },
 }