From 59a79fdcc0b6803109d2ff8eb10d5b37ffefcf13 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 19 Aug 2024 13:45:10 +0900 Subject: [PATCH] conflicts: extract materialize_merge_result_to_bytes() helper We have many callers of materialize_merge_result() who just want in-memory buffer. --- cli/src/diff_util.rs | 20 ++++++-------------- cli/src/merge_tools/builtin.rs | 6 ++---- cli/src/merge_tools/external.rs | 11 ++++------- lib/src/annotate.rs | 14 +++----------- lib/src/conflicts.rs | 13 +++++++++++++ lib/src/default_index/revset_engine.rs | 7 ++----- lib/src/local_working_copy.rs | 6 ++---- lib/tests/test_conflicts.rs | 6 ++---- 8 files changed, 34 insertions(+), 49 deletions(-) diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 7a7761882..06f3931af 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -32,7 +32,7 @@ use jj_lib::backend::CommitId; use jj_lib::backend::CopyRecord; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; -use jj_lib::conflicts::materialize_merge_result; +use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::materialized_diff_stream; use jj_lib::conflicts::MaterializedTreeDiffEntry; use jj_lib::conflicts::MaterializedTreeValue; @@ -863,15 +863,10 @@ fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result { - let mut data = vec![]; - materialize_merge_result(&contents, &mut data) - .expect("Failed to materialize conflict to in-memory buffer"); - Ok(FileContent { - is_binary: false, - contents: data, - }) - } + } => Ok(FileContent { + is_binary: false, + contents: materialize_merge_result_to_bytes(&contents).into(), + }), MaterializedTreeValue::OtherConflict { id } => Ok(FileContent { is_binary: false, contents: id.describe().into_bytes(), @@ -1181,12 +1176,9 @@ fn git_diff_part( } => { mode = if executable { "100755" } else { "100644" }; hash = DUMMY_HASH.to_owned(); - let mut data = vec![]; - materialize_merge_result(&contents, &mut data) - .expect("Failed to materialize conflict to in-memory buffer"); content = FileContent { is_binary: false, // TODO: are we sure this is never binary? - contents: data, + contents: materialize_merge_result_to_bytes(&contents).into(), }; } MaterializedTreeValue::OtherConflict { id } => { diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 160cabad0..ebd0e7bee 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -12,7 +12,7 @@ use jj_lib::backend::BackendResult; use jj_lib::backend::FileId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; -use jj_lib::conflicts::materialize_merge_result; +use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::materialize_tree_value; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::diff::Diff; @@ -211,9 +211,7 @@ fn read_file_contents( contents, executable: _, } => { - let mut buf = Vec::new(); - materialize_merge_result(&contents, &mut buf) - .expect("Failed to materialize conflict to in-memory buffer"); + let buf = materialize_merge_result_to_bytes(&contents).into(); // TODO: Render the ID somehow? let contents = buf_to_file_contents(None, buf); Ok(FileInfo { diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index 26e13b164..7476ac64d 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -12,7 +12,7 @@ use jj_lib::backend::FileId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::conflicts; -use jj_lib::conflicts::materialize_merge_result; +use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; @@ -157,13 +157,10 @@ pub fn run_mergetool_external( conflict: MergedTreeValue, tree: &MergedTree, ) -> Result { - let initial_output_content: Vec = if editor.merge_tool_edits_conflict_markers { - let mut materialized_conflict = vec![]; - materialize_merge_result(&content, &mut materialized_conflict) - .expect("Writing to an in-memory buffer should never fail"); - materialized_conflict + let initial_output_content = if editor.merge_tool_edits_conflict_markers { + materialize_merge_result_to_bytes(&content) } else { - vec![] + BString::default() }; assert_eq!(content.num_sides(), 2); let files: HashMap<&str, &[u8]> = maplit::hashmap! { diff --git a/lib/src/annotate.rs b/lib/src/annotate.rs index a238d3858..6b36fb676 100644 --- a/lib/src/annotate.rs +++ b/lib/src/annotate.rs @@ -32,7 +32,7 @@ use pollster::FutureExt; use crate::backend::BackendError; use crate::backend::CommitId; use crate::commit::Commit; -use crate::conflicts::materialize_merge_result; +use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; use crate::conflicts::MaterializedTreeValue; use crate::diff::Diff; @@ -358,16 +358,8 @@ fn get_file_contents( })?; Ok(file_contents) } - MaterializedTreeValue::FileConflict { id, contents, .. } => { - let mut materialized_conflict_buffer = Vec::new(); - materialize_merge_result(&contents, &mut materialized_conflict_buffer).map_err( - |io_err| BackendError::ReadFile { - path: path.to_owned(), - source: Box::new(io_err), - id: id.first().clone().unwrap(), - }, - )?; - Ok(materialized_conflict_buffer) + MaterializedTreeValue::FileConflict { contents, .. } => { + Ok(materialize_merge_result_to_bytes(&contents).into()) } _ => Ok(Vec::new()), } diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 205839f93..4d139e235 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -243,6 +243,19 @@ pub fn materialize_merge_result>( } } +pub fn materialize_merge_result_to_bytes>(single_hunk: &Merge) -> BString { + let merge_result = files::merge(single_hunk); + match merge_result { + MergeResult::Resolved(content) => content, + MergeResult::Conflict(hunks) => { + let mut output = Vec::new(); + materialize_conflict_hunks(&hunks, &mut output) + .expect("writing to an in-memory buffer should never fail"); + output.into() + } + } +} + fn materialize_conflict_hunks(hunks: &[Merge], output: &mut dyn Write) -> io::Result<()> { let num_conflicts = hunks .iter() diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 00389dbc4..10e50473b 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -42,7 +42,7 @@ use crate::backend::ChangeId; use crate::backend::CommitId; use crate::backend::MillisSinceEpoch; use crate::commit::Commit; -use crate::conflicts::materialize_merge_result; +use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; use crate::conflicts::MaterializedTreeValue; use crate::default_index::AsCompositeIndex; @@ -1346,10 +1346,7 @@ fn to_file_content(path: &RepoPath, value: MaterializedTreeValue) -> BackendResu MaterializedTreeValue::Symlink { id: _, target } => Ok(target.into_bytes()), MaterializedTreeValue::GitSubmodule(_) => Ok(vec![]), MaterializedTreeValue::FileConflict { contents, .. } => { - let mut content = vec![]; - materialize_merge_result(&contents, &mut content) - .expect("Failed to materialize conflict to in-memory buffer"); - Ok(content) + Ok(materialize_merge_result_to_bytes(&contents).into()) } MaterializedTreeValue::OtherConflict { .. } => Ok(vec![]), MaterializedTreeValue::Tree(id) => { diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 0b6ff39d0..9162fe7e3 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -61,7 +61,7 @@ use crate::backend::TreeId; use crate::backend::TreeValue; use crate::commit::Commit; use crate::conflicts; -use crate::conflicts::materialize_merge_result; +use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; use crate::conflicts::MaterializedTreeValue; use crate::file_util::check_symlink_support; @@ -1597,9 +1597,7 @@ impl TreeState { contents, executable, } => { - let mut data = vec![]; - materialize_merge_result(&contents, &mut data) - .expect("Failed to materialize conflict to in-memory buffer"); + let data = materialize_merge_result_to_bytes(&contents).into(); self.write_conflict(&disk_path, data, executable)? } MaterializedTreeValue::OtherConflict { id } => { diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 75066a94e..11bbf5ab7 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -15,7 +15,7 @@ use indoc::indoc; use jj_lib::backend::FileId; use jj_lib::conflicts::extract_as_single_hunk; -use jj_lib::conflicts::materialize_merge_result; +use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::parse_conflict; use jj_lib::conflicts::update_from_content; use jj_lib::merge::Merge; @@ -1063,10 +1063,8 @@ fn materialize_conflict_string( path: &RepoPath, conflict: &Merge>, ) -> String { - let mut result: Vec = vec![]; let contents = extract_as_single_hunk(conflict, store, path) .block_on() .unwrap(); - materialize_merge_result(&contents, &mut result).unwrap(); - String::from_utf8(result).unwrap() + String::from_utf8(materialize_merge_result_to_bytes(&contents).into()).unwrap() }