merge tools: split DiffEditWorkingCopies from DiffWorkingCopies

As suggested by @yuja in https://github.com/martinvonz/jj/pull/3296#discussion_r1525829712
This commit is contained in:
Ilya Grigoriev 2024-03-15 17:26:14 -07:00
parent 6a63c705aa
commit 8600750fce
2 changed files with 90 additions and 83 deletions

View file

@ -36,7 +36,6 @@ pub(crate) struct DiffWorkingCopies {
left_tree_state: TreeState, left_tree_state: TreeState,
right_tree_state: TreeState, right_tree_state: TreeState,
output_tree_state: Option<TreeState>, output_tree_state: Option<TreeState>,
instructions_path_to_cleanup: Option<PathBuf>,
} }
impl DiffWorkingCopies { impl DiffWorkingCopies {
@ -177,50 +176,55 @@ pub(crate) fn check_out_trees(
left_tree_state, left_tree_state,
right_tree_state, right_tree_state,
output_tree_state, output_tree_state,
instructions_path_to_cleanup: None,
}) })
} }
/// Checks out the trees, populates JJ_INSTRUCTIONS, and makes appropriate sides pub(crate) struct DiffEditWorkingCopies {
/// readonly. pub working_copies: DiffWorkingCopies,
pub fn check_out_trees_for_diffedit( instructions_path_to_cleanup: Option<PathBuf>,
store: &Arc<Store>, }
left_tree: &MergedTree,
right_tree: &MergedTree,
matcher: &dyn Matcher,
output_is: Option<DiffSide>,
instructions: Option<&str>,
) -> Result<DiffWorkingCopies, DiffEditError> {
let mut diff_wc = check_out_trees(store, left_tree, right_tree, matcher, output_is)?;
let got_output_field = output_is.is_some();
set_readonly_recursively(diff_wc.left_working_copy_path()) impl DiffEditWorkingCopies {
.map_err(ExternalToolError::SetUpDir)?; /// Checks out the trees, populates JJ_INSTRUCTIONS, and makes appropriate
if got_output_field { /// sides readonly.
set_readonly_recursively(diff_wc.right_working_copy_path()) pub fn check_out(
store: &Arc<Store>,
left_tree: &MergedTree,
right_tree: &MergedTree,
matcher: &dyn Matcher,
output_is: Option<DiffSide>,
instructions: Option<&str>,
) -> Result<Self, DiffEditError> {
let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, output_is)?;
let got_output_field = output_is.is_some();
set_readonly_recursively(diff_wc.left_working_copy_path())
.map_err(ExternalToolError::SetUpDir)?; .map_err(ExternalToolError::SetUpDir)?;
} if got_output_field {
let output_wc_path = diff_wc set_readonly_recursively(diff_wc.right_working_copy_path())
.output_working_copy_path() .map_err(ExternalToolError::SetUpDir)?;
.unwrap_or_else(|| diff_wc.right_working_copy_path()); }
let output_instructions_path = output_wc_path.join("JJ-INSTRUCTIONS"); let output_wc_path = diff_wc
// In the unlikely event that the file already exists, then the user will simply .output_working_copy_path()
// not get any instructions. .unwrap_or_else(|| diff_wc.right_working_copy_path());
let add_instructions = if !output_instructions_path.exists() { let output_instructions_path = output_wc_path.join("JJ-INSTRUCTIONS");
instructions // In the unlikely event that the file already exists, then the user will simply
} else { // not get any instructions.
None let add_instructions = if !output_instructions_path.exists() {
}; instructions
if let Some(instructions) = add_instructions { } else {
let mut output_instructions_file = None
File::create(&output_instructions_path).map_err(ExternalToolError::SetUpDir)?; };
if diff_wc.right_working_copy_path() != output_wc_path { if let Some(instructions) = add_instructions {
let mut right_instructions_file = let mut output_instructions_file =
File::create(diff_wc.right_working_copy_path().join("JJ-INSTRUCTIONS")) File::create(&output_instructions_path).map_err(ExternalToolError::SetUpDir)?;
.map_err(ExternalToolError::SetUpDir)?; if diff_wc.right_working_copy_path() != output_wc_path {
right_instructions_file let mut right_instructions_file =
.write_all( File::create(diff_wc.right_working_copy_path().join("JJ-INSTRUCTIONS"))
b"\ .map_err(ExternalToolError::SetUpDir)?;
right_instructions_file
.write_all(
b"\
The content of this pane should NOT be edited. Any edits will be The content of this pane should NOT be edited. Any edits will be
lost. lost.
@ -229,16 +233,16 @@ the following instructions may have been written with a 2-pane
diff editing in mind and be a little inaccurate. diff editing in mind and be a little inaccurate.
", ",
) )
.map_err(ExternalToolError::SetUpDir)?; .map_err(ExternalToolError::SetUpDir)?;
right_instructions_file right_instructions_file
.write_all(instructions.as_bytes()) .write_all(instructions.as_bytes())
.map_err(ExternalToolError::SetUpDir)?; .map_err(ExternalToolError::SetUpDir)?;
// Note that some diff tools might not show this message and delete the contents // Note that some diff tools might not show this message and delete the contents
// of the output dir instead. Meld does show this message. // of the output dir instead. Meld does show this message.
output_instructions_file output_instructions_file
.write_all( .write_all(
b"\ b"\
Please make your edits in this pane. Please make your edits in this pane.
You are using the experimental 3-pane diff editor config. Some of You are using the experimental 3-pane diff editor config. Some of
@ -246,35 +250,39 @@ the following instructions may have been written with a 2-pane
diff editing in mind and be a little inaccurate. diff editing in mind and be a little inaccurate.
", ",
) )
.map_err(ExternalToolError::SetUpDir)?;
}
output_instructions_file
.write_all(instructions.as_bytes())
.map_err(ExternalToolError::SetUpDir)?; .map_err(ExternalToolError::SetUpDir)?;
};
Ok(Self {
working_copies: diff_wc,
instructions_path_to_cleanup: add_instructions.map(|_| output_instructions_path),
})
}
pub fn snapshot_results(
self,
base_ignores: Arc<GitIgnoreFile>,
) -> Result<MergedTreeId, DiffEditError> {
if let Some(path) = self.instructions_path_to_cleanup {
std::fs::remove_file(path).ok();
} }
output_instructions_file
.write_all(instructions.as_bytes()) let diff_wc = self.working_copies;
.map_err(ExternalToolError::SetUpDir)?; // Snapshot changes in the temporary output directory.
diff_wc.instructions_path_to_cleanup = Some(output_instructions_path); let mut output_tree_state = diff_wc
.output_tree_state
.unwrap_or(diff_wc.right_tree_state);
output_tree_state.snapshot(SnapshotOptions {
base_ignores,
fsmonitor_kind: FsmonitorKind::None,
progress: None,
max_new_file_size: u64::MAX,
})?;
Ok(output_tree_state.current_tree_id().clone())
} }
Ok(diff_wc)
}
pub fn snapshot_diffedit_results(
diff_wc: DiffWorkingCopies,
base_ignores: Arc<GitIgnoreFile>,
) -> Result<MergedTreeId, DiffEditError> {
if let Some(path) = diff_wc.instructions_path_to_cleanup {
std::fs::remove_file(path).ok();
}
// Snapshot changes in the temporary output directory.
let mut output_tree_state = diff_wc
.output_tree_state
.unwrap_or(diff_wc.right_tree_state);
output_tree_state.snapshot(SnapshotOptions {
base_ignores,
fsmonitor_kind: FsmonitorKind::None,
progress: None,
max_new_file_size: u64::MAX,
})?;
Ok(output_tree_state.current_tree_id().clone())
} }

