diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 599fbf448..ac89b9451 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -17,14 +17,13 @@ use std::fmt::{Debug, Error, Formatter}; use std::pin::Pin; use std::sync::Arc; -use crate::matchers::AlwaysMatcher; use crate::repo_path::{ DirRepoPath, DirRepoPathComponent, FileRepoPath, RepoPath, RepoPathComponent, RepoPathJoin, }; use crate::store; use crate::store::{ConflictId, TreeEntriesNonRecursiveIter, TreeEntry, TreeId, TreeValue}; use crate::store_wrapper::StoreWrapper; -use crate::trees::{recursive_tree_diff, TreeValueDiff}; +use crate::trees::{recursive_tree_diff, Diff, TreeDiffIterator}; #[derive(Clone)] pub struct Tree { @@ -165,19 +164,21 @@ impl Tree { } } - pub fn diff(&self, other: &Tree, callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff)) { - recursive_tree_diff(self.clone(), other.clone(), &AlwaysMatcher {}, callback); + pub fn diff<'a>(&'a self, other: &'a Tree) -> TreeDiffIterator { + recursive_tree_diff(self.clone(), other.clone()) } pub fn diff_summary(&self, other: &Tree) -> DiffSummary { let mut modified = vec![]; let mut added = vec![]; let mut removed = vec![]; - self.diff(other, &mut |file, diff| match diff { - TreeValueDiff::Modified(_, _) => modified.push(file.clone()), - TreeValueDiff::Added(_) => added.push(file.clone()), - TreeValueDiff::Removed(_) => removed.push(file.clone()), - }); + for (file, diff) in self.diff(other) { + match diff { + Diff::Modified(_, _) => modified.push(file.clone()), + Diff::Added(_) => added.push(file.clone()), + Diff::Removed(_) => removed.push(file.clone()), + } + } modified.sort(); added.sort(); removed.sort(); diff --git a/lib/src/trees.rs b/lib/src/trees.rs index 3ff2cf0ec..69a70c744 100644 --- a/lib/src/trees.rs +++ b/lib/src/trees.rs @@ -14,10 +14,10 @@ use std::cmp::Ordering; use std::iter::Peekable; +use std::pin::Pin; use crate::files; use crate::files::MergeResult; -use crate::matchers::Matcher; use crate::repo_path::{ DirRepoPath, DirRepoPathComponent, FileRepoPath, FileRepoPathComponent, RepoPathJoin, }; @@ -124,91 +124,119 @@ fn diff_entries<'a>(tree1: &'a Tree, tree2: &'a Tree) -> TreeEntryDiffIterator<' TreeEntryDiffIterator::new(tree1, tree2) } -pub fn recursive_tree_diff( - root1: Tree, - root2: Tree, - matcher: &M, - callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff), -) where - M: Matcher, -{ - internal_recursive_tree_diff(vec![(DirRepoPath::root(), root1, root2)], matcher, callback) +pub fn recursive_tree_diff(root1: Tree, root2: Tree) -> TreeDiffIterator { + TreeDiffIterator::new(DirRepoPath::root(), root1, root2) } -fn internal_recursive_tree_diff( - work: Vec<(DirRepoPath, Tree, Tree)>, - _matcher: &M, - callback: &mut impl FnMut(&FileRepoPath, TreeValueDiff), -) where - M: Matcher, -{ - let mut new_work = Vec::new(); - // Diffs for which to invoke the callback after having visited subtrees. This is - // used for making sure that when a directory gets replaced by a file, we - // call the callback for the addition of the file after we call the callback +pub struct TreeDiffIterator { + dir: DirRepoPath, + tree1: Pin>, + tree2: Pin>, + // Iterator over the diffs between tree1 and tree2 + entry_iterator: TreeEntryDiffIterator<'static>, + // This is used for making sure that when a directory gets replaced by a file, we + // yield the value for the addition of the file after we yield the values // for removing files in the directory. - let mut late_file_diffs = Vec::new(); - for (dir, tree1, tree2) in &work { - for (name, diff) in diff_entries(tree1, tree2) { - let file_path = dir.join(&FileRepoPathComponent::from(name.as_str())); - let subdir = DirRepoPathComponent::from(name.as_str()); - let subdir_path = dir.join(&subdir); - // TODO: simplify this mess - match diff { - TreeValueDiff::Modified(TreeValue::Tree(id_before), TreeValue::Tree(id_after)) => { - new_work.push(( - subdir_path, - tree1.known_sub_tree(&subdir, &id_before), - tree2.known_sub_tree(&subdir, &id_after), - )); - } - TreeValueDiff::Modified(TreeValue::Tree(id_before), file_after) => { - new_work.push(( - subdir_path.clone(), - tree1.known_sub_tree(&subdir, &id_before), - Tree::null(tree2.store().clone(), subdir_path), - )); - late_file_diffs.push((file_path, TreeValueDiff::Added(file_after))); - } - TreeValueDiff::Modified(file_before, TreeValue::Tree(id_after)) => { - new_work.push(( - subdir_path.clone(), - Tree::null(tree1.store().clone(), subdir_path), - tree2.known_sub_tree(&subdir, &id_after), - )); - callback(&file_path, TreeValueDiff::Removed(file_before)); - } - TreeValueDiff::Modified(_, _) => { - callback(&file_path, diff); - } - TreeValueDiff::Added(TreeValue::Tree(id_after)) => { - new_work.push(( - subdir_path.clone(), - Tree::null(tree1.store().clone(), subdir_path), - tree2.known_sub_tree(&subdir, &id_after), - )); - } - TreeValueDiff::Added(_) => { - callback(&file_path, diff); - } - TreeValueDiff::Removed(TreeValue::Tree(id_before)) => { - new_work.push(( - subdir_path.clone(), - tree1.known_sub_tree(&subdir, &id_before), - Tree::null(tree2.store().clone(), subdir_path), - )); - } - TreeValueDiff::Removed(_) => { - callback(&file_path, diff); - } - } + added_file: Option<(FileRepoPath, TreeValue)>, + // Iterator over the diffs of a subdirectory, if we're currently visiting one. + subdir_iterator: Option>, +} + +impl TreeDiffIterator { + fn new(dir: DirRepoPath, tree1: Tree, tree2: Tree) -> TreeDiffIterator { + let tree1 = Box::pin(tree1); + let tree2 = Box::pin(tree2); + let root_entry_iterator: TreeEntryDiffIterator = diff_entries(&tree1, &tree2); + let root_entry_iterator: TreeEntryDiffIterator<'static> = + unsafe { std::mem::transmute(root_entry_iterator) }; + Self { + dir, + tree1, + tree2, + entry_iterator: root_entry_iterator, + added_file: None, + subdir_iterator: None, } } - if !new_work.is_empty() { - internal_recursive_tree_diff(new_work, _matcher, callback) - } - for (file_path, diff) in late_file_diffs { - callback(&file_path, diff); +} + +impl Iterator for TreeDiffIterator { + type Item = (FileRepoPath, Diff); + + fn next(&mut self) -> Option { + loop { + // First return results from any subdirectory we're currently visiting. + if let Some(subdir_iterator) = &mut self.subdir_iterator { + if let Some(element) = subdir_iterator.next() { + return Some(element); + } + } + + if let Some((name, value)) = self.added_file.take() { + return Some((name, Diff::Added(value))); + } + + // Note: whenever we say "file" below, it may also be a symlink or a conflict. + if let Some((name, diff)) = self.entry_iterator.next() { + let file_path = self.dir.join(&FileRepoPathComponent::from(name.as_str())); + let subdir = DirRepoPathComponent::from(name.as_str()); + let subdir_path = self.dir.join(&subdir); + // TODO: simplify this mess + match diff { + Diff::Modified(TreeValue::Tree(id_before), TreeValue::Tree(id_after)) => { + self.subdir_iterator = Some(Box::new(TreeDiffIterator::new( + subdir_path, + self.tree1.known_sub_tree(&subdir, &id_before), + self.tree2.known_sub_tree(&subdir, &id_after), + ))); + } + Diff::Modified(TreeValue::Tree(id_before), file_after) => { + self.subdir_iterator = Some(Box::new(TreeDiffIterator::new( + subdir_path.clone(), + self.tree1.known_sub_tree(&subdir, &id_before), + Tree::null(self.tree2.store().clone(), subdir_path), + ))); + self.added_file = Some((file_path, file_after.clone())); + } + Diff::Modified(file_before, TreeValue::Tree(id_after)) => { + self.subdir_iterator = Some(Box::new(TreeDiffIterator::new( + subdir_path.clone(), + Tree::null(self.tree1.store().clone(), subdir_path), + self.tree2.known_sub_tree(&subdir, &id_after), + ))); + return Some((file_path, Diff::Removed(file_before.clone()))); + } + Diff::Modified(file_before, file_after) => { + return Some(( + file_path, + Diff::Modified(file_before.clone(), file_after.clone()), + )); + } + Diff::Added(TreeValue::Tree(id_after)) => { + self.subdir_iterator = Some(Box::new(TreeDiffIterator::new( + subdir_path.clone(), + Tree::null(self.tree1.store().clone(), subdir_path), + self.tree2.known_sub_tree(&subdir, &id_after), + ))); + } + Diff::Added(value_after) => { + return Some((file_path, Diff::Added(value_after.clone()))); + } + Diff::Removed(TreeValue::Tree(id_before)) => { + self.subdir_iterator = Some(Box::new(TreeDiffIterator::new( + subdir_path.clone(), + self.tree1.known_sub_tree(&subdir, &id_before), + Tree::null(self.tree2.store().clone(), subdir_path), + ))); + } + Diff::Removed(value_before) => { + return Some((file_path, Diff::Removed(value_before.clone()))); + } + } + } else { + return None; + } + } } } diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index fb1e7eeac..ea2b988eb 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -42,7 +42,7 @@ use crate::repo_path::{ use crate::settings::UserSettings; use crate::store::{CommitId, FileId, MillisSinceEpoch, StoreError, SymlinkId, TreeId, TreeValue}; use crate::store_wrapper::StoreWrapper; -use crate::trees::TreeValueDiff; +use crate::trees::Diff; #[derive(Debug, PartialEq, Eq, Clone)] pub enum FileType { @@ -470,14 +470,14 @@ impl TreeState { removed_files: 0, }; - old_tree.diff(&new_tree, &mut |path, diff| { + for (path, diff) in old_tree.diff(&new_tree) { let disk_path = self .working_copy_path .join(PathBuf::from(path.to_internal_string())); // TODO: Check that the file has not changed before overwriting/removing it. match diff { - TreeValueDiff::Removed(_before) => { + Diff::Removed(_before) => { fs::remove_file(&disk_path).ok(); let mut parent_dir = disk_path.parent().unwrap(); loop { @@ -489,15 +489,15 @@ impl TreeState { self.file_states.remove(&path); stats.removed_files += 1; } - TreeValueDiff::Added(after) => { + Diff::Added(after) => { let file_state = match after { TreeValue::Normal { id, executable } => { - self.write_file(&disk_path, path, id, *executable) + self.write_file(&disk_path, &path, &id, executable) } - TreeValue::Symlink(id) => self.write_symlink(&disk_path, path, id), + TreeValue::Symlink(id) => self.write_symlink(&disk_path, &path, &id), TreeValue::GitSubmodule(_id) => { println!("ignoring git submodule at {:?}", path); - return; + continue; } TreeValue::Tree(_id) => { panic!("unexpected tree entry in diff at {:?}", path); @@ -512,7 +512,7 @@ impl TreeState { self.file_states.insert(path.clone(), file_state); stats.added_files += 1; } - TreeValueDiff::Modified(before, after) => { + Diff::Modified(before, after) => { fs::remove_file(&disk_path).ok(); let file_state = match (before, after) { ( @@ -524,19 +524,19 @@ impl TreeState { ) if id == old_id => { // Optimization for when only the executable bit changed assert_ne!(executable, old_executable); - self.set_executable(&disk_path, *executable); + self.set_executable(&disk_path, executable); let mut file_state = self.file_states.get(&path).unwrap().clone(); - file_state.mark_executable(*executable); + file_state.mark_executable(executable); file_state } (_, TreeValue::Normal { id, executable }) => { - self.write_file(&disk_path, path, id, *executable) + self.write_file(&disk_path, &path, &id, executable) } - (_, TreeValue::Symlink(id)) => self.write_symlink(&disk_path, path, id), + (_, TreeValue::Symlink(id)) => self.write_symlink(&disk_path, &path, &id), (_, TreeValue::GitSubmodule(_id)) => { println!("ignoring git submodule at {:?}", path); - self.file_states.remove(path); - return; + self.file_states.remove(&path); + continue; } (_, TreeValue::Tree(_id)) => { panic!("unexpected tree entry in diff at {:?}", path); @@ -553,7 +553,7 @@ impl TreeState { stats.updated_files += 1; } } - }); + } self.tree_id = tree_id; self.save(); Ok(stats) diff --git a/src/commands.rs b/src/commands.rs index df13f0d41..7771fc7cd 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -44,7 +44,7 @@ use jujube_lib::settings::UserSettings; use jujube_lib::store::{CommitId, StoreError, Timestamp, TreeValue}; use jujube_lib::store_wrapper::StoreWrapper; use jujube_lib::tree::Tree; -use jujube_lib::trees::TreeValueDiff; +use jujube_lib::trees::Diff; use jujube_lib::working_copy::{CheckoutStats, WorkingCopy}; use jujube_lib::{conflicts, files, git}; use pest::Parser; @@ -777,7 +777,6 @@ fn cmd_diff( } let mut repo = get_repo(ui, &matches)?; let mut_repo = Arc::get_mut(&mut repo).unwrap(); - if sub_matches.is_present("from") || sub_matches.is_present("to") {} let from_tree; let to_tree; if sub_matches.is_present("from") || sub_matches.is_present("to") { @@ -800,136 +799,139 @@ fn cmd_diff( } else { let mut styler = ui.styler(); styler.add_label(String::from("diff")); - from_tree.diff(&to_tree, &mut |path, diff| match diff { - TreeValueDiff::Added(TreeValue::Normal { - id, - executable: false, - }) => { - styler.add_label(String::from("header")); - styler.write_str(&format!("added file {}:\n", path.to_internal_string())); - styler.remove_label(); + for (path, diff) in from_tree.diff(&to_tree) { + match diff { + Diff::Added(TreeValue::Normal { + id, + executable: false, + }) => { + styler.add_label(String::from("header")); + styler.write_str(&format!("added file {}:\n", path.to_internal_string())); + styler.remove_label(); - let mut file_reader = repo.store().read_file(path, id).unwrap(); - styler.write_from_reader(&mut file_reader); - } - TreeValueDiff::Modified( - TreeValue::Normal { - id: id_left, - executable: left_executable, - }, - TreeValue::Normal { - id: id_right, - executable: right_executable, - }, - ) if left_executable == right_executable => { - styler.add_label(String::from("header")); - if *left_executable { + let mut file_reader = repo.store().read_file(&path, &id).unwrap(); + styler.write_from_reader(&mut file_reader); + } + Diff::Modified( + TreeValue::Normal { + id: id_left, + executable: left_executable, + }, + TreeValue::Normal { + id: id_right, + executable: right_executable, + }, + ) if left_executable == right_executable => { + styler.add_label(String::from("header")); + if left_executable { + styler.write_str(&format!( + "modified executable file {}:\n", + path.to_internal_string() + )); + } else { + styler + .write_str(&format!("modified file {}:\n", path.to_internal_string())); + } + styler.remove_label(); + + let mut file_reader_left = repo.store().read_file(&path, &id_left).unwrap(); + let mut buffer_left = vec![]; + file_reader_left.read_to_end(&mut buffer_left).unwrap(); + let mut file_reader_right = repo.store().read_file(&path, &id_right).unwrap(); + let mut buffer_right = vec![]; + file_reader_right.read_to_end(&mut buffer_right).unwrap(); + + print_diff( + buffer_left.as_slice(), + buffer_right.as_slice(), + styler.as_mut(), + ); + } + Diff::Modified( + TreeValue::Conflict(id_left), + TreeValue::Normal { + id: id_right, + executable: false, + }, + ) => { + styler.add_label(String::from("header")); styler.write_str(&format!( - "modified executable file {}:\n", + "resolved conflict in file {}:\n", path.to_internal_string() )); - } else { - styler.write_str(&format!("modified file {}:\n", path.to_internal_string())); + styler.remove_label(); + + let conflict_left = repo.store().read_conflict(&id_left).unwrap(); + let mut buffer_left = vec![]; + conflicts::materialize_conflict( + repo.store(), + &path.to_repo_path(), + &conflict_left, + &mut buffer_left, + ); + let mut file_reader_right = repo.store().read_file(&path, &id_right).unwrap(); + let mut buffer_right = vec![]; + file_reader_right.read_to_end(&mut buffer_right).unwrap(); + + print_diff( + buffer_left.as_slice(), + buffer_right.as_slice(), + styler.as_mut(), + ); } - styler.remove_label(); + Diff::Modified( + TreeValue::Normal { + id: id_left, + executable: false, + }, + TreeValue::Conflict(id_right), + ) => { + styler.add_label(String::from("header")); + styler.write_str(&format!( + "new conflict in file {}:\n", + path.to_internal_string() + )); + styler.remove_label(); - let mut file_reader_left = repo.store().read_file(path, id_left).unwrap(); - let mut buffer_left = vec![]; - file_reader_left.read_to_end(&mut buffer_left).unwrap(); - let mut file_reader_right = repo.store().read_file(path, id_right).unwrap(); - let mut buffer_right = vec![]; - file_reader_right.read_to_end(&mut buffer_right).unwrap(); + let mut file_reader_left = repo.store().read_file(&path, &id_left).unwrap(); + let mut buffer_left = vec![]; + file_reader_left.read_to_end(&mut buffer_left).unwrap(); + let conflict_right = repo.store().read_conflict(&id_right).unwrap(); + let mut buffer_right = vec![]; + conflicts::materialize_conflict( + repo.store(), + &path.to_repo_path(), + &conflict_right, + &mut buffer_right, + ); - print_diff( - buffer_left.as_slice(), - buffer_right.as_slice(), - styler.as_mut(), - ); - } - TreeValueDiff::Modified( - TreeValue::Conflict(id_left), - TreeValue::Normal { - id: id_right, + print_diff( + buffer_left.as_slice(), + buffer_right.as_slice(), + styler.as_mut(), + ); + } + Diff::Removed(TreeValue::Normal { + id, executable: false, - }, - ) => { - styler.add_label(String::from("header")); - styler.write_str(&format!( - "resolved conflict in file {}:\n", - path.to_internal_string() - )); - styler.remove_label(); + }) => { + styler.add_label(String::from("header")); + styler.write_str(&format!("removed file {}:\n", path.to_internal_string())); + styler.remove_label(); - let conflict_left = repo.store().read_conflict(id_left).unwrap(); - let mut buffer_left = vec![]; - conflicts::materialize_conflict( - repo.store(), - &path.to_repo_path(), - &conflict_left, - &mut buffer_left, - ); - let mut file_reader_right = repo.store().read_file(path, id_right).unwrap(); - let mut buffer_right = vec![]; - file_reader_right.read_to_end(&mut buffer_right).unwrap(); - - print_diff( - buffer_left.as_slice(), - buffer_right.as_slice(), - styler.as_mut(), - ); + let mut file_reader = repo.store().read_file(&path, &id).unwrap(); + styler.write_from_reader(&mut file_reader); + } + other => { + writeln!( + styler, + "unhandled diff case in path {:?}: {:?}", + path, other + ) + .unwrap(); + } } - TreeValueDiff::Modified( - TreeValue::Normal { - id: id_left, - executable: false, - }, - TreeValue::Conflict(id_right), - ) => { - styler.add_label(String::from("header")); - styler.write_str(&format!( - "new conflict in file {}:\n", - path.to_internal_string() - )); - styler.remove_label(); - - let mut file_reader_left = repo.store().read_file(path, id_left).unwrap(); - let mut buffer_left = vec![]; - file_reader_left.read_to_end(&mut buffer_left).unwrap(); - let conflict_right = repo.store().read_conflict(id_right).unwrap(); - let mut buffer_right = vec![]; - conflicts::materialize_conflict( - repo.store(), - &path.to_repo_path(), - &conflict_right, - &mut buffer_right, - ); - - print_diff( - buffer_left.as_slice(), - buffer_right.as_slice(), - styler.as_mut(), - ); - } - TreeValueDiff::Removed(TreeValue::Normal { - id, - executable: false, - }) => { - styler.add_label(String::from("header")); - styler.write_str(&format!("removed file {}:\n", path.to_internal_string())); - styler.remove_label(); - - let mut file_reader = repo.store().read_file(path, id).unwrap(); - styler.write_from_reader(&mut file_reader); - } - other => { - writeln!( - styler, - "unhandled diff case in path {:?}: {:?}", - path, other - ) - .unwrap(); - } - }); + } styler.remove_label(); } Ok(()) diff --git a/src/diff_edit.rs b/src/diff_edit.rs index 8b136c8d1..c5f9d9015 100644 --- a/src/diff_edit.rs +++ b/src/diff_edit.rs @@ -98,7 +98,7 @@ pub fn edit_diff(left_tree: &Tree, right_tree: &Tree) -> Result Result