Skip to content

Commit

Permalink
copies: filter rename source entries by CopiesTreeDiffStream
Browse files Browse the repository at this point in the history
  • Loading branch information
yuja committed Aug 21, 2024
1 parent 48a2a76 commit 9a39501
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 61 deletions.
1 change: 0 additions & 1 deletion cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,6 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T
formatter,
store,
tree_diff,
&Default::default(), // TODO: real copy tracking
path_converter,
&options,
)
Expand Down
33 changes: 2 additions & 31 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,7 @@ impl<'a> DiffRenderer<'a> {
DiffFormat::ColorWords(options) => {
let tree_diff =
from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
show_color_words_diff(
formatter,
store,
tree_diff,
copy_records,
path_converter,
options,
)?;
show_color_words_diff(formatter, store, tree_diff, path_converter, options)?;
}
DiffFormat::Tool(tool) => {
match tool.diff_invocation_mode {
Expand All @@ -348,7 +341,6 @@ impl<'a> DiffRenderer<'a> {
store,
tree_diff,
path_converter,
copy_records,
tool,
)
}
Expand Down Expand Up @@ -759,7 +751,6 @@ pub fn show_color_words_diff(
formatter: &mut dyn Formatter,
store: &Store,
tree_diff: BoxStream<CopiesTreeDiffEntry>,
copy_records: &CopyRecords,
path_converter: &RepoPathUiConverter,
options: &ColorWordsOptions,
) -> Result<(), DiffRenderError> {
Expand All @@ -774,9 +765,6 @@ pub fn show_color_words_diff(
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) = diff?;
if right_value.is_absent() && copy_records.has_source(&left_path) {
continue;
}

match (&left_value, &right_value) {
(MaterializedTreeValue::AccessDenied(source), _) => {
Expand Down Expand Up @@ -909,7 +897,6 @@ pub fn show_file_by_file_diff(
store: &Store,
tree_diff: BoxStream<CopiesTreeDiffEntry>,
path_converter: &RepoPathUiConverter,
copy_records: &CopyRecords,
tool: &ExternalMergeTool,
) -> Result<(), DiffRenderError> {
fn create_file(
Expand All @@ -936,10 +923,6 @@ pub fn show_file_by_file_diff(
}) = diff_stream.next().await
{
let (left_value, right_value) = diff?;
if right_value.is_absent() && copy_records.has_source(&left_path) {
continue;
}

let left_ui_path = path_converter.format_file_path(&left_path);
let right_ui_path = path_converter.format_file_path(&right_path);

Expand Down Expand Up @@ -1290,11 +1273,6 @@ pub fn show_git_diff(
let left_part = git_diff_part(&left_path, left_value)?;
let right_part = git_diff_part(&right_path, right_value)?;

// Skip the "delete" entry when there is a rename.
if right_part.mode.is_none() && copy_records.has_source(&left_path) {
continue;
}

formatter.with_label("file_header", |formatter| {
writeln!(
formatter,
Expand Down Expand Up @@ -1405,11 +1383,7 @@ pub fn show_diff_summary(
match (before.is_present(), after.is_present()) {
(true, true) => writeln!(formatter.labeled("modified"), "M {path}")?,
(false, true) => writeln!(formatter.labeled("added"), "A {path}")?,
(true, false) => {
if !copy_records.has_source(&before_path) {
writeln!(formatter.labeled("removed"), "D {path}")?;
}
}
(true, false) => writeln!(formatter.labeled("removed"), "D {path}")?,
(false, false) => unreachable!(),
}
}
Expand Down Expand Up @@ -1565,9 +1539,6 @@ pub fn show_types(
}) = tree_diff.next().await
{
let (before, after) = diff?;
if after.is_absent() && copy_records.has_source(&source) {
continue;
}
writeln!(
formatter.labeled("modified"),
"{}{} {}",
Expand Down
41 changes: 24 additions & 17 deletions lib/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,26 +121,33 @@ impl Stream for CopiesTreeDiffStream<'_> {
type Item = CopiesTreeDiffEntry;

fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let Some(diff_entry) = ready!(self.inner.as_mut().poll_next(cx)) else {
return Poll::Ready(None);
};
while let Some(diff_entry) = ready!(self.inner.as_mut().poll_next(cx)) {
let Some(CopyRecord { source, .. }) = self.copy_records.for_target(&diff_entry.path)
else {
let target_deleted =
matches!(&diff_entry.value, Ok((_, target_value)) if target_value.is_absent());
if target_deleted && self.copy_records.has_source(&diff_entry.path) {
// Skip the "delete" entry when there is a rename.
continue;
}
return Poll::Ready(Some(CopiesTreeDiffEntry {
source: diff_entry.path.clone(),
target: diff_entry.path,
value: diff_entry.value,
}));
};

let Some(CopyRecord { source, .. }) = self.copy_records.for_target(&diff_entry.path) else {
return Poll::Ready(Some(CopiesTreeDiffEntry {
source: diff_entry.path.clone(),
source: source.clone(),
target: diff_entry.path,
value: diff_entry.value,
value: diff_entry.value.and_then(|(_, target_value)| {
self.source_tree
.path_value(source)
.map(|source_value| (source_value, target_value))
}),
}));
};

Poll::Ready(Some(CopiesTreeDiffEntry {
source: source.clone(),
target: diff_entry.path,
value: diff_entry.value.and_then(|(_, target_value)| {
self.source_tree
.path_value(source)
.map(|source_value| (source_value, target_value))
}),
}))
}

Poll::Ready(None)
}
}
13 changes: 1 addition & 12 deletions lib/tests/test_merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ fn test_diff_copy_tracing() {
.map(|diff| (diff.source, diff.target, diff.value.unwrap()))
.collect()
.block_on();
assert_eq!(diff.len(), 4);
assert_eq!(diff.len(), 3);
assert_eq!(
diff[0].clone(),
(
Expand All @@ -880,17 +880,6 @@ fn test_diff_copy_tracing() {
);
assert_eq!(
diff[2].clone(),
(
removed_path.to_owned(),
removed_path.to_owned(),
(
Merge::resolved(before.path_value(removed_path).unwrap()),
Merge::absent()
),
)
);
assert_eq!(
diff[3].clone(),
(
removed_path.to_owned(),
added_path.to_owned(),
Expand Down

0 comments on commit 9a39501

Please sign in to comment.