merge_tools: extract configuration error to separate type

write!(ui.hint(), ..) error is suppressed because it seemed weird if the
configuration error had io::Error variant. The write error isn't important
anyway.

cli/src/cli_util.rs

@@ -88,6 +88,7 @@ use crate::git_util::{
 };
 use crate::merge_tools::{
     ConflictResolveError, DiffEditError, DiffEditor, DiffGenerateError, MergeEditor,
+    MergeToolConfigError,
 };
 use crate::template_parser::{TemplateAliasesMap, TemplateParseError, TemplateParseErrorKind};
 use crate::templater::Template;
@@ -353,6 +354,12 @@ 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)
+    }
+}
+
 impl From<git2::Error> for CommandError {
     fn from(err: git2::Error) -> Self {
         user_error_with_message("Git operation failed", err)
@@ -1110,13 +1117,13 @@ impl WorkspaceCommandHelper {
 
     /// Loads diff editor from the settings.
     // TODO: override settings by --tool= option (#2575)
-    pub fn diff_editor(&self, ui: &Ui) -> Result<DiffEditor, DiffEditError> {
+    pub fn diff_editor(&self, ui: &Ui) -> Result<DiffEditor, MergeToolConfigError> {
         DiffEditor::from_settings(ui, &self.settings)
     }
 
     /// Loads 3-way merge editor from the settings.
     // TODO: override settings by --tool= option (#2575)
-    pub fn merge_editor(&self, ui: &Ui) -> Result<MergeEditor, ConflictResolveError> {
+    pub fn merge_editor(&self, ui: &Ui) -> Result<MergeEditor, MergeToolConfigError> {
         MergeEditor::from_settings(ui, &self.settings)
     }
 

cli/src/merge_tools/external.rs

@@ -5,7 +5,6 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, ExitStatus, Stdio};
 use std::sync::Arc;
 
-use config::ConfigError;
 use futures::StreamExt;
 use itertools::Itertools;
 use jj_lib::backend::{FileId, MergedTreeId, TreeValue};
@@ -107,13 +106,6 @@ impl ExternalMergeTool {
 
 #[derive(Debug, Error)]
 pub enum ExternalToolError {
-    #[error("Invalid config")]
-    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)"
-    )]
-    MergeArgsNotConfigured { tool_name: String },
     #[error("Error setting up temporary directory")]
     SetUpDir(#[source] std::io::Error),
     // TODO: Remove the "(run with --debug to see the exact invocation)"

cli/src/merge_tools/mod.rs

@@ -150,6 +150,17 @@ pub fn edit_diff(
     }
 }
 
+#[derive(Debug, Error)]
+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)"
+    )]
+    MergeArgsNotConfigured { tool_name: String },
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum MergeTool {
     Builtin,
@@ -161,7 +172,7 @@ fn editor_args_from_settings(
     ui: &Ui,
     settings: &UserSettings,
     key: &str,
-) -> Result<CommandNameAndArgs, ExternalToolError> {
+) -> Result<CommandNameAndArgs, ConfigError> {
     // TODO: Make this configuration have a table of possible editors and detect the
     // best one here.
     if let Some(args) = settings.config().get(key).optional()? {
@@ -173,7 +184,7 @@ fn editor_args_from_settings(
             "Using default editor '{default_editor}'; run `jj config set --user {key} :builtin` \
              to disable this message."
         )
-        .map_err(ExternalToolError::Io)?;
+        .ok();
         Ok(default_editor.into())
     }
 }
@@ -219,11 +230,10 @@ pub struct DiffEditor {
 
 impl DiffEditor {
     /// Loads the default diff editor from the settings.
-    // TODO: extract tool configuration error from DiffEditError
-    pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result<Self, DiffEditError> {
+    pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result<Self, MergeToolConfigError> {
         let args = editor_args_from_settings(ui, settings, "ui.diff-editor")?;
         let tool = if let CommandNameAndArgs::String(name) = &args {
-            get_tool_config(settings, name).map_err(ExternalToolError::Config)?
+            get_tool_config(settings, name)?
         } else {
             None
         }
@@ -240,20 +250,18 @@ pub struct MergeEditor {
 
 impl MergeEditor {
     /// Loads the default 3-way merge editor from the settings.
-    // TODO: extract tool configuration error from ConflictResolveError
-    pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result<Self, ConflictResolveError> {
+    pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result<Self, MergeToolConfigError> {
         let args = editor_args_from_settings(ui, settings, "ui.merge-editor")?;
         let tool = if let CommandNameAndArgs::String(name) = &args {
-            get_tool_config(settings, name).map_err(ExternalToolError::Config)?
+            get_tool_config(settings, name)?
         } else {
             None
         }
         .unwrap_or_else(|| MergeTool::External(ExternalMergeTool::with_merge_args(&args)));
         if matches!(&tool, MergeTool::External(mergetool) if mergetool.merge_args.is_empty()) {
-            return Err(ExternalToolError::MergeArgsNotConfigured {
+            return Err(MergeToolConfigError::MergeArgsNotConfigured {
                 tool_name: args.to_string(),
-            }
-            .into());
+            });
         }
         Ok(MergeEditor { tool })
     }
@@ -435,11 +443,9 @@ mod tests {
 
         // Just program name
         insta::assert_debug_snapshot!(get(r#"ui.merge-editor = "my-merge""#).unwrap_err(), @r###"
-        ExternalTool(
-            MergeArgsNotConfigured {
-                tool_name: "my-merge",
-            },
-        )
+        MergeArgsNotConfigured {
+            tool_name: "my-merge",
+        }
         "###);
 
         // String args
@@ -527,11 +533,9 @@ mod tests {
         // List args should never be a merge-tools key
         insta::assert_debug_snapshot!(
             get(r#"ui.merge-editor = ["meld"]"#).unwrap_err(), @r###"
-        ExternalTool(
-            MergeArgsNotConfigured {
-                tool_name: "meld",
-            },
-        )
+        MergeArgsNotConfigured {
+            tool_name: "meld",
+        }
         "###);
 
         // Invalid type