From 65b06e92ede571ff7a1ad1e3fddeb8be06fc1cdb Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 2 Aug 2023 13:20:27 +0900 Subject: [PATCH] cli: error out if ui.diff.format configuration is invalid As I'm going to add "--tool " option, we'll anyway need error handling there. --- src/commands/mod.rs | 13 ++++++++----- src/diff_util.rs | 47 ++++++++++++++++++++++++++------------------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index ff1d350c37..10204d4f3b 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1445,6 +1445,7 @@ 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(), @@ -1452,7 +1453,7 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), &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,6 +1851,7 @@ 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(), @@ -1856,7 +1859,7 @@ fn cmd_interdiff( &from_tree, &to.tree(), matcher.as_ref(), - &diff_util::diff_formats_for(command.settings(), &args.format), + &diff_formats, ) } diff --git a/src/diff_util.rs b/src/diff_util.rs index 909d52c1eb..81177f75cc 100644 --- a/src/diff_util.rs +++ b/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 { +pub fn diff_formats_for( + settings: &UserSettings, + args: &DiffFormatArgs, +) -> Result, 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 { +) -> Result, 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 { @@ -103,19 +106,23 @@ fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec { .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 { + 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}" + ))), } }