config: introduce our ConfigGetError type

Since most callers don't need to handle loading/parsing errors, it's probably
better to add a separate error type for "get" operations. The other uses of
ConfigError will be migrated later.

Since ConfigGetError can preserve the source name/path information, this patch
also removes some ad-hock error handling codes.

cli/src/cli_util.rs

@@ -57,9 +57,9 @@ use jj_lib::backend::CommitId;
 use jj_lib::backend::MergedTreeId;
 use jj_lib::backend::TreeValue;
 use jj_lib::commit::Commit;
-use jj_lib::config::ConfigError;
+use jj_lib::config::ConfigGetError;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::config::ConfigNamePathBuf;
-use jj_lib::config::ConfigResultExt as _;
 use jj_lib::config::StackedConfig;
 use jj_lib::conflicts::ConflictMarkerStyle;
 use jj_lib::file_util;
@@ -2691,8 +2691,16 @@ fn load_template_aliases(
         let table = match layer.look_up_table(&table_name) {
             Ok(Some(table)) => table,
             Ok(None) => continue,
-            // TODO: rewrite error construction after migrating to toml_edit
-            Err(item) => return Err(item.clone().into_table().unwrap_err().into()),
+            Err(item) => {
+                // TODO: rewrite error construction after migrating to toml_edit
+                let error = item.clone().into_table().unwrap_err();
+                return Err(ConfigGetError::Type {
+                    name: table_name.to_string(),
+                    error: error.into(),
+                    source_path: layer.path.clone(),
+                }
+                .into());
+            }
         };
         // TODO: remove sorting after migrating to toml_edit
         for (decl, value) in table.iter().sorted_by_key(|&(decl, _)| decl) {
@@ -2721,7 +2729,7 @@ pub struct LogContentFormat {
 
 impl LogContentFormat {
     /// Creates new formatting helper for the terminal.
-    pub fn new(ui: &Ui, settings: &UserSettings) -> Result<Self, ConfigError> {
+    pub fn new(ui: &Ui, settings: &UserSettings) -> Result<Self, ConfigGetError> {
         Ok(LogContentFormat {
             width: ui.term_width(),
             word_wrap: settings.get_bool("ui.log-word-wrap")?,
@@ -2763,9 +2771,7 @@ pub fn run_ui_editor(settings: &UserSettings, edit_path: &Path) -> Result<(), Co
     // Work around UNC paths not being well supported on Windows (no-op for
     // non-Windows): https://github.com/martinvonz/jj/issues/3986
     let edit_path = dunce::simplified(edit_path);
-    let editor: CommandNameAndArgs = settings
-        .get("ui.editor")
-        .map_err(|err| config_error_with_message("Invalid `ui.editor`", err))?;
+    let editor: CommandNameAndArgs = settings.get("ui.editor")?;
     let mut cmd = editor.to_command();
     cmd.arg(edit_path);
     tracing::info!(?cmd, "running editor");
@@ -3084,7 +3090,7 @@ impl ValueParserFactory for RevisionArg {
 fn get_string_or_array(
     config: &StackedConfig,
     key: &'static str,
-) -> Result<Vec<String>, ConfigError> {
+) -> Result<Vec<String>, ConfigGetError> {
     config
         .get(key)
         .map(|string| vec![string])
@@ -3539,16 +3545,7 @@ impl CliRunner {
             config_env.reset_repo_path(loader.repo_path());
             config_env.reload_repo_config(&mut config)?;
         }
-        ui.reset(&config).map_err(|e| {
-            let user_config_path = config_env.existing_user_config_path();
-            let repo_config_path = config_env.existing_repo_config_path();
-            let paths = [repo_config_path, user_config_path]
-                .into_iter()
-                .flatten()
-                .map(|path| format!("- {}", path.display()))
-                .join("\n");
-            e.hinted(format!("Check the following config files:\n{paths}"))
-        })?;
+        ui.reset(&config)?;
 
         if env::var_os("COMPLETE").is_some() {
             return handle_shell_completion(ui, &self.app, &config, &cwd);

cli/src/command_error.rs

@@ -23,6 +23,7 @@ use std::sync::Arc;
 use itertools::Itertools as _;
 use jj_lib::backend::BackendError;
 use jj_lib::config::ConfigError;
+use jj_lib::config::ConfigGetError;
 use jj_lib::dsl_util::Diagnostics;
 use jj_lib::fileset::FilePatternParseError;
 use jj_lib::fileset::FilesetParseError;
@@ -251,6 +252,20 @@ impl From<ConfigEnvError> for CommandError {
     }
 }
 
+impl From<ConfigGetError> for CommandError {
+    fn from(err: ConfigGetError) -> Self {
+        let hint = match &err {
+            ConfigGetError::NotFound { .. } => None,
+            ConfigGetError::Type { source_path, .. } => source_path
+                .as_ref()
+                .map(|path| format!("Check the config file: {}", path.display())),
+        };
+        let mut cmd_err = config_error(err);
+        cmd_err.extend_hints(hint);
+        cmd_err
+    }
+}
+
 impl From<RewriteRootCommit> for CommandError {
     fn from(err: RewriteRootCommit) -> Self {
         internal_error_with_message("Attempted to rewrite the root commit", err)

cli/src/commands/config/get.rs

@@ -13,14 +13,15 @@
 // limitations under the License.
 
 use std::io::Write as _;
+use std::path::PathBuf;
 
 use clap_complete::ArgValueCandidates;
 use jj_lib::config::ConfigError;
+use jj_lib::config::ConfigGetError;
 use jj_lib::config::ConfigNamePathBuf;
 use tracing::instrument;
 
 use crate::cli_util::CommandHelper;
-use crate::command_error::config_error;
 use crate::command_error::CommandError;
 use crate::complete;
 use crate::ui::Ui;
@@ -48,29 +49,19 @@ pub fn cmd_config_get(
     args: &ConfigGetArgs,
 ) -> Result<(), CommandError> {
     let value = command.settings().get_value(&args.name)?;
-    let stringified = value.into_string().map_err(|err| match err {
-        ConfigError::Type {
-            origin,
-            unexpected,
-            expected,
-            key,
-        } => {
-            let expected = format!("a value convertible to {expected}");
-            // Copied from `impl fmt::Display for ConfigError`. We can't use
-            // the `Display` impl directly because `expected` is required to
-            // be a `'static str`.
-            let mut buf = String::new();
-            use std::fmt::Write;
-            write!(buf, "invalid type: {unexpected}, expected {expected}").unwrap();
-            if let Some(key) = key {
-                write!(buf, " for key `{key}`").unwrap();
+    let stringified = value.into_string().map_err(|err| -> CommandError {
+        match err {
+            ConfigError::Type {
+                origin, unexpected, ..
+            } => ConfigGetError::Type {
+                name: args.name.to_string(),
+                error: format!("Expected a value convertible to a string, but is {unexpected}")
+                    .into(),
+                source_path: origin.map(PathBuf::from),
             }
-            if let Some(origin) = origin {
-                write!(buf, " in {origin}").unwrap();
-            }
-            config_error(buf)
+            .into(),
+            err => err.into(),
         }
-        err => err.into(),
     })?;
     writeln!(ui.stdout(), "{stringified}")?;
     Ok(())

cli/src/commands/git/fetch.rs

@@ -14,7 +14,7 @@
 
 use clap_complete::ArgValueCandidates;
 use itertools::Itertools;
-use jj_lib::config::ConfigResultExt as _;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::repo::Repo;
 use jj_lib::settings::UserSettings;
 use jj_lib::str_util::StringPattern;

cli/src/commands/git/push.rs

@@ -21,7 +21,7 @@ use clap::ArgGroup;
 use clap_complete::ArgValueCandidates;
 use itertools::Itertools;
 use jj_lib::backend::CommitId;
-use jj_lib::config::ConfigResultExt as _;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::git;
 use jj_lib::git::GitBranchPushTargets;
 use jj_lib::git::GitPushError;

cli/src/commands/log.rs

@@ -14,8 +14,8 @@
 
 use clap_complete::ArgValueCandidates;
 use jj_lib::backend::CommitId;
-use jj_lib::config::ConfigError;
-use jj_lib::config::ConfigResultExt as _;
+use jj_lib::config::ConfigGetError;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::graph::GraphEdgeType;
 use jj_lib::graph::ReverseGraphIterator;
 use jj_lib::graph::TopoGroupedGraphIterator;
@@ -331,7 +331,7 @@ pub(crate) fn cmd_log(
 pub fn get_node_template(
     style: GraphStyle,
     settings: &UserSettings,
-) -> Result<String, ConfigError> {
+) -> Result<String, ConfigGetError> {
     let symbol = settings.get_string("templates.log_node").optional()?;
     let default = if style.is_ascii() {
         "builtin_log_node_ascii"

cli/src/commands/operation/log.rs

@@ -15,8 +15,8 @@
 use std::slice;
 
 use itertools::Itertools as _;
-use jj_lib::config::ConfigError;
-use jj_lib::config::ConfigResultExt as _;
+use jj_lib::config::ConfigGetError;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::op_walk;
 use jj_lib::operation::Operation;
 use jj_lib::repo::RepoLoader;
@@ -247,7 +247,7 @@ fn do_op_log(
     Ok(())
 }
 
-fn get_node_template(style: GraphStyle, settings: &UserSettings) -> Result<String, ConfigError> {
+fn get_node_template(style: GraphStyle, settings: &UserSettings) -> Result<String, ConfigGetError> {
     let symbol = settings.get_string("templates.op_log_node").optional()?;
     let default = if style.is_ascii() {
         "builtin_op_log_node_ascii"

cli/src/diff_util.rs

@@ -32,8 +32,8 @@ use jj_lib::backend::CommitId;
 use jj_lib::backend::CopyRecord;
 use jj_lib::backend::TreeValue;
 use jj_lib::commit::Commit;
-use jj_lib::config::ConfigError;
-use jj_lib::config::ConfigResultExt as _;
+use jj_lib::config::ConfigGetError;
+use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::conflicts::materialize_merge_result_to_bytes;
 use jj_lib::conflicts::materialized_diff_stream;
 use jj_lib::conflicts::ConflictMarkerStyle;
@@ -147,7 +147,7 @@ pub enum DiffFormat {
 pub fn diff_formats_for(
     settings: &UserSettings,
     args: &DiffFormatArgs,
-) -> Result<Vec<DiffFormat>, ConfigError> {
+) -> Result<Vec<DiffFormat>, ConfigGetError> {
     let formats = diff_formats_from_args(settings, args)?;
     if formats.is_empty() {
         Ok(vec![default_diff_format(settings, args)?])
@@ -162,7 +162,7 @@ pub fn diff_formats_for_log(
     settings: &UserSettings,
     args: &DiffFormatArgs,
     patch: bool,
-) -> Result<Vec<DiffFormat>, ConfigError> {
+) -> Result<Vec<DiffFormat>, ConfigGetError> {
     let mut formats = diff_formats_from_args(settings, args)?;
     // --patch implies default if no format other than --summary is specified
     if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) {
@@ -175,7 +175,7 @@ pub fn diff_formats_for_log(
 fn diff_formats_from_args(
     settings: &UserSettings,
     args: &DiffFormatArgs,
-) -> Result<Vec<DiffFormat>, ConfigError> {
+) -> Result<Vec<DiffFormat>, ConfigGetError> {
     let mut formats = Vec::new();
     if args.summary {
         formats.push(DiffFormat::Summary);
@@ -209,7 +209,7 @@ fn diff_formats_from_args(
 fn default_diff_format(
     settings: &UserSettings,
     args: &DiffFormatArgs,
-) -> Result<DiffFormat, ConfigError> {
+) -> Result<DiffFormat, ConfigGetError> {
     if let Some(args) = settings.get("ui.diff.tool").optional()? {
         // External "tool" overrides the internal "format" option.
         let tool = if let CommandNameAndArgs::String(name) = &args {
@@ -243,7 +243,11 @@ fn default_diff_format(
             let options = DiffStatOptions::from_args(args);
             Ok(DiffFormat::Stat(Box::new(options)))
         }
-        _ => Err(ConfigError::Message(format!("invalid diff format: {name}"))),
+        _ => Err(ConfigGetError::Type {
+            name: "ui.diff.format".to_owned(),
+            error: format!("Invalid diff format: {name}").into(),
+            source_path: None,
+        }),
     }
 }
 
@@ -544,15 +548,16 @@ impl ColorWordsDiffOptions {
     fn from_settings_and_args(
         settings: &UserSettings,
         args: &DiffFormatArgs,
-    ) -> Result<Self, ConfigError> {
+    ) -> Result<Self, ConfigGetError> {
         let max_inline_alternation = {
-            let key = "diff.color-words.max-inline-alternation";
-            match settings.get_int(key)? {
+            let name = "diff.color-words.max-inline-alternation";
+            match settings.get_int(name)? {
                 -1 => None, // unlimited
-                n => Some(
-                    usize::try_from(n)
-                        .map_err(|err| ConfigError::Message(format!("invalid {key}: {err}")))?,
-                ),
+                n => Some(usize::try_from(n).map_err(|err| ConfigGetError::Type {
+                    name: name.to_owned(),
+                    error: err.into(),
+                    source_path: None,
+                })?),
             }
         };
         let context = args
@@ -1252,7 +1257,7 @@ impl UnifiedDiffOptions {
     fn from_settings_and_args(
         settings: &UserSettings,
         args: &DiffFormatArgs,
-    ) -> Result<Self, ConfigError> {
+    ) -> Result<Self, ConfigGetError> {
         let context = args
             .context
             .map_or_else(|| settings.get("diff.git.context"), Ok)?;