Skip to content

Commit

Permalink
merged_tree: rename MergedTreeValue to MergedTreeVal
Browse files Browse the repository at this point in the history 
I'm going to add `MergedTreeValue` as an alias for
`Merge<Option<TreeValue>>`, but we already have a type by that name in
`merged_tree`. This patch renames it away, to make room for the new
alias. I used `MergedTreeVal` for this borrowing version to be a bit
like how `str` is a borrowed version of `String`.
  • Loading branch information
@martinvonz
martinvonz committed Oct 26, 2023
1 parent b6eb00d commit 6ad71e6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
38 changes: 17 additions & 21 deletions lib/src/merged_tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ pub enum MergedTree {

/// The value at a given path in a `MergedTree`.
#[derive(PartialEq, Eq, Hash, Clone, Debug)]
pub enum MergedTreeValue<'a> {
pub enum MergedTreeVal<'a> {
/// A single non-conflicted value.
Resolved(Option<&'a TreeValue>),
/// TODO: Make this a `Merge<Option<&'a TreeValue>>` (reference to the
/// value) once we have removed the `MergedTree::Legacy` variant.
Conflict(Merge<Option<TreeValue>>),
}

impl MergedTreeValue<'_> {
impl MergedTreeVal<'_> {
fn to_merge(&self) -> Merge<Option<TreeValue>> {
match self {
MergedTreeValue::Resolved(value) => Merge::resolved(value.cloned()),
MergedTreeValue::Conflict(merge) => merge.clone(),
MergedTreeVal::Resolved(value) => Merge::resolved(value.cloned()),
MergedTreeVal::Conflict(merge) => merge.clone(),
}
}
}
Expand Down Expand Up @@ -178,26 +178,26 @@ impl MergedTree {
/// `self` is a `Merge`, which happens if the value at the path can be
/// trivially merged. Does not recurse, so if `basename` refers to a Tree,
/// then a `TreeValue::Tree` will be returned.
pub fn value(&self, basename: &RepoPathComponent) -> MergedTreeValue {
pub fn value(&self, basename: &RepoPathComponent) -> MergedTreeVal {
match self {
MergedTree::Legacy(tree) => match tree.value(basename) {
Some(TreeValue::Conflict(conflict_id)) => {
let path = tree.dir().join(basename);
let conflict = tree.store().read_conflict(&path, conflict_id).unwrap();
MergedTreeValue::Conflict(conflict)
MergedTreeVal::Conflict(conflict)
}
other => MergedTreeValue::Resolved(other),
other => MergedTreeVal::Resolved(other),
},
MergedTree::Merge(trees) => {
if let Some(tree) = trees.as_resolved() {
return MergedTreeValue::Resolved(tree.value(basename));
return MergedTreeVal::Resolved(tree.value(basename));
}
let value = trees.map(|tree| tree.value(basename));
if let Some(resolved) = value.resolve_trivial() {
return MergedTreeValue::Resolved(*resolved);
return MergedTreeVal::Resolved(*resolved);
}

MergedTreeValue::Conflict(value.map(|x| x.cloned()))
MergedTreeVal::Conflict(value.map(|x| x.cloned()))
}
}
}
Expand Down Expand Up @@ -238,14 +238,14 @@ impl MergedTree {
tree.sub_tree(name).map(MergedTree::Legacy)
} else {
match self.value(name) {
MergedTreeValue::Resolved(Some(TreeValue::Tree(sub_tree_id))) => {
MergedTreeVal::Resolved(Some(TreeValue::Tree(sub_tree_id))) => {
let subdir = self.dir().join(name);
Some(MergedTree::resolved(
self.store().get_tree(&subdir, sub_tree_id).unwrap(),
))
}
MergedTreeValue::Resolved(_) => None,
MergedTreeValue::Conflict(merge) => {
MergedTreeVal::Resolved(_) => None,
MergedTreeVal::Conflict(merge) => {
let merged_trees = merge.map(|value| match value {
Some(TreeValue::Tree(sub_tree_id)) => {
let subdir = self.dir().join(name);
Expand Down Expand Up @@ -517,7 +517,7 @@ impl<'a> TreeEntriesNonRecursiveIterator<'a> {
}

impl<'a> Iterator for TreeEntriesNonRecursiveIterator<'a> {
type Item = (&'a RepoPathComponent, MergedTreeValue<'a>);
type Item = (&'a RepoPathComponent, MergedTreeVal<'a>);

fn next(&mut self) -> Option<Self::Item> {
self.basename_iter.next().map(|basename| {
Expand Down Expand Up @@ -627,8 +627,8 @@ impl<'a> Iterator for ConflictEntriesNonRecursiveIterator<'a> {
fn next(&mut self) -> Option<Self::Item> {
for basename in self.basename_iter.by_ref() {
match self.merged_tree.value(basename) {
MergedTreeValue::Resolved(_) => {}
MergedTreeValue::Conflict(tree_values) => {
MergedTreeVal::Resolved(_) => {}
MergedTreeVal::Conflict(tree_values) => {
return Some((basename, tree_values));
}
}
Expand Down Expand Up @@ -765,11 +765,7 @@ impl<'a> TreeEntryDiffIterator<'a> {
}

impl<'a> Iterator for TreeEntryDiffIterator<'a> {
type Item = (
&'a RepoPathComponent,
MergedTreeValue<'a>,
MergedTreeValue<'a>,
);
type Item = (&'a RepoPathComponent, MergedTreeVal<'a>, MergedTreeVal<'a>);

fn next(&mut self) -> Option<Self::Item> {
for basename in self.basename_iter.by_ref() {
Expand Down
16 changes: 8 additions & 8 deletions lib/tests/test_merged_tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use jj_lib::backend::{FileId, MergedTreeId, TreeValue};
use jj_lib::files::MergeResult;
use jj_lib::matchers::{EverythingMatcher, FilesMatcher};
use jj_lib::merge::{Merge, MergeBuilder};
use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder, MergedTreeValue};
use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder, MergedTreeVal};
use jj_lib::repo::Repo;
use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
use jj_lib::tree::merge_trees;
Expand Down Expand Up @@ -119,20 +119,20 @@ fn test_from_legacy_tree() {
let merged_tree = MergedTree::from_legacy_tree(tree.clone()).unwrap();
assert_eq!(
merged_tree.value(&RepoPathComponent::from("missing")),
MergedTreeValue::Resolved(None)
MergedTreeVal::Resolved(None)
);
// file1: regular file without conflicts
assert_eq!(
merged_tree.value(&file1_path.components()[0]),
MergedTreeValue::Resolved(Some(&TreeValue::File {
MergedTreeVal::Resolved(Some(&TreeValue::File {
id: file1_id.clone(),
executable: false,
}))
);
// file2: 3-way conflict
assert_eq!(
merged_tree.value(&file2_path.components()[0]),
MergedTreeValue::Conflict(Merge::new(
MergedTreeVal::Conflict(Merge::new(
vec![Some(file_value(&file2_v1_id)), None],
vec![
Some(file_value(&file2_v2_id)),
Expand All @@ -144,15 +144,15 @@ fn test_from_legacy_tree() {
// file3: modify/delete conflict
assert_eq!(
merged_tree.value(&file3_path.components()[0]),
MergedTreeValue::Conflict(Merge::new(
MergedTreeVal::Conflict(Merge::new(
vec![Some(file_value(&file3_v1_id)), None],
vec![Some(file_value(&file3_v2_id)), None, None],
))
);
// file4: add/add conflict
assert_eq!(
merged_tree.value(&file4_path.components()[0]),
MergedTreeValue::Conflict(Merge::new(
MergedTreeVal::Conflict(Merge::new(
vec![None, None],
vec![
Some(file_value(&file4_v1_id)),
Expand All @@ -164,7 +164,7 @@ fn test_from_legacy_tree() {
// file5: 5-way conflict
assert_eq!(
merged_tree.value(&file5_path.components()[0]),
MergedTreeValue::Conflict(Merge::new(
MergedTreeVal::Conflict(Merge::new(
vec![
Some(file_value(&file5_v1_id)),
Some(file_value(&file5_v2_id)),
Expand All @@ -179,7 +179,7 @@ fn test_from_legacy_tree() {
// file6: directory without conflicts
assert_eq!(
merged_tree.value(&dir1_basename),
MergedTreeValue::Resolved(tree.value(&dir1_basename))
MergedTreeVal::Resolved(tree.value(&dir1_basename))
);

// Also test that MergedTreeBuilder can create the same tree by starting from an
Expand Down

0 comments on commit 6ad71e6

Please sign in to comment.