From ea1aa5334c6cc0a7db988d07742bf420766fdef0 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 29 Feb 2024 18:11:11 +0900 Subject: [PATCH] merge_tools: take diff editor instruction as Option<&str> This clarifies that the instructions text can be omitted. All callers appear to pass non-empty instructions, though. --- cli/src/cli_util.rs | 4 ++-- cli/src/commands/commit.rs | 2 +- cli/src/commands/diffedit.rs | 8 +++++++- cli/src/commands/move.rs | 2 +- cli/src/commands/split.rs | 2 +- cli/src/commands/squash.rs | 2 +- cli/src/commands/unsquash.rs | 2 +- cli/src/merge_tools/external.rs | 15 +++++++++------ cli/src/merge_tools/mod.rs | 2 +- 9 files changed, 24 insertions(+), 15 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 3b3dac4725..5d350d0fa0 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1742,7 +1742,7 @@ impl WorkspaceCommandTransaction<'_> { left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, - instructions: &str, + instructions: Option<&str>, ) -> Result { let base_ignores = self.helper.base_ignores()?; let settings = &self.helper.settings; @@ -1763,7 +1763,7 @@ impl WorkspaceCommandTransaction<'_> { left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, - instructions: &str, + instructions: Option<&str>, interactive: bool, ) -> Result { if interactive { diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index 458e8593b3..4e7effec98 100644 --- a/cli/src/commands/commit.rs +++ b/cli/src/commands/commit.rs @@ -66,7 +66,7 @@ new working-copy commit. &base_tree, &commit.tree()?, matcher.as_ref(), - &instructions, + Some(&instructions), args.interactive, )?; let middle_tree = tx.repo().store().get_root_tree(&tree_id)?; diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index d077237fd9..cb1bad77b6 100644 --- a/cli/src/commands/diffedit.rs +++ b/cli/src/commands/diffedit.rs @@ -90,7 +90,13 @@ don't make any changes, then the operation will be aborted.", ); let base_tree = merge_commit_trees(tx.repo(), base_commits.as_slice())?; let tree = target_commit.tree()?; - let tree_id = tx.edit_diff(ui, &base_tree, &tree, &EverythingMatcher, &instructions)?; + let tree_id = tx.edit_diff( + ui, + &base_tree, + &tree, + &EverythingMatcher, + Some(&instructions), + )?; if tree_id == *target_commit.tree_id() { writeln!(ui.stderr(), "Nothing changed.")?; } else { diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index 2a7ceb1bc3..9d3b5528e5 100644 --- a/cli/src/commands/move.rs +++ b/cli/src/commands/move.rs @@ -93,7 +93,7 @@ from the source will be moved into the destination. &parent_tree, &source_tree, matcher.as_ref(), - &instructions, + Some(&instructions), args.interactive, )?; if args.interactive && new_parent_tree_id == parent_tree.id() { diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 7b1563a761..7f342fad96 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -81,7 +81,7 @@ don't make any changes, then the operation will be aborted. &base_tree, &end_tree, matcher.as_ref(), - &instructions, + Some(&instructions), interactive, )?; if &selected_tree_id == commit.tree_id() && interactive { diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index d249048a80..94863a3148 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -90,7 +90,7 @@ from the source will be moved into the parent. &parent_tree, &tree, matcher.as_ref(), - &instructions, + Some(&instructions), args.interactive, )?; if &new_parent_tree_id == parent.tree_id() { diff --git a/cli/src/commands/unsquash.rs b/cli/src/commands/unsquash.rs index 4ad63261d7..a1ac8e0d07 100644 --- a/cli/src/commands/unsquash.rs +++ b/cli/src/commands/unsquash.rs @@ -86,7 +86,7 @@ aborted. &parent_base_tree, &parent_tree, &EverythingMatcher, - &instructions, + Some(&instructions), )?; if new_parent_tree_id == parent_base_tree.id() { return Err(user_error("No changes selected")); diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index f9c59e2b0d..15c58d5797 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -427,7 +427,7 @@ pub fn edit_diff_external( left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, - instructions: &str, + instructions: Option<&str>, base_ignores: Arc, settings: &UserSettings, ) -> Result { @@ -452,10 +452,13 @@ pub fn edit_diff_external( let instructions_path_to_cleanup = output_wc_path.join("JJ-INSTRUCTIONS"); // In the unlikely event that the file already exists, then the user will simply // not get any instructions. - let add_instructions = settings.diff_instructions() - && !instructions.is_empty() - && !instructions_path_to_cleanup.exists(); - if add_instructions { + let add_instructions = if settings.diff_instructions() && !instructions_path_to_cleanup.exists() + { + instructions + } else { + None + }; + if let Some(instructions) = add_instructions { let mut output_instructions_file = File::create(&instructions_path_to_cleanup).map_err(ExternalToolError::SetUpDir)?; if diff_wc.right_working_copy_path() != output_wc_path { @@ -513,7 +516,7 @@ diff editing in mind and be a little inaccurate. exit_status, })); } - if add_instructions { + if add_instructions.is_some() { std::fs::remove_file(instructions_path_to_cleanup).ok(); } diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index a6c3310e80..f686b9859c 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -132,7 +132,7 @@ pub fn edit_diff( left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, - instructions: &str, + instructions: Option<&str>, base_ignores: Arc, settings: &UserSettings, ) -> Result {