From fbabd0c9a7978c6490a898566d51a6edb68c7f28 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 1acc9295e..5b4067791 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1751,7 +1751,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; @@ -1772,7 +1772,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 458e8593b..4e7effec9 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 d077237fd..cb1bad77b 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 2a7ceb1bc..9d3b5528e 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 7b1563a76..7f342fad9 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 d249048a8..94863a314 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 4ad63261d..a1ac8e0d0 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 f9c59e2b0..15c58d579 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 a6c3310e8..f686b9859 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 {