diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 1ff7f2a1de..f381e71383 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -772,14 +772,11 @@ pub fn show_color_words_diff( ) -> Result<(), DiffRenderError> { let mut diff_stream = materialized_diff_stream(store, tree_diff); async { - while let Some(MaterializedTreeDiffEntry { - source: left_path, - target: right_path, - values, - }) = diff_stream.next().await - { - let left_ui_path = path_converter.format_file_path(&left_path); - let right_ui_path = path_converter.format_file_path(&right_path); + while let Some(MaterializedTreeDiffEntry { path, values }) = diff_stream.next().await { + let left_path = path.source(); + let right_path = path.target(); + let left_ui_path = path_converter.format_file_path(left_path); + let right_ui_path = path_converter.format_file_path(right_path); let (left_value, right_value) = values?; match (&left_value, &right_value) { @@ -807,7 +804,7 @@ pub fn show_color_words_diff( formatter.labeled("header"), "Added {description} {right_ui_path}:" )?; - let right_content = diff_content(&right_path, right_value)?; + let right_content = diff_content(right_path, right_value)?; if right_content.is_empty() { writeln!(formatter.labeled("empty"), " (empty)")?; } else if right_content.is_binary { @@ -863,8 +860,8 @@ pub fn show_color_words_diff( ) } }; - let left_content = diff_content(&left_path, left_value)?; - let right_content = diff_content(&right_path, right_value)?; + let left_content = diff_content(left_path, left_value)?; + let right_content = diff_content(right_path, right_value)?; if left_path == right_path { writeln!( formatter.labeled("header"), @@ -892,7 +889,7 @@ pub fn show_color_words_diff( formatter.labeled("header"), "Removed {description} {right_ui_path}:" )?; - let left_content = diff_content(&left_path, left_value)?; + let left_content = diff_content(left_path, left_value)?; if left_content.is_empty() { writeln!(formatter.labeled("empty"), " (empty)")?; } else if left_content.is_binary { @@ -932,15 +929,12 @@ pub fn show_file_by_file_diff( let right_wc_dir = temp_dir.path().join("right"); let mut diff_stream = materialized_diff_stream(store, tree_diff); async { - while let Some(MaterializedTreeDiffEntry { - source: left_path, - target: right_path, - values, - }) = diff_stream.next().await - { + while let Some(MaterializedTreeDiffEntry { path, values }) = diff_stream.next().await { let (left_value, right_value) = values?; - let left_ui_path = path_converter.format_file_path(&left_path); - let right_ui_path = path_converter.format_file_path(&right_path); + let left_path = path.source(); + let right_path = path.target(); + let left_ui_path = path_converter.format_file_path(left_path); + let right_ui_path = path_converter.format_file_path(right_path); match (&left_value, &right_value) { (_, MaterializedTreeValue::AccessDenied(source)) => { @@ -961,8 +955,8 @@ pub fn show_file_by_file_diff( } _ => {} } - let left_path = create_file(&left_path, &left_wc_dir, left_value)?; - let right_path = create_file(&right_path, &right_wc_dir, right_value)?; + let left_path = create_file(left_path, &left_wc_dir, left_value)?; + let right_path = create_file(right_path, &right_wc_dir, right_value)?; invoke_external_diff( ui, @@ -1276,18 +1270,15 @@ pub fn show_git_diff( let mut diff_stream = materialized_diff_stream(store, tree_diff); async { - while let Some(MaterializedTreeDiffEntry { - source: left_path, - target: right_path, - values, - }) = diff_stream.next().await - { + while let Some(MaterializedTreeDiffEntry { path, values }) = diff_stream.next().await { + let left_path = path.source(); + let right_path = path.target(); let left_path_string = left_path.as_internal_file_string(); let right_path_string = right_path.as_internal_file_string(); let (left_value, right_value) = values?; - let left_part = git_diff_part(&left_path, left_value)?; - let right_part = git_diff_part(&right_path, right_value)?; + let left_part = git_diff_part(left_path, left_value)?; + let right_part = git_diff_part(right_path, right_value)?; formatter.with_label("file_header", |formatter| { writeln!( @@ -1307,7 +1298,7 @@ pub fn show_git_diff( } (Some(left_mode), Some(right_mode)) => { if left_path != right_path { - let operation = if to_tree.path_value(&left_path)?.is_absent() { + let operation = if to_tree.path_value(left_path)?.is_absent() { "rename" } else { "copy" @@ -1380,22 +1371,19 @@ pub fn show_diff_summary( let mut tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records); async { - while let Some(CopiesTreeDiffEntry { - source: before_path, - target: after_path, - values, - }) = tree_diff.next().await - { + while let Some(CopiesTreeDiffEntry { path, values }) = tree_diff.next().await { let (before, after) = values?; + let before_path = path.source(); + let after_path = path.target(); if before_path != after_path { - let path = path_converter.format_copied_path(&before_path, &after_path); - if to_tree.path_value(&before_path).unwrap().is_absent() { + let path = path_converter.format_copied_path(before_path, after_path); + if to_tree.path_value(before_path).unwrap().is_absent() { writeln!(formatter.labeled("renamed"), "R {path}")? } else { writeln!(formatter.labeled("copied"), "C {path}")? } } else { - let path = path_converter.format_file_path(&after_path); + let path = path_converter.format_file_path(after_path); match (before.is_present(), after.is_present()) { (true, true) => writeln!(formatter.labeled("modified"), "M {path}")?, (false, true) => writeln!(formatter.labeled("added"), "A {path}")?, @@ -1459,22 +1447,19 @@ pub fn show_diff_stat( let mut diff_stream = materialized_diff_stream(store, tree_diff); async { - while let Some(MaterializedTreeDiffEntry { - source: left_path, - target: right_path, - values, - }) = diff_stream.next().await - { + while let Some(MaterializedTreeDiffEntry { path, values }) = diff_stream.next().await { let (left, right) = values?; - let left_content = diff_content(&left_path, left)?; - let right_content = diff_content(&right_path, right)?; + let left_path = path.source(); + let right_path = path.target(); + let left_content = diff_content(left_path, left)?; + let right_content = diff_content(right_path, right)?; - let left_ui_path = path_converter.format_file_path(&left_path); + let left_ui_path = path_converter.format_file_path(left_path); let path = if left_path == right_path { left_ui_path } else { unresolved_renames.insert(left_ui_path); - path_converter.format_copied_path(&left_path, &right_path) + path_converter.format_copied_path(left_path, right_path) }; max_path_width = max(max_path_width, path.width()); let stat = get_diff_stat(path, &left_content, &right_content); @@ -1548,19 +1533,14 @@ pub fn show_types( let mut tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records); async { - while let Some(CopiesTreeDiffEntry { - source, - target, - values, - }) = tree_diff.next().await - { + while let Some(CopiesTreeDiffEntry { path, values }) = tree_diff.next().await { let (before, after) = values?; writeln!( formatter.labeled("modified"), "{}{} {}", diff_summary_char(&before), diff_summary_char(&after), - path_converter.format_copied_path(&source, &target) + path_converter.format_copied_path(path.source(), path.target()) )?; } Ok(()) @@ -1587,11 +1567,12 @@ pub fn show_names( path_converter: &RepoPathUiConverter, ) -> io::Result<()> { async { - while let Some(CopiesTreeDiffEntry { - target: repo_path, .. - }) = tree_diff.next().await - { - writeln!(formatter, "{}", path_converter.format_file_path(&repo_path))?; + while let Some(CopiesTreeDiffEntry { path, .. }) = tree_diff.next().await { + writeln!( + formatter, + "{}", + path_converter.format_file_path(path.target()) + )?; } Ok(()) } diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 3805b8fd4d..e7fc416380 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -35,6 +35,7 @@ use crate::backend::SymlinkId; use crate::backend::TreeId; use crate::backend::TreeValue; use crate::copies::CopiesTreeDiffEntry; +use crate::copies::CopiesTreeDiffEntryPath; use crate::diff::Diff; use crate::diff::DiffHunk; use crate::files; @@ -44,7 +45,6 @@ use crate::merge::Merge; use crate::merge::MergeBuilder; use crate::merge::MergedTreeValue; use crate::repo_path::RepoPath; -use crate::repo_path::RepoPathBuf; use crate::store::Store; const CONFLICT_START_LINE: &[u8] = b"<<<<<<<"; @@ -343,8 +343,7 @@ fn diff_size(hunks: &[DiffHunk]) -> usize { } pub struct MaterializedTreeDiffEntry { - pub source: RepoPathBuf, - pub target: RepoPathBuf, + pub path: CopiesTreeDiffEntryPath, pub values: BackendResult<(MaterializedTreeValue, MaterializedTreeValue)>, } @@ -353,31 +352,20 @@ pub fn materialized_diff_stream<'a>( tree_diff: BoxStream<'a, CopiesTreeDiffEntry>, ) -> impl Stream + 'a { tree_diff - .map( - |CopiesTreeDiffEntry { - source, - target, - values, - }| async { - match values { - Err(err) => MaterializedTreeDiffEntry { - source, - target, - values: Err(err), - }, - Ok((before, after)) => { - let before_future = materialize_tree_value(store, &source, before); - let after_future = materialize_tree_value(store, &target, after); - let values = try_join!(before_future, after_future); - MaterializedTreeDiffEntry { - source, - target, - values, - } - } + .map(|CopiesTreeDiffEntry { path, values }| async { + match values { + Err(err) => MaterializedTreeDiffEntry { + path, + values: Err(err), + }, + Ok((before, after)) => { + let before_future = materialize_tree_value(store, path.source(), before); + let after_future = materialize_tree_value(store, path.target(), after); + let values = try_join!(before_future, after_future); + MaterializedTreeDiffEntry { path, values } } - }, - ) + } + }) .buffered((store.concurrency() / 2).max(1)) } diff --git a/lib/src/copies.rs b/lib/src/copies.rs index 78160a2e35..01b6732835 100644 --- a/lib/src/copies.rs +++ b/lib/src/copies.rs @@ -92,12 +92,31 @@ impl CopyRecords { /// A `TreeDiffEntry` with copy information. pub struct CopiesTreeDiffEntry { + /// The path. + pub path: CopiesTreeDiffEntryPath, + /// The resolved tree values if available. + pub values: BackendResult<(MergedTreeValue, MergedTreeValue)>, +} + +/// Path of `CopiesTreeDiffEntry`. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CopiesTreeDiffEntryPath { /// The source path. pub source: RepoPathBuf, /// The target path. pub target: RepoPathBuf, - /// The resolved tree values if available. - pub values: BackendResult<(MergedTreeValue, MergedTreeValue)>, +} + +impl CopiesTreeDiffEntryPath { + /// The source path. + pub fn source(&self) -> &RepoPath { + &self.source + } + + /// The target path. + pub fn target(&self) -> &RepoPath { + &self.target + } } /// Wraps a `TreeDiffStream`, adding support for copies and renames. @@ -136,15 +155,19 @@ impl Stream for CopiesTreeDiffStream<'_> { continue; } return Poll::Ready(Some(CopiesTreeDiffEntry { - source: diff_entry.path.clone(), - target: diff_entry.path, + path: CopiesTreeDiffEntryPath { + source: diff_entry.path.clone(), + target: diff_entry.path, + }, values: diff_entry.values, })); }; return Poll::Ready(Some(CopiesTreeDiffEntry { - source: source.clone(), - target: diff_entry.path, + path: CopiesTreeDiffEntryPath { + source: source.clone(), + target: diff_entry.path, + }, values: diff_entry.values.and_then(|(_, target_value)| { self.source_tree .path_value(source) diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index 0af49791e8..7b2aa23c27 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -19,6 +19,7 @@ use jj_lib::backend::CopyRecord; use jj_lib::backend::FileId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; +use jj_lib::copies::CopiesTreeDiffEntryPath; use jj_lib::copies::CopyRecords; use jj_lib::files::MergeResult; use jj_lib::matchers::EverythingMatcher; @@ -867,15 +868,17 @@ fn test_diff_copy_tracing() { let diff: Vec<_> = before_merged .diff_stream_with_copies(&after_merged, &EverythingMatcher, ©_records) - .map(|diff| (diff.source, diff.target, diff.values.unwrap())) + .map(|diff| (diff.path, diff.values.unwrap())) .collect() .block_on(); assert_eq!(diff.len(), 3); assert_eq!( diff[0].clone(), ( - modified_path.to_owned(), - modified_path.to_owned(), + CopiesTreeDiffEntryPath { + source: modified_path.to_owned(), + target: modified_path.to_owned(), + }, ( Merge::resolved(before.path_value(modified_path).unwrap()), Merge::resolved(after.path_value(modified_path).unwrap()) @@ -885,8 +888,10 @@ fn test_diff_copy_tracing() { assert_eq!( diff[1].clone(), ( - modified_path.to_owned(), - copied_path.to_owned(), + CopiesTreeDiffEntryPath { + source: modified_path.to_owned(), + target: copied_path.to_owned(), + }, ( Merge::resolved(before.path_value(modified_path).unwrap()), Merge::resolved(after.path_value(copied_path).unwrap()), @@ -896,8 +901,10 @@ fn test_diff_copy_tracing() { assert_eq!( diff[2].clone(), ( - removed_path.to_owned(), - added_path.to_owned(), + CopiesTreeDiffEntryPath { + source: removed_path.to_owned(), + target: added_path.to_owned(), + }, ( Merge::resolved(before.path_value(removed_path).unwrap()), Merge::resolved(after.path_value(added_path).unwrap())