diff --git a/src/merge_tools.rs b/src/merge_tools.rs index 451029fed..e7e5f5c4d 100644 --- a/src/merge_tools.rs +++ b/src/merge_tools.rs @@ -43,21 +43,22 @@ pub enum ExternalToolError { ConfigError(#[from] ConfigError), #[error("Error setting up temporary directory: {0:?}")] SetUpDirError(#[source] std::io::Error), - #[error("Error executing '{tool_binary}' with args {args:?}: {source}")] + // TODO: Remove the "(run with --verbose to see the exact invocation)" + // from this and other errors. Print it as a hint but only if --verbose is *not* set. + #[error( + "Error executing '{tool_binary}' (run with --verbose to see the exact invocation). \ + {source}" + )] FailedToExecute { tool_binary: String, - args: Vec, #[source] source: std::io::Error, }, #[error( - "Tool exited with a non-zero code.\n Details: tool '{tool_binary}' was \ - executed with args {args:?}. Exit code: {}.", + "Tool exited with a non-zero code (run with --verbose to see the exact invocation). Exit code: {}.", exit_status.code().map(|c| c.to_string()).unwrap_or_else(|| "".to_string()) )] ToolAborted { - tool_binary: String, - args: Vec, exit_status: std::process::ExitStatus, }, #[error("I/O error: {0:?}")] @@ -101,7 +102,10 @@ pub enum ConflictResolveError { removes: usize, adds: usize, }, - #[error("The output file is either unchanged or empty after the editor quit.")] + #[error( + "The output file is either unchanged or empty after the editor quit (run with --verbose \ + to see the exact invocation)." + )] EmptyOrUnchanged, #[error("Backend error: {0:?}")] BackendError(#[from] jujutsu_lib::backend::BackendError), @@ -227,22 +231,17 @@ pub fn run_mergetool( .try_collect()?; let args = interpolate_mergetool_filename_patterns(&editor.merge_args, &paths); - let args_str = args - .iter() - .map(|p| p.to_string_lossy().into_owned()) - .collect_vec(); - let exit_status = Command::new(&editor.program) - .args(args) + let mut cmd = Command::new(&editor.program); + cmd.args(args); + tracing::debug!(?cmd, "Invoking the external merge tool:"); + let exit_status = cmd .status() .map_err(|e| ExternalToolError::FailedToExecute { tool_binary: editor.program.clone(), - args: args_str.clone(), source: e, })?; if !exit_status.success() { return Err(ConflictResolveError::from(ExternalToolError::ToolAborted { - tool_binary: editor.program, - args: args_str, exit_status, })); } @@ -351,24 +350,20 @@ pub fn edit_diff( } let editor = get_diff_editor_from_settings(ui, settings)?; - let mut args_str = editor.edit_args.clone(); - args_str.push(left_wc_dir.display().to_string()); - args_str.push(right_wc_dir.display().to_string()); // Start a diff editor on the two directories. - let exit_status = Command::new(&editor.program) - .args(&editor.edit_args) + let mut cmd = Command::new(&editor.program); + cmd.args(&editor.edit_args) .arg(&left_wc_dir) - .arg(&right_wc_dir) + .arg(&right_wc_dir); + tracing::debug!(?cmd, "Invoking the external diff editor:"); + let exit_status = cmd .status() .map_err(|e| ExternalToolError::FailedToExecute { tool_binary: editor.program.clone(), - args: args_str.clone(), source: e, })?; if !exit_status.success() { return Err(DiffEditError::from(ExternalToolError::ToolAborted { - tool_binary: editor.program, - args: args_str, exit_status, })); } diff --git a/tests/test_diffedit_command.rs b/tests/test_diffedit_command.rs index d35d00fa9..7d3cb8e63 100644 --- a/tests/test_diffedit_command.rs +++ b/tests/test_diffedit_command.rs @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use regex::Regex; - use crate::common::TestEnvironment; pub mod common; @@ -52,10 +50,8 @@ fn test_diffedit() { // Nothing happens if the diff-editor exits with an error std::fs::write(&edit_script, "rm file2\0fail").unwrap(); - let stderr = test_env.jj_cmd_failure(&repo_path, &["diffedit"]); - insta::assert_snapshot!(Regex::new(r"Details: [^\n]+").unwrap().replace(&stderr, "Details: "), @r###" - Error: Failed to edit diff: Tool exited with a non-zero code. - Details: + insta::assert_snapshot!(&test_env.jj_cmd_failure(&repo_path, &["diffedit"]), @r###" + Error: Failed to edit diff: Tool exited with a non-zero code (run with --verbose to see the exact invocation). Exit code: 1. "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]); insta::assert_snapshot!(stdout, @r###" @@ -202,10 +198,8 @@ fn test_diffedit_old_restore_interactive_tests() { // Nothing happens if the diff-editor exits with an error std::fs::write(&edit_script, "rm file2\0fail").unwrap(); - let stderr = test_env.jj_cmd_failure(&repo_path, &["diffedit", "--from", "@-"]); - insta::assert_snapshot!(Regex::new(r"Details: [^\n]+").unwrap().replace(&stderr, "Details: "), @r###" - Error: Failed to edit diff: Tool exited with a non-zero code. - Details: + insta::assert_snapshot!(&test_env.jj_cmd_failure(&repo_path, &["diffedit", "--from", "@-"]), @r###" + Error: Failed to edit diff: Tool exited with a non-zero code (run with --verbose to see the exact invocation). Exit code: 1. "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]); insta::assert_snapshot!(stdout, @r###" diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs index efa8a7a8d..2f9e81dea 100644 --- a/tests/test_resolve_command.rs +++ b/tests/test_resolve_command.rs @@ -275,17 +275,15 @@ fn check_resolve_produces_input_file( std::fs::write(editor_script, format!("expect\n{expected_content}")).unwrap(); let merge_arg_config = format!(r#"merge-tools.fake-editor.merge-args = ["${role}"]"#); - let error = - test_env.jj_cmd_failure(repo_path, &["resolve", "--config-toml", &merge_arg_config]); // This error means that fake-editor exited successfully but did not modify the // output file. // We cannot use `insta::assert_snapshot!` here after insta 1.22 due to // https://github.com/mitsuhiko/insta/commit/745b45b. Hopefully, this will again become possible // in the future. See also https://github.com/mitsuhiko/insta/issues/313. assert_eq!( - error, + &test_env.jj_cmd_failure(repo_path, &["resolve", "--config-toml", &merge_arg_config]), "Error: Failed to use external tool to resolve: The output file is either unchanged or \ - empty after the editor quit.\n" + empty after the editor quit (run with --verbose to see the exact invocation).\n" ); }