diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 10a7d7070..62b780d3c 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::path::Path; use std::sync::Arc; +use bstr::BString; use futures::StreamExt; use futures::TryFutureExt; use futures::TryStreamExt; @@ -16,7 +17,6 @@ use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::diff::Diff; use jj_lib::diff::DiffHunk; use jj_lib::files; -use jj_lib::files::ContentHunk; use jj_lib::files::MergeResult; use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; @@ -534,8 +534,8 @@ fn make_merge_sections( ) -> Result>, BuiltinToolError> { let mut sections = Vec::new(); match merge_result { - MergeResult::Resolved(ContentHunk(buf)) => { - let contents = buf_to_file_contents(None, buf); + MergeResult::Resolved(buf) => { + let contents = buf_to_file_contents(None, buf.into()); let section = match contents { FileContents::Absent => None, FileContents::Text { @@ -561,7 +561,7 @@ fn make_merge_sections( MergeResult::Conflict(hunks) => { for hunk in hunks { let section = match hunk.into_resolved() { - Ok(ContentHunk(contents)) => { + Ok(contents) => { let contents = std::str::from_utf8(&contents).map_err(|err| { BuiltinToolError::DecodeUtf8 { source: err, @@ -587,7 +587,6 @@ fn make_merge_sections( .cycle(), ) .map(|(contents, change_type)| -> Result<_, BuiltinToolError> { - let ContentHunk(contents) = contents; let contents = std::str::from_utf8(contents).map_err(|err| { BuiltinToolError::DecodeUtf8 { source: err, @@ -613,7 +612,7 @@ fn make_merge_sections( pub fn edit_merge_builtin( tree: &MergedTree, path: &RepoPath, - content: Merge, + content: Merge, ) -> Result { let merge_result = files::merge(&content); let sections = make_merge_sections(merge_result)?; diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index aea5a2f8e..1800d692f 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -6,6 +6,7 @@ use std::process::ExitStatus; use std::process::Stdio; use std::sync::Arc; +use bstr::BString; use itertools::Itertools; use jj_lib::backend::FileId; use jj_lib::backend::MergedTreeId; @@ -151,7 +152,7 @@ pub enum ExternalToolError { pub fn run_mergetool_external( editor: &ExternalMergeTool, file_merge: Merge>, - content: Merge, + content: Merge, repo_path: &RepoPath, conflict: MergedTreeValue, tree: &MergedTree, @@ -166,9 +167,9 @@ pub fn run_mergetool_external( }; assert_eq!(content.num_sides(), 2); let files: HashMap<&str, &[u8]> = maplit::hashmap! { - "base" => content.get_remove(0).unwrap().0.as_slice(), - "left" => content.get_add(0).unwrap().0.as_slice(), - "right" => content.get_add(1).unwrap().0.as_slice(), + "base" => content.get_remove(0).unwrap().as_slice(), + "left" => content.get_add(0).unwrap().as_slice(), + "right" => content.get_add(1).unwrap().as_slice(), "output" => initial_output_content.as_slice(), }; diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index e7fc41638..fe4484a5e 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -18,6 +18,7 @@ use std::io::Read; use std::io::Write; use std::iter::zip; +use bstr::BString; use futures::stream::BoxStream; use futures::try_join; use futures::Stream; @@ -39,7 +40,6 @@ use crate::copies::CopiesTreeDiffEntryPath; use crate::diff::Diff; use crate::diff::DiffHunk; use crate::files; -use crate::files::ContentHunk; use crate::files::MergeResult; use crate::merge::Merge; use crate::merge::MergeBuilder; @@ -98,7 +98,7 @@ async fn get_file_contents( store: &Store, path: &RepoPath, term: &Option, -) -> BackendResult { +) -> BackendResult { match term { Some(id) => { let mut content = vec![]; @@ -111,11 +111,11 @@ async fn get_file_contents( id: id.clone(), source: err.into(), })?; - Ok(ContentHunk(content)) + Ok(BString::new(content)) } // If the conflict had removed the file on one side, we pretend that the file // was empty there. - None => Ok(ContentHunk(vec![])), + None => Ok(BString::new(vec![])), } } @@ -123,8 +123,8 @@ pub async fn extract_as_single_hunk( merge: &Merge>, store: &Store, path: &RepoPath, -) -> BackendResult> { - let builder: MergeBuilder = futures::stream::iter(merge.iter()) +) -> BackendResult> { + let builder: MergeBuilder = futures::stream::iter(merge.iter()) .then(|term| get_file_contents(store, path, term)) .try_collect() .await?; @@ -232,13 +232,13 @@ async fn materialize_tree_value_no_access_denied( } pub fn materialize_merge_result( - single_hunk: &Merge, + single_hunk: &Merge, output: &mut dyn Write, ) -> std::io::Result<()> { let merge_result = files::merge(single_hunk); match merge_result { MergeResult::Resolved(content) => { - output.write_all(&content.0)?; + output.write_all(&content)?; } MergeResult::Conflict(hunks) => { let num_conflicts = hunks @@ -248,7 +248,7 @@ pub fn materialize_merge_result( let mut conflict_index = 0; for hunk in hunks { if let Some(content) = hunk.as_resolved() { - output.write_all(&content.0)?; + output.write_all(content)?; } else { conflict_index += 1; output.write_all(CONFLICT_START_LINE)?; @@ -272,15 +272,15 @@ pub fn materialize_merge_result( // terms as snapshots. output.write_all(CONFLICT_MINUS_LINE)?; output.write_all(format!(" Contents of {base_str}\n").as_bytes())?; - output.write_all(&left.0)?; + output.write_all(left)?; continue; }; - let diff1 = Diff::by_line([&left.0, &right1.0]).hunks().collect_vec(); + let diff1 = Diff::by_line([&left, &right1]).hunks().collect_vec(); // Check if the diff against the next positive term is better. Since // we want to preserve the order of the terms, we don't match against // any later positive terms. if let Some(right2) = hunk.get_add(add_index + 1) { - let diff2 = Diff::by_line([&left.0, &right2.0]).hunks().collect_vec(); + let diff2 = Diff::by_line([&left, &right2]).hunks().collect_vec(); if diff_size(&diff2) < diff_size(&diff1) { // If the next positive term is a better match, emit // the current positive term as a snapshot and the next @@ -289,7 +289,7 @@ pub fn materialize_merge_result( output.write_all( format!(" Contents of side #{}\n", add_index + 1).as_bytes(), )?; - output.write_all(&right1.0)?; + output.write_all(right1)?; output.write_all(CONFLICT_DIFF_LINE)?; output.write_all( format!( @@ -319,7 +319,7 @@ pub fn materialize_merge_result( output.write_all( format!(" Contents of side #{}\n", add_index + 1).as_bytes(), )?; - output.write_all(&slice.0)?; + output.write_all(slice)?; } output.write_all(CONFLICT_END_LINE)?; output.write_all( @@ -375,7 +375,7 @@ pub fn materialized_diff_stream<'a>( /// invalid if they don't have the expected arity. // TODO: "parse" is not usually the opposite of "materialize", so maybe we // should rename them to "serialize" and "deserialize"? -pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option>> { +pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option>> { if input.is_empty() { return None; } @@ -395,7 +395,7 @@ pub fn parse_conflict(input: &[u8], num_sides: usize) -> Option Option Merge { +fn parse_conflict_hunk(input: &[u8]) -> Merge { enum State { Diff, Minus, @@ -433,18 +431,18 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge { match line[0] { CONFLICT_DIFF_LINE_CHAR => { state = State::Diff; - removes.push(ContentHunk(vec![])); - adds.push(ContentHunk(vec![])); + removes.push(BString::new(vec![])); + adds.push(BString::new(vec![])); continue; } CONFLICT_MINUS_LINE_CHAR => { state = State::Minus; - removes.push(ContentHunk(vec![])); + removes.push(BString::new(vec![])); continue; } CONFLICT_PLUS_LINE_CHAR => { state = State::Plus; - adds.push(ContentHunk(vec![])); + adds.push(BString::new(vec![])); continue; } _ => {} @@ -453,26 +451,26 @@ fn parse_conflict_hunk(input: &[u8]) -> Merge { match state { State::Diff => { if let Some(rest) = line.strip_prefix(b"-") { - removes.last_mut().unwrap().0.extend_from_slice(rest); + removes.last_mut().unwrap().extend_from_slice(rest); } else if let Some(rest) = line.strip_prefix(b"+") { - adds.last_mut().unwrap().0.extend_from_slice(rest); + adds.last_mut().unwrap().extend_from_slice(rest); } else if let Some(rest) = line.strip_prefix(b" ") { - removes.last_mut().unwrap().0.extend_from_slice(rest); - adds.last_mut().unwrap().0.extend_from_slice(rest); + removes.last_mut().unwrap().extend_from_slice(rest); + adds.last_mut().unwrap().extend_from_slice(rest); } else { // Doesn't look like a conflict - return Merge::resolved(ContentHunk(vec![])); + return Merge::resolved(BString::new(vec![])); } } State::Minus => { - removes.last_mut().unwrap().0.extend_from_slice(line); + removes.last_mut().unwrap().extend_from_slice(line); } State::Plus => { - adds.last_mut().unwrap().0.extend_from_slice(line); + adds.last_mut().unwrap().extend_from_slice(line); } State::Unknown => { // Doesn't look like a conflict - return Merge::resolved(ContentHunk(vec![])); + return Merge::resolved(BString::new(vec![])); } } } @@ -526,11 +524,11 @@ pub async fn update_from_content( for hunk in hunks { if let Some(slice) = hunk.as_resolved() { for content in contents.iter_mut() { - content.extend_from_slice(&slice.0); + content.extend_from_slice(slice); } } else { for (content, slice) in zip(contents.iter_mut(), hunk.into_iter()) { - content.extend(slice.0); + content.extend(Vec::from(slice)); } } } diff --git a/lib/src/files.rs b/lib/src/files.rs index 605de2e7f..f4125e507 100644 --- a/lib/src/files.rs +++ b/lib/src/files.rs @@ -16,13 +16,11 @@ use std::borrow::Borrow; use std::collections::VecDeque; -use std::fmt::Debug; -use std::fmt::Error; -use std::fmt::Formatter; use std::iter; use std::mem; use bstr::BStr; +use bstr::BString; use crate::diff::Diff; use crate::diff::DiffHunk; @@ -184,26 +182,10 @@ where } } -// TODO: Maybe ContentHunk can be replaced with BString? -#[derive(PartialEq, Eq, Clone)] -pub struct ContentHunk(pub Vec); - -impl AsRef<[u8]> for ContentHunk { - fn as_ref(&self) -> &[u8] { - self.0.as_ref() - } -} - -impl Debug for ContentHunk { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { - String::from_utf8_lossy(&self.0).fmt(f) - } -} - #[derive(PartialEq, Eq, Clone, Debug)] pub enum MergeResult { - Resolved(ContentHunk), - Conflict(Vec>), + Resolved(BString), + Conflict(Vec>), } pub fn merge>(slices: &Merge) -> MergeResult { @@ -216,28 +198,24 @@ pub fn merge>(slices: &Merge) -> MergeResult { } fn merge_hunks(diff: &Diff, num_diffs: usize) -> MergeResult { - let mut resolved_hunk = ContentHunk(vec![]); - let mut merge_hunks: Vec> = vec![]; + let mut resolved_hunk = BString::new(vec![]); + let mut merge_hunks: Vec> = vec![]; for diff_hunk in diff.hunks() { match diff_hunk { DiffHunk::Matching(content) => { - resolved_hunk.0.extend_from_slice(content); + resolved_hunk.extend_from_slice(content); } DiffHunk::Different(parts) => { if let Some(resolved) = trivial_merge(&parts[..num_diffs], &parts[num_diffs..]) { - resolved_hunk.0.extend_from_slice(resolved); + resolved_hunk.extend_from_slice(resolved); } else { - if !resolved_hunk.0.is_empty() { + if !resolved_hunk.is_empty() { merge_hunks.push(Merge::resolved(resolved_hunk)); - resolved_hunk = ContentHunk(vec![]); + resolved_hunk = BString::new(vec![]); } merge_hunks.push(Merge::from_removes_adds( - parts[..num_diffs] - .iter() - .map(|part| ContentHunk(part.to_vec())), - parts[num_diffs..] - .iter() - .map(|part| ContentHunk(part.to_vec())), + parts[..num_diffs].iter().copied().map(BString::from), + parts[num_diffs..].iter().copied().map(BString::from), )); } } @@ -247,7 +225,7 @@ fn merge_hunks(diff: &Diff, num_diffs: usize) -> MergeResult { if merge_hunks.is_empty() { MergeResult::Resolved(resolved_hunk) } else { - if !resolved_hunk.0.is_empty() { + if !resolved_hunk.is_empty() { merge_hunks.push(Merge::resolved(resolved_hunk)); } MergeResult::Conflict(merge_hunks) @@ -258,8 +236,8 @@ fn merge_hunks(diff: &Diff, num_diffs: usize) -> MergeResult { mod tests { use super::*; - fn hunk(data: &[u8]) -> ContentHunk { - ContentHunk(data.to_vec()) + fn hunk(data: &[u8]) -> BString { + data.into() } fn merge(removes: &[&[u8]], adds: &[&[u8]]) -> MergeResult { diff --git a/lib/src/tree.rs b/lib/src/tree.rs index e66a7c323..68d7b7424 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -471,7 +471,7 @@ pub fn try_resolve_file_conflict( let merge_result = files::merge(&contents); match merge_result { MergeResult::Resolved(merged_content) => { - let id = store.write_file(filename, &mut merged_content.0.as_slice())?; + let id = store.write_file(filename, &mut merged_content.as_slice())?; Ok(Some(TreeValue::File { id, executable })) } MergeResult::Conflict(_) => Ok(None),