diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index b9b2dc586f..6f52ad294a 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -40,7 +40,7 @@ use unicode_width::UnicodeWidthStr as _; use crate::cli_util::{CommandError, WorkspaceCommandHelper}; use crate::config::CommandNameAndArgs; use crate::formatter::Formatter; -use crate::merge_tools::{self, ExternalMergeTool, MergeTool}; +use crate::merge_tools::{self, ExternalMergeTool}; use crate::text_util; use crate::ui::Ui; @@ -129,14 +129,9 @@ fn diff_formats_from_args( .filter_map(|(arg, format)| arg.then_some(format)) .collect_vec(); if let Some(name) = &args.tool { - let tool = merge_tools::get_tool_config(settings, name)? - .unwrap_or_else(|| MergeTool::External(ExternalMergeTool::with_program(name))); - match tool { - MergeTool::Builtin => {} - MergeTool::External(tool) => { - formats.push(DiffFormat::Tool(Box::new(tool))); - } - } + let tool = merge_tools::get_external_tool_config(settings, name)? + .unwrap_or_else(|| ExternalMergeTool::with_program(name)); + formats.push(DiffFormat::Tool(Box::new(tool))); } Ok(formats) } @@ -146,17 +141,12 @@ fn default_diff_format(settings: &UserSettings) -> Result {} - MergeTool::External(tool) => { - return Ok(DiffFormat::Tool(Box::new(tool))); - } - } + .unwrap_or_else(|| ExternalMergeTool::with_diff_args(&args)); + return Ok(DiffFormat::Tool(Box::new(tool))); } let name = if let Some(name) = config.get_string("ui.diff.format").optional()? { name diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 98570c7e8e..a6c3310e80 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -185,10 +185,7 @@ fn editor_args_from_settings( /// Resolves builtin merge tool name or loads external tool options from /// `[merge-tools.]`. -pub fn get_tool_config( - settings: &UserSettings, - name: &str, -) -> Result, ConfigError> { +fn get_tool_config(settings: &UserSettings, name: &str) -> Result, ConfigError> { if name == BUILTIN_EDITOR_NAME { Ok(Some(MergeTool::Builtin)) } else { diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index e73ceccf4a..32a1313f17 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -753,6 +753,14 @@ fn test_diff_external_tool() { insta::assert_snapshot!(stderr, @r###" Tool exited with a non-zero code (run with --debug to see the exact invocation). Exit code: 1. "###); + + // --tool=:builtin shouldn't be ignored + let stderr = test_env.jj_cmd_failure(&repo_path, &["diff", "--tool=:builtin"]); + insta::assert_snapshot!(strip_last_line(&stderr), @r###" + Error: Failed to generate diff + Caused by: + 1: Error executing ':builtin' (run with --debug to see the exact invocation) + "###); } #[cfg(unix)] @@ -977,3 +985,9 @@ fn test_diff_binary() { 4 files changed, 6 insertions(+), 6 deletions(-) "###); } + +fn strip_last_line(s: &str) -> &str { + s.trim_end_matches('\n') + .rsplit_once('\n') + .map_or(s, |(h, _)| &s[..h.len() + 1]) +}