cli: add more CommandError factory functions, consolidate inner type

I'm going to reorganize CommandError as (kind, err, hints) tuple so that we
can add_hint() to the constructed error object.

Some config error messages are slightly adjusted because the inner error is
now printed in separate line.

cli/src/cli_util.rs

@@ -74,8 +74,9 @@ use tracing_chrome::ChromeLayerBuilder;
 use tracing_subscriber::prelude::*;
 
 use crate::command_error::{
-    handle_command_result, internal_error, internal_error_with_message, user_error,
-    user_error_with_hint, user_error_with_message, CommandError,
+    cli_error, config_error_with_message, handle_command_result, internal_error,
+    internal_error_with_message, user_error, user_error_with_hint, user_error_with_message,
+    CommandError,
 };
 use crate::commit_templater::{CommitTemplateLanguage, CommitTemplateLanguageExtension};
 use crate::config::{
@@ -886,7 +887,10 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
                 .unwrap_or_else(|_| self.settings.default_revset());
             if !revset_string.is_empty() {
                 let disambiguation_revset = self.parse_revset(&revset_string).map_err(|err| {
-                    CommandError::ConfigError(format!("Invalid `revsets.short-prefixes`: {err}"))
+                    config_error_with_message(
+                        "Invalid `revsets.short-prefixes`",
+                        revset_util::format_parse_error(&err),
+                    )
                 })?;
                 context = context.disambiguate_within(revset::optimize(disambiguation_revset));
             }
@@ -1884,7 +1888,7 @@ pub fn run_ui_editor(settings: &UserSettings, edit_path: &PathBuf) -> Result<(),
     let editor: CommandNameAndArgs = settings
         .config()
         .get("ui.editor")
-        .map_err(|err| CommandError::ConfigError(format!("ui.editor: {err}")))?;
+        .map_err(|err| config_error_with_message("Invalid `ui.editor`", err))?;
     let exit_status = editor.to_command().arg(edit_path).status().map_err(|err| {
         user_error_with_message(
             format!(
@@ -2309,7 +2313,7 @@ pub fn expand_args(
         if let Some(string_arg) = arg_os.to_str() {
             string_args.push(string_arg.to_owned());
         } else {
-            return Err(CommandError::CliError("Non-utf8 argument".to_string()));
+            return Err(cli_error("Non-utf8 argument"));
         }
     }
 

cli/src/command_error.rs

@@ -48,9 +48,9 @@ pub enum CommandError {
         err: Arc<dyn error::Error + Send + Sync>,
         hint: Option<String>,
     },
-    ConfigError(String),
+    ConfigError(Arc<dyn error::Error + Send + Sync>),
     /// Invalid command line
-    CliError(String),
+    CliError(Arc<dyn error::Error + Send + Sync>),
     /// Invalid command line detected by clap
     ClapCliError {
         err: Arc<clap::Error>,
@@ -116,6 +116,21 @@ pub fn user_error_with_hint_opt(
     }
 }
 
+pub fn config_error(err: impl Into<Box<dyn error::Error + Send + Sync>>) -> CommandError {
+    CommandError::ConfigError(Arc::from(err.into()))
+}
+
+pub fn config_error_with_message(
+    message: impl Into<String>,
+    source: impl Into<Box<dyn error::Error + Send + Sync>>,
+) -> CommandError {
+    config_error(ErrorWithMessage::new(message, source))
+}
+
+pub fn cli_error(err: impl Into<Box<dyn error::Error + Send + Sync>>) -> CommandError {
+    CommandError::CliError(Arc::from(err.into()))
+}
+
 pub fn internal_error(err: impl Into<Box<dyn error::Error + Send + Sync>>) -> CommandError {
     CommandError::InternalError(Arc::from(err.into()))
 }
@@ -152,13 +167,13 @@ impl From<io::Error> for CommandError {
 
 impl From<config::ConfigError> for CommandError {
     fn from(err: config::ConfigError) -> Self {
-        CommandError::ConfigError(err.to_string())
+        config_error(err)
     }
 }
 
 impl From<crate::config::ConfigError> for CommandError {
     fn from(err: crate::config::ConfigError) -> Self {
-        CommandError::ConfigError(err.to_string())
+        config_error(err)
     }
 }
 
@@ -475,16 +490,18 @@ fn try_handle_command_result(
             }
             Ok(ExitCode::from(1))
         }
-        Err(CommandError::ConfigError(message)) => {
-            writeln!(ui.error(), "Config error: {message}")?;
+        Err(CommandError::ConfigError(err)) => {
+            writeln!(ui.error(), "Config error: {err}")?;
+            print_error_sources(ui, err.source())?;
             writeln!(
                 ui.hint(),
                 "For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md."
             )?;
             Ok(ExitCode::from(1))
         }
-        Err(CommandError::CliError(message)) => {
-            writeln!(ui.error(), "Error: {message}")?;
+        Err(CommandError::CliError(err)) => {
+            writeln!(ui.error(), "Error: {err}")?;
+            print_error_sources(ui, err.source())?;
             Ok(ExitCode::from(2))
         }
         Err(CommandError::ClapCliError { err, hint }) => {

cli/src/commands/config.rs

@@ -22,7 +22,7 @@ use crate::cli_util::{
     get_new_config_file_path, run_ui_editor, serialize_config_value, write_config_value_to_file,
     CommandHelper,
 };
-use crate::command_error::{user_error, CommandError};
+use crate::command_error::{config_error, user_error, CommandError};
 use crate::config::{AnnotatedValue, ConfigSource};
 use crate::generic_templater::GenericTemplateLanguage;
 use crate::template_builder::TemplateLanguage as _;
@@ -295,7 +295,7 @@ pub(crate) fn cmd_config_get(
                 if let Some(origin) = origin {
                     write!(buf, " in {origin}").unwrap();
                 }
-                CommandError::ConfigError(buf.to_string())
+                config_error(buf)
             }
             err => err.into(),
         })?;

cli/src/commands/merge.rs

@@ -16,7 +16,7 @@ use tracing::instrument;
 
 use super::new;
 use crate::cli_util::CommandHelper;
-use crate::command_error::CommandError;
+use crate::command_error::{cli_error, CommandError};
 use crate::ui::Ui;
 
 #[instrument(skip_all)]
@@ -34,9 +34,7 @@ pub(crate) fn cmd_merge(
         "warning: `jj merge` will be removed in a future version, and this will be a hard error"
     )?;
     if args.revisions.len() < 2 {
-        return Err(CommandError::CliError(String::from(
-            "Merge requires at least two revisions",
-        )));
+        return Err(cli_error("Merge requires at least two revisions"));
     }
     new::cmd_new(ui, command, args)
 }

cli/src/commands/resolve.rs

@@ -23,7 +23,7 @@ use jj_lib::repo_path::RepoPathBuf;
 use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, WorkspaceCommandHelper};
-use crate::command_error::CommandError;
+use crate::command_error::{cli_error, CommandError};
 use crate::formatter::Formatter;
 use crate::ui::Ui;
 
@@ -81,14 +81,11 @@ pub(crate) fn cmd_resolve(
         .filter(|path| matcher.matches(&path.0))
         .collect_vec();
     if conflicts.is_empty() {
-        return Err(CommandError::CliError(format!(
-            "No conflicts found {}",
-            if args.paths.is_empty() {
-                "at this revision"
-            } else {
-                "at the given path(s)"
-            }
-        )));
+        return Err(cli_error(if args.paths.is_empty() {
+            "No conflicts found at this revision"
+        } else {
+            "No conflicts found at the given path(s)"
+        }));
     }
     if args.list {
         return print_conflicted_paths(

cli/src/ui.rs

@@ -21,7 +21,7 @@ use std::{env, fmt, io, mem};
 use minus::Pager as MinusPager;
 use tracing::instrument;
 
-use crate::command_error::CommandError;
+use crate::command_error::{config_error_with_message, CommandError};
 use crate::config::CommandNameAndArgs;
 use crate::formatter::{Formatter, FormatterFactory, LabeledWriter};
 
@@ -233,13 +233,13 @@ pub enum PaginationChoice {
 fn pagination_setting(config: &config::Config) -> Result<PaginationChoice, CommandError> {
     config
         .get::<PaginationChoice>("ui.paginate")
-        .map_err(|err| CommandError::ConfigError(format!("Invalid `ui.paginate`: {err}")))
+        .map_err(|err| config_error_with_message("Invalid `ui.paginate`", err))
 }
 
 fn pager_setting(config: &config::Config) -> Result<CommandNameAndArgs, CommandError> {
     config
         .get::<CommandNameAndArgs>("ui.pager")
-        .map_err(|err| CommandError::ConfigError(format!("Invalid `ui.pager`: {err}")))
+        .map_err(|err| config_error_with_message("Invalid `ui.pager`", err))
 }
 
 impl Ui {

cli/tests/test_log_command.rs

@@ -445,7 +445,8 @@ fn test_log_bad_short_prefixes() {
     let stderr = test_env.jj_cmd_failure(&repo_path, &["status"]);
     insta::assert_snapshot!(stderr,
         @r###"
-    Config error: Invalid `revsets.short-prefixes`:  --> 1:1
+    Config error: Invalid `revsets.short-prefixes`
+    Caused by: Failed to parse revset:  --> 1:1
       |
     1 | !nval!d
       | ^---