ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

refs: stick to Option<CommitId> type while merging ref targets

Browse source 
This might look more complicated than the original code, but it clarifies
that we always eliminate a (remove, add) pair. The behavior slightly changed
since an absent ref (i.e. None) in "removes" can now be paired with an "add"
even if "removes" contained other ids. Before, it was possible only when
removes.is_empty().
This commit is contained in:
Yuya Nishihara 2023-07-07 21:11:16 +09:00
parent 424786def1
commit ab2fa35a71
1 changed files with 32 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

65
lib/src/refs.rs
View file

@ -37,12 +37,22 @@ pub fn merge_ref_targets(
.flatten() .flatten()
.simplify(); .simplify();
// TODO: switch to conflict.is_resolved()
if conflict.as_resolved().is_some() {
conflict_to_ref_target(conflict)
} else {
let conflict = merge_ref_targets_non_trivial(index, conflict);
conflict_to_ref_target(conflict)
}
}
fn conflict_to_ref_target(conflict: Conflict<Option<CommitId>>) -> Option<RefTarget> {
match conflict.as_resolved() { match conflict.as_resolved() {
Some(Some(id)) => Some(RefTarget::Normal(id.clone())), Some(Some(id)) => Some(RefTarget::Normal(id.clone())),
Some(None) => None, // Deleted ref Some(None) => None, // Deleted ref
None => { None => {
let (removes, adds) = conflict.into_legacy_form(); let (removes, adds) = conflict.into_legacy_form();
merge_ref_targets_non_trivial(index, removes, adds) Some(RefTarget::Conflict { removes, adds })
} }
} }
} }
@ -63,49 +73,38 @@ fn ref_target_to_conflict(maybe_target: Option<&RefTarget>) -> Conflict<Option<C
fn merge_ref_targets_non_trivial( fn merge_ref_targets_non_trivial(
index: &dyn Index, index: &dyn Index,
mut removes: Vec<CommitId>, conflict: Conflict<Option<CommitId>>,
mut adds: Vec<CommitId>, ) -> Conflict<Option<CommitId>> {
) -> Option<RefTarget> { let (mut removes, mut adds) = conflict.take();
while let Some((maybe_remove_index, add_index)) = find_pair_to_remove(index, &removes, &adds) { while let Some((remove_index, add_index)) = find_pair_to_remove(index, &removes, &adds) {
if let Some(remove_index) = maybe_remove_index { removes.remove(remove_index);
removes.remove(remove_index);
}
adds.remove(add_index); adds.remove(add_index);
} }
Conflict::new(removes, adds)
if adds.is_empty() {
None
} else if adds.len() == 1 && removes.is_empty() {
Some(RefTarget::Normal(adds[0].clone()))
} else {
Some(RefTarget::Conflict { removes, adds })
}
} }
fn find_pair_to_remove( fn find_pair_to_remove(
index: &dyn Index, index: &dyn Index,
removes: &[CommitId], removes: &[Option<CommitId>],
adds: &[CommitId], adds: &[Option<CommitId>],
) -> Option<(Option<usize>, usize)> { ) -> Option<(usize, usize)> {
// If a "remove" is an ancestor of two different "adds" and one of the // If a "remove" is an ancestor of two different "adds" and one of the
// "adds" is an ancestor of the other, then pick the descendant. // "adds" is an ancestor of the other, then pick the descendant.
for (add_index1, add1) in adds.iter().enumerate() { for (add_index1, add1) in adds.iter().enumerate() {
for (add_index2, add2) in adds.iter().enumerate().skip(add_index1 + 1) { for (add_index2, add2) in adds.iter().enumerate().skip(add_index1 + 1) {
let (add_index, add) = if add1 == add2 || index.is_ancestor(add1, add2) { // TODO: Instead of relying on the list order, maybe ((add1, add2), remove)
(add_index1, add1) // combination should be somehow weighted?
} else if index.is_ancestor(add2, add1) { let (add_index, add_id) = match (add1, add2) {
(add_index2, add2) (Some(id1), Some(id2)) if id1 == id2 => (add_index1, id1),
} else { (Some(id1), Some(id2)) if index.is_ancestor(id1, id2) => (add_index1, id1),
continue; (Some(id1), Some(id2)) if index.is_ancestor(id2, id1) => (add_index2, id2),
_ => continue,
}; };
if removes.is_empty() { if let Some(remove_index) = removes.iter().position(|remove| match remove {
return Some((None, add_index)); Some(id) => index.is_ancestor(id, add_id),
} None => true, // Absent ref can be considered a root
if let Some(remove_index) = removes }) {
.iter() return Some((remove_index, add_index));
.position(|remove| index.is_ancestor(remove, add))
{
return Some((Some(remove_index), add_index));
} }
} }
} }