This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9306

copies-rust: encapsulate internal sets on `changes`
ClosedPublic

Authored by marmoute on Nov 12 2020, 10:16 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG2367937982ba: copies-rust: encapsulate internal sets on `changes`
Summary

The goal is to eventually stop creating the underlying set. So we need to
encapsulate the call first.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Nov 12 2020, 10:16 AM
Alphare accepted this revision.Nov 13 2020, 7:58 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/copy_tracing.rs
54

The variants themselves should be documented instead

marmoute updated this revision to Diff 23500.Nov 13 2020, 7:05 PM
Alphare accepted this revision.Nov 16 2020, 6:21 AM
Alphare added inline comments.
rust/hg-core/src/copy_tracing.rs
57

Repeating the variant's name is not useful, because it's right next to the comment even in rustdoc context. But this is not a big deal.

Alphare updated this revision to Diff 23729.Nov 27 2020, 11:11 AM
marmoute updated this revision to Diff 23811.Nov 29 2020, 6:12 AM
pulkit accepted this revision.Nov 29 2020, 2:09 PM
This revision is now accepted and ready to land.Nov 29 2020, 2:09 PM
marmoute added a commit: rHG2367937982ba: copies-rust: encapsulate internal sets on `changes`.Nov 29 2020, 2:55 PM
Closed by commit rHG2367937982ba: copies-rust: encapsulate internal sets on `changes` (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/copy_tracing.rs (31 lines)

Diff 23729

rust/hg-core/src/copy_tracing.rs

/// (actually, this could be the edge from the other parent, but it does /// (actually, this could be the edge from the other parent, but it does
/// not matters) /// not matters)
Removed(&'a HgPath), Removed(&'a HgPath),
/// The parent → children edge introduce copy information between (dest, /// The parent → children edge introduce copy information between (dest,
/// source) /// source)
Copied(&'a HgPath, &'a HgPath), Copied(&'a HgPath, &'a HgPath),
} }
/// This express the possible "special" case we can get in a merge
///
/// See mercurial/metadata.py for details on these values.
AlphareUnsubmitted
Not Done

The variants themselves should be documented instead

Alphare: The variants themselves should be documented instead
#[derive(PartialEq)]
enum MergeCase {
/// Merged: file had history on both side that needed to be merged
AlphareUnsubmitted
Not Done

Repeating the variant's name is not useful, because it's right next to the comment even in rustdoc context. But this is not a big deal.

Alphare: Repeating the variant's name is not useful, because it's right next to the comment even in…
Merged,
/// Salvaged: file was candidate for deletion, but survived the merge
Salvaged,
/// Normal: Not one of the two cases above
Normal,
}
impl ChangedFiles { impl ChangedFiles {
pub fn new( pub fn new(
removed: HashSet<HgPathBuf>, removed: HashSet<HgPathBuf>,
merged: HashSet<HgPathBuf>, merged: HashSet<HgPathBuf>,
salvaged: HashSet<HgPathBuf>, salvaged: HashSet<HgPathBuf>,
copied_from_p1: PathCopies, copied_from_p1: PathCopies,
copied_from_p2: PathCopies, copied_from_p2: PathCopies,
) -> Self { ) -> Self {
2 => self.copied_from_p2.iter(), 2 => self.copied_from_p2.iter(),
_ => unreachable!(), _ => unreachable!(),
}; };
let remove_iter = self.removed.iter(); let remove_iter = self.removed.iter();
let copies_iter = copies_iter.map(|(x, y)| Action::Copied(x, y)); let copies_iter = copies_iter.map(|(x, y)| Action::Copied(x, y));
let remove_iter = remove_iter.map(|x| Action::Removed(x)); let remove_iter = remove_iter.map(|x| Action::Removed(x));
copies_iter.chain(remove_iter) copies_iter.chain(remove_iter)
} }
/// return the MergeCase value associated with a filename
fn get_merge_case(&self, path: &HgPath) -> MergeCase {
if self.salvaged.contains(path) {
return MergeCase::Salvaged;
} else if self.merged.contains(path) {
return MergeCase::Merged;
} else {
return MergeCase::Normal;
}
}
} }
/// A struct responsible for answering "is X ancestors of Y" quickly /// A struct responsible for answering "is X ancestors of Y" quickly
/// ///
/// The structure will delegate ancestors call to a callback, and cache the /// The structure will delegate ancestors call to a callback, and cache the
/// result. /// result.
#[derive(Debug)] #[derive(Debug)]
struct AncestorOracle<'a, A: Fn(Revision, Revision) -> bool> { struct AncestorOracle<'a, A: Fn(Revision, Revision) -> bool> {
} else { } else {
pick_major(); pick_major();
} }
} else if src_major.rev == src_minor.rev { } else if src_major.rev == src_minor.rev {
// We cannot get copy information for both p1 and p2 in the // We cannot get copy information for both p1 and p2 in the
// same rev. So this is the same value. // same rev. So this is the same value.
unreachable!(); unreachable!();
} else { } else {
let action = changes.get_merge_case(&dest);
if src_major.path.is_none() if src_major.path.is_none()
&& changes.salvaged.contains(dest) && action == MergeCase::Salvaged
{ {
// If the file is "deleted" in the major side but was // If the file is "deleted" in the major side but was
// salvaged by the merge, we keep the minor side alive // salvaged by the merge, we keep the minor side alive
pick_minor(); pick_minor();
} else if src_minor.path.is_none() } else if src_minor.path.is_none()
&& changes.salvaged.contains(dest) && action == MergeCase::Salvaged
{ {
// If the file is "deleted" in the minor side but was // If the file is "deleted" in the minor side but was
// salvaged by the merge, unconditionnaly preserve the // salvaged by the merge, unconditionnaly preserve the
// major side. // major side.
pick_major(); pick_major();
} else if changes.merged.contains(dest) { } else if action == MergeCase::Merged {
// If the file was actively merged, copy information // If the file was actively merged, copy information
// from each side might conflict. The major side will // from each side might conflict. The major side will
// win such conflict. // win such conflict.
pick_major(); pick_major();
} else if oracle.is_ancestor(src_major.rev, src_minor.rev) } else if oracle.is_ancestor(src_major.rev, src_minor.rev)
{ {
// If the minor side is strictly newer than the major // If the minor side is strictly newer than the major
// side, it should be kept. // side, it should be kept.