From d50a7d96985435fdc8c59ea4103362b623fac530 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Mon, 11 Mar 2024 21:46:08 -0700 Subject: [PATCH 1/2] external diff editor: extract functions to populate and delete JJ_INSTRUCTIONS This will be reused for integration with the new `:builtin-web` diff editor in #3292. `instructions-path_to_cleanup` is moved into DiffWorkingCopies. DiffWorkingCopies: add instructions_path_to_cleanup --- cli/src/merge_tools/external.rs | 181 +++++++++++++++++++------------- 1 file changed, 106 insertions(+), 75 deletions(-) diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index d3e25ad7f4..036684101e 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -132,11 +132,14 @@ pub enum DiffCheckoutError { TreeState(#[from] TreeStateError), } +// TODO: Extract this type and related functions to a separate file, to be used +// by the external tools and edit_diff_web. struct DiffWorkingCopies { - _temp_dir: TempDir, + _temp_dir: TempDir, // Temp dir will be deleted when this is dropped left_tree_state: TreeState, right_tree_state: TreeState, output_tree_state: Option, + instructions_path_to_cleanup: Option, } impl DiffWorkingCopies { @@ -277,9 +280,108 @@ 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. +fn check_out_trees_for_diffedit( + store: &Arc, + left_tree: &MergedTree, + right_tree: &MergedTree, + matcher: &dyn Matcher, + output_is: Option, + instructions: Option<&str>, +) -> Result { + 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()) + .map_err(ExternalToolError::SetUpDir)?; + 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 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"\ +The content of this pane should NOT be edited. Any edits will be +lost. + +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)?; + 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)?; + diff_wc.instructions_path_to_cleanup = Some(instructions_path_to_cleanup); + } + + Ok(diff_wc) +} + +fn snapshot_diffedit_results( + diff_wc: DiffWorkingCopies, + base_ignores: Arc, +) -> Result { + 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()) +} + pub fn run_mergetool_external( editor: &ExternalMergeTool, file_merge: Merge>, @@ -423,72 +525,14 @@ pub fn edit_diff_external( ) -> Result { let got_output_field = find_all_variables(&editor.edit_args).contains(&"output"); let store = left_tree.store(); - let diff_wc = check_out_trees( + let diff_wc = check_out_trees_for_diffedit( store, left_tree, right_tree, matcher, got_output_field.then_some(DiffSide::Right), + instructions, )?; - 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()) - .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"\ -The content of this pane should NOT be edited. Any edits will be -lost. - -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)?; - 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)?; - } let patterns = diff_wc.to_command_variables(); let mut cmd = Command::new(&editor.program); @@ -505,21 +549,8 @@ diff editing in mind and be a little inaccurate. exit_status, })); } - if add_instructions.is_some() { - std::fs::remove_file(instructions_path_to_cleanup).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()) + snapshot_diffedit_results(diff_wc, base_ignores) } /// Generates textual diff by the specified `tool`, and writes into `writer`. From 599d37b5111848848ea194b71f7c2da3188fb427 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Thu, 14 Mar 2024 17:41:54 -0700 Subject: [PATCH 2/2] external diff editor: move utility functions to a new file This is preparation for #3292, which will use these functions. The main goal is to merge the parts of #3292 that are likely to cause merge conflicts with other PRs while I polish it up. --- cli/src/merge_tools/diff_working_copies.rs | 280 +++++++++++++++++++++ cli/src/merge_tools/external.rs | 274 +------------------- cli/src/merge_tools/mod.rs | 4 +- 3 files changed, 288 insertions(+), 270 deletions(-) create mode 100644 cli/src/merge_tools/diff_working_copies.rs diff --git a/cli/src/merge_tools/diff_working_copies.rs b/cli/src/merge_tools/diff_working_copies.rs new file mode 100644 index 0000000000..b3789b0927 --- /dev/null +++ b/cli/src/merge_tools/diff_working_copies.rs @@ -0,0 +1,280 @@ +use std::collections::HashMap; +use std::fs::File; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use futures::StreamExt; +use jj_lib::backend::MergedTreeId; +use jj_lib::fsmonitor::FsmonitorKind; +use jj_lib::gitignore::GitIgnoreFile; +use jj_lib::local_working_copy::{TreeState, TreeStateError}; +use jj_lib::matchers::Matcher; +use jj_lib::merged_tree::MergedTree; +use jj_lib::repo_path::RepoPathBuf; +use jj_lib::store::Store; +use jj_lib::working_copy::{CheckoutError, SnapshotOptions}; +use pollster::FutureExt; +use tempfile::TempDir; +use thiserror::Error; + +use super::external::ExternalToolError; +use super::DiffEditError; + +#[derive(Debug, Error)] +pub enum DiffCheckoutError { + #[error("Failed to write directories to diff")] + Checkout(#[from] CheckoutError), + #[error("Error setting up temporary directory")] + SetUpDir(#[source] std::io::Error), + #[error(transparent)] + TreeState(#[from] TreeStateError), +} + +pub(crate) struct DiffWorkingCopies { + _temp_dir: TempDir, // Temp dir will be deleted when this is dropped + left_tree_state: TreeState, + right_tree_state: TreeState, + output_tree_state: Option, + instructions_path_to_cleanup: Option, +} + +impl DiffWorkingCopies { + pub fn left_working_copy_path(&self) -> &Path { + self.left_tree_state.working_copy_path() + } + + pub fn right_working_copy_path(&self) -> &Path { + self.right_tree_state.working_copy_path() + } + + pub fn output_working_copy_path(&self) -> Option<&Path> { + self.output_tree_state + .as_ref() + .map(|state| state.working_copy_path()) + } + + pub fn to_command_variables(&self) -> HashMap<&'static str, &str> { + let left_wc_dir = self.left_working_copy_path(); + let right_wc_dir = self.right_working_copy_path(); + let mut result = maplit::hashmap! { + "left" => left_wc_dir.to_str().expect("temp_dir should be valid utf-8"), + "right" => right_wc_dir.to_str().expect("temp_dir should be valid utf-8"), + }; + if let Some(output_wc_dir) = self.output_working_copy_path() { + result.insert( + "output", + output_wc_dir + .to_str() + .expect("temp_dir should be valid utf-8"), + ); + } + result + } +} + +fn check_out( + store: Arc, + wc_dir: PathBuf, + state_dir: PathBuf, + tree: &MergedTree, + sparse_patterns: Vec, +) -> Result { + std::fs::create_dir(&wc_dir).map_err(DiffCheckoutError::SetUpDir)?; + std::fs::create_dir(&state_dir).map_err(DiffCheckoutError::SetUpDir)?; + let mut tree_state = TreeState::init(store, wc_dir, state_dir)?; + tree_state.set_sparse_patterns(sparse_patterns)?; + tree_state.check_out(tree)?; + Ok(tree_state) +} + +pub(crate) fn new_utf8_temp_dir(prefix: &str) -> io::Result { + let temp_dir = tempfile::Builder::new().prefix(prefix).tempdir()?; + if temp_dir.path().to_str().is_none() { + // Not using .display() as we know the path contains unprintable character + let message = format!("path {:?} is not valid UTF-8", temp_dir.path()); + return Err(io::Error::new(io::ErrorKind::InvalidData, message)); + } + Ok(temp_dir) +} + +pub(crate) fn set_readonly_recursively(path: &Path) -> Result<(), std::io::Error> { + // Directory permission is unchanged since files under readonly directory cannot + // be removed. + let metadata = path.symlink_metadata()?; + if metadata.is_dir() { + for entry in path.read_dir()? { + set_readonly_recursively(&entry?.path())?; + } + Ok(()) + } else if metadata.is_file() { + let mut perms = metadata.permissions(); + perms.set_readonly(true); + std::fs::set_permissions(path, perms) + } else { + Ok(()) + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) enum DiffSide { + // Left, + Right, +} + +/// Check out the two trees in temporary directories. Only include changed files +/// in the sparse checkout patterns. +pub(crate) fn check_out_trees( + store: &Arc, + left_tree: &MergedTree, + right_tree: &MergedTree, + matcher: &dyn Matcher, + output_is: Option, +) -> Result { + let changed_files: Vec<_> = left_tree + .diff_stream(right_tree, matcher) + .map(|(path, _diff)| path) + .collect() + .block_on(); + + let temp_dir = new_utf8_temp_dir("jj-diff-").map_err(DiffCheckoutError::SetUpDir)?; + let left_wc_dir = temp_dir.path().join("left"); + let left_state_dir = temp_dir.path().join("left_state"); + let right_wc_dir = temp_dir.path().join("right"); + let right_state_dir = temp_dir.path().join("right_state"); + let left_tree_state = check_out( + store.clone(), + left_wc_dir, + left_state_dir, + left_tree, + changed_files.clone(), + )?; + let right_tree_state = check_out( + store.clone(), + right_wc_dir, + right_state_dir, + right_tree, + changed_files.clone(), + )?; + let output_tree_state = output_is + .map(|output_side| { + let output_wc_dir = temp_dir.path().join("output"); + let output_state_dir = temp_dir.path().join("output_state"); + check_out( + store.clone(), + output_wc_dir, + output_state_dir, + match output_side { + // DiffSide::Left => left_tree, + DiffSide::Right => right_tree, + }, + changed_files, + ) + }) + .transpose()?; + Ok(DiffWorkingCopies { + _temp_dir: temp_dir, + 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, + left_tree: &MergedTree, + right_tree: &MergedTree, + matcher: &dyn Matcher, + output_is: Option, + instructions: Option<&str>, +) -> Result { + 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()) + .map_err(ExternalToolError::SetUpDir)?; + 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 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"\ +The content of this pane should NOT be edited. Any edits will be +lost. + +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)?; + 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)?; + diff_wc.instructions_path_to_cleanup = Some(instructions_path_to_cleanup); + } + + Ok(diff_wc) +} + +pub fn snapshot_diffedit_results( + diff_wc: DiffWorkingCopies, + base_ignores: Arc, +) -> Result { + 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()) +} diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index 036684101e..a5a6122472 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -1,28 +1,24 @@ use std::collections::HashMap; -use std::fs::File; use std::io::{self, Write}; -use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus, Stdio}; use std::sync::Arc; -use futures::StreamExt; use itertools::Itertools; use jj_lib::backend::{FileId, MergedTreeId, TreeValue}; use jj_lib::conflicts::{self, materialize_merge_result}; -use jj_lib::fsmonitor::FsmonitorKind; use jj_lib::gitignore::GitIgnoreFile; -use jj_lib::local_working_copy::{TreeState, TreeStateError}; use jj_lib::matchers::Matcher; use jj_lib::merge::{Merge, MergedTreeValue}; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; -use jj_lib::repo_path::{RepoPath, RepoPathBuf}; -use jj_lib::store::Store; -use jj_lib::working_copy::{CheckoutError, SnapshotOptions}; +use jj_lib::repo_path::RepoPath; use pollster::FutureExt; use regex::{Captures, Regex}; -use tempfile::TempDir; 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, +}; use super::{ConflictResolveError, DiffEditError, DiffGenerateError}; use crate::config::CommandNameAndArgs; use crate::ui::Ui; @@ -122,266 +118,6 @@ pub enum ExternalToolError { Io(#[source] std::io::Error), } -#[derive(Debug, Error)] -pub enum DiffCheckoutError { - #[error("Failed to write directories to diff")] - Checkout(#[from] CheckoutError), - #[error("Error setting up temporary directory")] - SetUpDir(#[source] std::io::Error), - #[error(transparent)] - TreeState(#[from] TreeStateError), -} - -// TODO: Extract this type and related functions to a separate file, to be used -// by the external tools and edit_diff_web. -struct DiffWorkingCopies { - _temp_dir: TempDir, // Temp dir will be deleted when this is dropped - left_tree_state: TreeState, - right_tree_state: TreeState, - output_tree_state: Option, - instructions_path_to_cleanup: Option, -} - -impl DiffWorkingCopies { - fn left_working_copy_path(&self) -> &Path { - self.left_tree_state.working_copy_path() - } - - fn right_working_copy_path(&self) -> &Path { - self.right_tree_state.working_copy_path() - } - - fn output_working_copy_path(&self) -> Option<&Path> { - self.output_tree_state - .as_ref() - .map(|state| state.working_copy_path()) - } - - fn to_command_variables(&self) -> HashMap<&'static str, &str> { - let left_wc_dir = self.left_working_copy_path(); - let right_wc_dir = self.right_working_copy_path(); - let mut result = maplit::hashmap! { - "left" => left_wc_dir.to_str().expect("temp_dir should be valid utf-8"), - "right" => right_wc_dir.to_str().expect("temp_dir should be valid utf-8"), - }; - if let Some(output_wc_dir) = self.output_working_copy_path() { - result.insert( - "output", - output_wc_dir - .to_str() - .expect("temp_dir should be valid utf-8"), - ); - } - result - } -} - -fn check_out( - store: Arc, - wc_dir: PathBuf, - state_dir: PathBuf, - tree: &MergedTree, - sparse_patterns: Vec, -) -> Result { - std::fs::create_dir(&wc_dir).map_err(DiffCheckoutError::SetUpDir)?; - std::fs::create_dir(&state_dir).map_err(DiffCheckoutError::SetUpDir)?; - let mut tree_state = TreeState::init(store, wc_dir, state_dir)?; - tree_state.set_sparse_patterns(sparse_patterns)?; - tree_state.check_out(tree)?; - Ok(tree_state) -} - -fn new_utf8_temp_dir(prefix: &str) -> io::Result { - let temp_dir = tempfile::Builder::new().prefix(prefix).tempdir()?; - if temp_dir.path().to_str().is_none() { - // Not using .display() as we know the path contains unprintable character - let message = format!("path {:?} is not valid UTF-8", temp_dir.path()); - return Err(io::Error::new(io::ErrorKind::InvalidData, message)); - } - Ok(temp_dir) -} - -fn set_readonly_recursively(path: &Path) -> Result<(), std::io::Error> { - // Directory permission is unchanged since files under readonly directory cannot - // be removed. - let metadata = path.symlink_metadata()?; - if metadata.is_dir() { - for entry in path.read_dir()? { - set_readonly_recursively(&entry?.path())?; - } - Ok(()) - } else if metadata.is_file() { - let mut perms = metadata.permissions(); - perms.set_readonly(true); - std::fs::set_permissions(path, perms) - } else { - Ok(()) - } -} - -#[derive(Debug, Clone, Copy)] -enum DiffSide { - // Left, - Right, -} - -/// Check out the two trees in temporary directories. Only include changed files -/// in the sparse checkout patterns. -fn check_out_trees( - store: &Arc, - left_tree: &MergedTree, - right_tree: &MergedTree, - matcher: &dyn Matcher, - output_is: Option, -) -> Result { - let changed_files: Vec<_> = left_tree - .diff_stream(right_tree, matcher) - .map(|(path, _diff)| path) - .collect() - .block_on(); - - let temp_dir = new_utf8_temp_dir("jj-diff-").map_err(DiffCheckoutError::SetUpDir)?; - let left_wc_dir = temp_dir.path().join("left"); - let left_state_dir = temp_dir.path().join("left_state"); - let right_wc_dir = temp_dir.path().join("right"); - let right_state_dir = temp_dir.path().join("right_state"); - let left_tree_state = check_out( - store.clone(), - left_wc_dir, - left_state_dir, - left_tree, - changed_files.clone(), - )?; - let right_tree_state = check_out( - store.clone(), - right_wc_dir, - right_state_dir, - right_tree, - changed_files.clone(), - )?; - let output_tree_state = output_is - .map(|output_side| { - let output_wc_dir = temp_dir.path().join("output"); - let output_state_dir = temp_dir.path().join("output_state"); - check_out( - store.clone(), - output_wc_dir, - output_state_dir, - match output_side { - // DiffSide::Left => left_tree, - DiffSide::Right => right_tree, - }, - changed_files, - ) - }) - .transpose()?; - Ok(DiffWorkingCopies { - _temp_dir: temp_dir, - 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. -fn check_out_trees_for_diffedit( - store: &Arc, - left_tree: &MergedTree, - right_tree: &MergedTree, - matcher: &dyn Matcher, - output_is: Option, - instructions: Option<&str>, -) -> Result { - 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()) - .map_err(ExternalToolError::SetUpDir)?; - 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 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"\ -The content of this pane should NOT be edited. Any edits will be -lost. - -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)?; - 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)?; - diff_wc.instructions_path_to_cleanup = Some(instructions_path_to_cleanup); - } - - Ok(diff_wc) -} - -fn snapshot_diffedit_results( - diff_wc: DiffWorkingCopies, - base_ignores: Arc, -) -> Result { - 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()) -} - pub fn run_mergetool_external( editor: &ExternalMergeTool, file_merge: Merge>, diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index a8e8299363..92deb61de9 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -13,6 +13,7 @@ // limitations under the License. mod builtin; +mod diff_working_copies; mod external; use std::sync::Arc; @@ -30,7 +31,8 @@ use pollster::FutureExt; use thiserror::Error; use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError}; -use self::external::{edit_diff_external, DiffCheckoutError, ExternalToolError}; +use self::diff_working_copies::DiffCheckoutError; +use self::external::{edit_diff_external, ExternalToolError}; pub use self::external::{generate_diff, ExternalMergeTool}; use crate::config::CommandNameAndArgs; use crate::ui::Ui;