Skip to content

merge tools: split DiffEditWorkingCopies from DiffWorkingCopies #3306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilyagr merged 2 commits into from
Mar 16, 2024
Merged

merge tools: split DiffEditWorkingCopies from DiffWorkingCopies #3306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aed2187
merge tools: rename variable to make diff for the next commit simpler
ilyagr Mar 16, 2024
5741b2c
merge tools: split DiffEditWorkingCopies from DiffWorkingCopies
ilyagr Mar 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 84 additions & 76 deletions cli/src/merge_tools/diff_working_copies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub(crate) struct DiffWorkingCopies {
left_tree_state: TreeState,
right_tree_state: TreeState,
output_tree_state: Option<TreeState>,
instructions_path_to_cleanup: Option<PathBuf>,
}

impl DiffWorkingCopies {
Expand Down Expand Up @@ -177,50 +176,55 @@ pub(crate) fn check_out_trees(
left_tree_state,
right_tree_state,
output_tree_state,
instructions_path_to_cleanup: None,
})
}

/// Checks out the trees, populates JJ_INSTRUCTIONS, and makes appropriate sides
/// readonly.
pub fn check_out_trees_for_diffedit(
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();
pub(crate) struct DiffEditWorkingCopies {
pub working_copies: DiffWorkingCopies,
instructions_path_to_cleanup: Option<PathBuf>,
}

set_readonly_recursively(diff_wc.left_working_copy_path())
.map_err(ExternalToolError::SetUpDir)?;
if got_output_field {
set_readonly_recursively(diff_wc.right_working_copy_path())
impl DiffEditWorkingCopies {
/// Checks out the trees, populates JJ_INSTRUCTIONS, and makes appropriate
/// sides readonly.
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)?;
}
let output_wc_path = diff_wc
.output_working_copy_path()
.unwrap_or_else(|| diff_wc.right_working_copy_path());
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 = if !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 {
let mut right_instructions_file =
File::create(diff_wc.right_working_copy_path().join("JJ-INSTRUCTIONS"))
.map_err(ExternalToolError::SetUpDir)?;
right_instructions_file
.write_all(
b"\
if got_output_field {
set_readonly_recursively(diff_wc.right_working_copy_path())
.map_err(ExternalToolError::SetUpDir)?;
}
let output_wc_path = diff_wc
.output_working_copy_path()
.unwrap_or_else(|| diff_wc.right_working_copy_path());
let output_instructions_path = 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 = if !output_instructions_path.exists() {
instructions
} else {
None
};
if let Some(instructions) = add_instructions {
let mut output_instructions_file =
File::create(&output_instructions_path).map_err(ExternalToolError::SetUpDir)?;
if diff_wc.right_working_copy_path() != output_wc_path {
let mut right_instructions_file =
File::create(diff_wc.right_working_copy_path().join("JJ-INSTRUCTIONS"))
.map_err(ExternalToolError::SetUpDir)?;
right_instructions_file
.write_all(
b"\
The content of this pane should NOT be edited. Any edits will be
lost.

Expand All @@ -229,52 +233,56 @@ the following instructions may have been written with a 2-pane
diff editing in mind and be a little inaccurate.

",
)
.map_err(ExternalToolError::SetUpDir)?;
right_instructions_file
.write_all(instructions.as_bytes())
.map_err(ExternalToolError::SetUpDir)?;
// Note that some diff tools might not show this message and delete the contents
// of the output dir instead. Meld does show this message.
output_instructions_file
.write_all(
b"\
)
.map_err(ExternalToolError::SetUpDir)?;
right_instructions_file
.write_all(instructions.as_bytes())
.map_err(ExternalToolError::SetUpDir)?;
// Note that some diff tools might not show this message and delete the contents
// of the output dir instead. Meld does show this message.
output_instructions_file
.write_all(
b"\
Please make your edits in this pane.

You are using the experimental 3-pane diff editor config. Some of
the following instructions may have been written with a 2-pane
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)?;
}
output_instructions_file
.write_all(instructions.as_bytes())
.map_err(ExternalToolError::SetUpDir)?;
diff_wc.instructions_path_to_cleanup = Some(instructions_path_to_cleanup);
};

Ok(Self {
working_copies: diff_wc,
instructions_path_to_cleanup: add_instructions.map(|_| output_instructions_path),
})
}

Ok(diff_wc)
}
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();
}

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();
let diff_wc = self.working_copies;
// 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())
}

// 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())
}
9 changes: 4 additions & 5 deletions cli/src/merge_tools/external.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ use regex::{Captures, Regex};
use thiserror::Error;

use super::diff_working_copies::{
check_out_trees, check_out_trees_for_diffedit, new_utf8_temp_dir, set_readonly_recursively,
snapshot_diffedit_results, DiffSide,
check_out_trees, new_utf8_temp_dir, set_readonly_recursively, DiffEditWorkingCopies, DiffSide,
};
use super::{ConflictResolveError, DiffEditError, DiffGenerateError};
use crate::config::CommandNameAndArgs;
Expand Down Expand Up @@ -261,7 +260,7 @@ pub fn edit_diff_external(
) -> Result<MergedTreeId, DiffEditError> {
let got_output_field = find_all_variables(&editor.edit_args).contains(&"output");
let store = left_tree.store();
let diff_wc = check_out_trees_for_diffedit(
let diffedit_wc = DiffEditWorkingCopies::check_out(
store,
left_tree,
right_tree,
Expand All @@ -270,7 +269,7 @@ pub fn edit_diff_external(
instructions,
)?;

let patterns = diff_wc.to_command_variables();
let patterns = diffedit_wc.working_copies.to_command_variables();
let mut cmd = Command::new(&editor.program);
cmd.args(interpolate_variables(&editor.edit_args, &patterns));
tracing::info!(?cmd, "Invoking the external diff editor:");
Expand All @@ -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`.
Expand Down