Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove base from merge signature #56

Merged
merged 1 commit into from
May 25, 2024
Merged

refactor: remove base from merge signature #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion bin/src/control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ pub fn run_tool_on_merge_scenario(

log::info!("Starting merge of the trees");
let result = merge::merge(
&base_tree,
&left_tree,
&right_tree,
&matchings_left_base,
Expand Down
30 changes: 16 additions & 14 deletions merge/src/merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use model::CSTNode;
use crate::merged_cst_node::MergedCSTNode;

pub fn merge<'a>(
base: &'a CSTNode<'a>,
left: &'a CSTNode<'a>,
right: &'a CSTNode<'a>,
base_left_matchings: &'a Matchings<'a>,
Expand All @@ -27,11 +26,22 @@ pub fn merge<'a>(
));
}

match (base, left, right) {
(CSTNode::Terminal(a_base), CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => {
merge_terminals(a_base, a_left, a_right)
match (left, right) {
(CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => {
let matching_base_left = base_left_matchings.find_matching_for(left);
let matching_base_right = base_right_matchings.find_matching_for(right);
assert_eq!(matching_base_left, matching_base_right);

let base = matching_base_left
.map(|matching| matching.matching_node)
.and_then(|node| match node {
CSTNode::Terminal(terminal) => Some(terminal),
_ => None,
});

merge_terminals(base, a_left, a_right)
}
(CSTNode::NonTerminal(_), CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => {
(CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => {
if a_left.are_children_unordered && a_right.are_children_unordered {
Ok(unordered_merge(
a_left,
Expand All @@ -50,7 +60,7 @@ pub fn merge<'a>(
)?)
}
}
(_, _, _) => {
(_, _) => {
log::debug!(
"Error while merging NonTerminal with Terminal {} and {}",
left.contents(),
Expand All @@ -75,14 +85,6 @@ mod tests {
#[test]
fn test_can_not_merge_terminal_with_non_terminal() -> Result<(), Box<dyn std::error::Error>> {
let error = merge(
&CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 7 },
value: "value",
is_block_end_delimiter: false,
}),
&CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind",
Expand Down
78 changes: 17 additions & 61 deletions merge/src/merge_terminals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,23 @@ use model::cst_node::Terminal;
use crate::{MergeError, MergedCSTNode};

pub fn merge_terminals<'a>(
base: &'a Terminal<'a>,
base: Option<&'a Terminal<'a>>,
left: &'a Terminal<'a>,
right: &'a Terminal<'a>,
) -> Result<MergedCSTNode<'a>, MergeError> {
// Nodes of different kind, early return
if left.kind != right.kind {
return Err(MergeError::NodesWithDifferentKinds(
left.kind.to_string(),
right.kind.to_string(),
));
if left.value == right.value {
return Ok(left.to_owned().into());
}

// Unchanged
if left.value == base.value && right.value == base.value {
Ok(base.to_owned().into())
// Changed in both
} else if left.value != base.value && right.value != base.value {
match diffy::merge(base.value, left.value, right.value) {
Ok(value) => Ok(MergedCSTNode::Terminal {
kind: base.kind,
value,
}),
Err(value) => Ok(MergedCSTNode::Terminal {
kind: base.kind,
value,
}),
}
// Only left changed
} else if left.value != base.value {
Ok(left.to_owned().into())
// Only right changed
} else {
Ok(right.to_owned().into())
match diffy::merge(base.map_or("", |node| node.value), left.value, right.value) {
Ok(value) => Ok(MergedCSTNode::Terminal {
kind: left.kind,
value,
}),
Err(value) => Ok(MergedCSTNode::Terminal {
kind: left.kind,
value,
}),
}
}

Expand All @@ -46,7 +30,7 @@ mod tests {
use model::{cst_node::Terminal, Point};

fn assert_merge_is_correct_and_idempotent_with_respect_to_parent_side(
base: &Terminal,
base: Option<&Terminal>,
parent_a: &Terminal,
parent_b: &Terminal,
expected_merge: &MergedCSTNode,
Expand All @@ -72,7 +56,7 @@ mod tests {
};

assert_merge_is_correct_and_idempotent_with_respect_to_parent_side(
&node,
Some(&node),
&node,
&node,
&node.clone().into(),
Expand Down Expand Up @@ -108,7 +92,7 @@ mod tests {
};

assert_merge_is_correct_and_idempotent_with_respect_to_parent_side(
&base,
Some(&base),
&left,
&right,
&MergedCSTNode::Terminal {
Expand Down Expand Up @@ -147,7 +131,7 @@ mod tests {
};

assert_eq!(
merge_terminals(&base, &left, &right)?,
merge_terminals(Some(&base), &left, &right)?,
MergedCSTNode::Terminal {
kind: "kind",
value: "<<<<<<< ours\nleft_value||||||| original\nvalue=======\nright_value>>>>>>> theirs\n".to_string()
Expand Down Expand Up @@ -178,38 +162,10 @@ mod tests {
};

assert_merge_is_correct_and_idempotent_with_respect_to_parent_side(
&base_and_left,
Some(&base_and_left),
&base_and_left,
&changed_parent,
&changed_parent.clone().into(),
)
}

#[test]
fn i_get_an_error_if_i_try_to_merge_nodes_of_different_kinds() {
let kind_a = Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_a",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 7 },
value: "value",
is_block_end_delimiter: false,
};
let kind_b = Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_b",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 7 },
value: "value_right",
is_block_end_delimiter: false,
};

let result = merge_terminals(&kind_a, &kind_a, &kind_b);

assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
MergeError::NodesWithDifferentKinds("kind_a".to_string(), "kind_b".to_string())
);
}
}
2 changes: 0 additions & 2 deletions merge/src/ordered_merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ pub fn ordered_merge<'a>(
) {
(true, Some(_), Some(_), Some(_), Some(_)) => {
result_children.push(crate::merge(
cur_left,
cur_left,
cur_right,
base_left_matchings,
Expand All @@ -56,7 +55,6 @@ pub fn ordered_merge<'a>(
}
(true, Some(_), None, Some(_), None) => {
result_children.push(crate::merge(
cur_left,
cur_left,
cur_right,
base_left_matchings,
Expand Down
4 changes: 0 additions & 4 deletions merge/src/unordered_merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ pub fn unordered_merge<'a>(
}
(None, Some(right_matching)) => {
result_children.push(merge(
left_child,
left_child,
right_matching.matching_node,
base_left_matchings,
Expand All @@ -71,7 +70,6 @@ pub fn unordered_merge<'a>(
}
(Some(_), Some(right_matching)) => {
result_children.push(merge(
left_child,
left_child,
right_matching.matching_node,
base_left_matchings,
Expand Down Expand Up @@ -99,7 +97,6 @@ pub fn unordered_merge<'a>(
}
(None, Some(matching_left_right)) => {
result_children.push(merge(
right_child,
matching_left_right.matching_node,
right_child,
base_left_matchings,
Expand All @@ -119,7 +116,6 @@ pub fn unordered_merge<'a>(
}
(Some(_), Some(matching_left_right)) => {
result_children.push(merge(
right_child,
matching_left_right.matching_node,
right_child,
base_left_matchings,
Expand Down
Loading