View file

@ -16,8 +16,7 @@ use regex::{Captures, Regex};
use thiserror::Error; use thiserror::Error;
use super::diff_working_copies::{ use super::diff_working_copies::{
check_out_trees, check_out_trees_for_diffedit, new_utf8_temp_dir, set_readonly_recursively, check_out_trees, new_utf8_temp_dir, set_readonly_recursively, DiffEditWorkingCopies, DiffSide,
snapshot_diffedit_results, DiffSide,
}; };
use super::{ConflictResolveError, DiffEditError, DiffGenerateError}; use super::{ConflictResolveError, DiffEditError, DiffGenerateError};
use crate::config::CommandNameAndArgs; use crate::config::CommandNameAndArgs;
@ -261,7 +260,7 @@ pub fn edit_diff_external(
) -> Result<MergedTreeId, DiffEditError> { ) -> Result<MergedTreeId, DiffEditError> {
let got_output_field = find_all_variables(&editor.edit_args).contains(&"output"); let got_output_field = find_all_variables(&editor.edit_args).contains(&"output");
let store = left_tree.store(); let store = left_tree.store();
let diff_wc = check_out_trees_for_diffedit( let diffedit_wc = DiffEditWorkingCopies::check_out(
store, store,
left_tree, left_tree,
right_tree, right_tree,
@ -270,7 +269,7 @@ pub fn edit_diff_external(
instructions, instructions,
)?; )?;
let patterns = diff_wc.to_command_variables(); let patterns = diffedit_wc.working_copies.to_command_variables();
let mut cmd = Command::new(&editor.program); let mut cmd = Command::new(&editor.program);
cmd.args(interpolate_variables(&editor.edit_args, &patterns)); cmd.args(interpolate_variables(&editor.edit_args, &patterns));
tracing::info!(?cmd, "Invoking the external diff editor:"); tracing::info!(?cmd, "Invoking the external diff editor:");
@ -286,7 +285,7 @@ pub fn edit_diff_external(
})); }));
} }
snapshot_diffedit_results(diff_wc, base_ignores) diffedit_wc.snapshot_results(base_ignores)
} }
/// Generates textual diff by the specified `tool`, and writes into `writer`. /// Generates textual diff by the specified `tool`, and writes into `writer`.