cli: error out if ui.diff.format configuration is invalid

As I'm going to add "--tool <name>" option, we'll anyway need error handling
there.

src/commands/mod.rs

@@ -1445,14 +1445,15 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(),
         to_tree = commit.tree()
     }
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
+    let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
     ui.request_pager();
     diff_util::show_diff(
         ui.stdout_formatter().as_mut(),
         &workspace_command,
         &from_tree,
         &to_tree,
         matcher.as_ref(),
-        &diff_util::diff_formats_for(command.settings(), &args.format),
+        &diff_formats,
     )?;
     Ok(())
 }
@@ -1463,6 +1464,7 @@ fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(),
     let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
     let template_string = command.settings().config().get_string("templates.show")?;
     let template = workspace_command.parse_commit_template(&template_string)?;
+    let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
     ui.request_pager();
     let mut formatter = ui.stdout_formatter();
     let formatter = formatter.as_mut();
@@ -1472,7 +1474,7 @@ fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(),
         &workspace_command,
         &commit,
         &EverythingMatcher,
-        &diff_util::diff_formats_for(command.settings(), &args.format),
+        &diff_formats,
     )?;
     Ok(())
 }
@@ -1609,7 +1611,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
 
     let store = repo.store();
     let diff_formats =
-        diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch);
+        diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?;
 
     let template_string = match &args.template {
         Some(value) => value.to_string(),
@@ -1745,7 +1747,7 @@ fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result
     let wc_commit_id = workspace_command.get_wc_commit_id();
 
     let diff_formats =
-        diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch);
+        diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?;
 
     let template_string = match &args.template {
         Some(value) => value.to_string(),
@@ -1849,14 +1851,15 @@ fn cmd_interdiff(
 
     let from_tree = rebase_to_dest_parent(&workspace_command, &from, &to)?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
+    let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
     ui.request_pager();
     diff_util::show_diff(
         ui.stdout_formatter().as_mut(),
         &workspace_command,
         &from_tree,
         &to.tree(),
         matcher.as_ref(),
-        &diff_util::diff_formats_for(command.settings(), &args.format),
+        &diff_formats,
     )
 }
 

src/diff_util.rs

@@ -25,7 +25,7 @@ use jj_lib::files::DiffLine;
 use jj_lib::matchers::Matcher;
 use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::repo_path::RepoPath;
-use jj_lib::settings::UserSettings;
+use jj_lib::settings::{ConfigResultExt as _, UserSettings};
 use jj_lib::tree::{Tree, TreeDiffIterator};
 use jj_lib::{diff, files, rewrite, tree};
 use tracing::instrument;
@@ -66,12 +66,15 @@ pub enum DiffFormat {
 }
 
 /// Returns a list of requested diff formats, which will never be empty.
-pub fn diff_formats_for(settings: &UserSettings, args: &DiffFormatArgs) -> Vec<DiffFormat> {
+pub fn diff_formats_for(
+    settings: &UserSettings,
+    args: &DiffFormatArgs,
+) -> Result<Vec<DiffFormat>, config::ConfigError> {
     let formats = diff_formats_from_args(args);
     if formats.is_empty() {
-        vec![default_diff_format(settings)]
+        Ok(vec![default_diff_format(settings)?])
     } else {
-        formats
+        Ok(formats)
     }
 }
 
@@ -81,14 +84,14 @@ pub fn diff_formats_for_log(
     settings: &UserSettings,
     args: &DiffFormatArgs,
     patch: bool,
-) -> Vec<DiffFormat> {
+) -> Result<Vec<DiffFormat>, config::ConfigError> {
     let mut formats = diff_formats_from_args(args);
     // --patch implies default if no format other than --summary is specified
     if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) {
-        formats.push(default_diff_format(settings));
+        formats.push(default_diff_format(settings)?);
         formats.dedup();
     }
-    formats
+    Ok(formats)
 }
 
 fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec<DiffFormat> {
@@ -103,19 +106,23 @@ fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec<DiffFormat> {
     .collect()
 }
 
-fn default_diff_format(settings: &UserSettings) -> DiffFormat {
-    match settings
-        .config()
-        .get_string("ui.diff.format")
-        // old config name
-        .or_else(|_| settings.config().get_string("diff.format"))
-        .as_deref()
-    {
-        Ok("summary") => DiffFormat::Summary,
-        Ok("types") => DiffFormat::Types,
-        Ok("git") => DiffFormat::Git,
-        Ok("color-words") => DiffFormat::ColorWords,
-        _ => DiffFormat::ColorWords,
+fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::ConfigError> {
+    let config = settings.config();
+    let name = if let Some(name) = config.get_string("ui.diff.format").optional()? {
+        name
+    } else if let Some(name) = config.get_string("diff.format").optional()? {
+        name // old config name
+    } else {
+        "color-words".to_owned()
+    };
+    match name.as_ref() {
+        "summary" => Ok(DiffFormat::Summary),
+        "types" => Ok(DiffFormat::Types),
+        "git" => Ok(DiffFormat::Git),
+        "color-words" => Ok(DiffFormat::ColorWords),
+        _ => Err(config::ConfigError::Message(format!(
+            "invalid diff format: {name}"
+        ))),
     }
 }