( )⚙ D9203 dirstate-tree: simplify the control flow in the Node.insert method

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9203

dirstate-tree: simplify the control flow in the Node.insert method
ClosedPublic

Authored by marmoute on Oct 13 2020, 5:25 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGae2873e92250: dirstate-tree: simplify the control flow in the Node.insert method
Summary

But explicitly with the special case early, laying out the various case become
simpler.

(The initial motivation was to make some future lifetime error simpler).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Oct 13 2020, 5:25 PM
Alphare accepted this revision.Oct 16 2020, 4:48 AM
Alphare added a subscriber: Alphare.

We did this cleanup together, this is better than the original code. Thanks

marmoute added a commit: rHGae2873e92250: dirstate-tree: simplify the control flow in the Node.insert method.Oct 28 2020, 9:58 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGae2873e92250: dirstate-tree: simplify the control flow in the Node.insert method (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 23334

rust/hg-core/src/dirstate/dirstate_tree/node.rs

) -> InsertResult { ) -> InsertResult {
let mut split = path.splitn(2, |&c| c == b'/'); let mut split = path.splitn(2, |&c| c == b'/');
let head = split.next().unwrap_or(b""); let head = split.next().unwrap_or(b"");
let tail = split.next().unwrap_or(b""); let tail = split.next().unwrap_or(b"");
// Are we're modifying the current file ? Is the the end of the path ? // Are we're modifying the current file ? Is the the end of the path ?
let is_current_file = tail.is_empty() && head.is_empty(); let is_current_file = tail.is_empty() && head.is_empty();
// Potentially Replace the current file with a directory if it's marked
// as `Removed`
if !is_current_file {
if let NodeKind::File(file) = &mut self.kind { if let NodeKind::File(file) = &mut self.kind {
if file.entry.state == EntryState::Removed {
self.kind = NodeKind::Directory(Directory {
was_file: Some(Box::from(file.clone())),
children: Default::default(),
})
}
}
}
match &mut self.kind {
NodeKind::Directory(directory) => {
Node::insert_in_directory(directory, new_entry, head, tail)
}
NodeKind::File(file) => {
if is_current_file { if is_current_file {
let new = Self { let new = Self {
kind: NodeKind::File(File { kind: NodeKind::File(File {
entry: new_entry, entry: new_entry,
..file.clone() ..file.clone()
}), }),
}; };
return InsertResult { InsertResult {
did_insert: false, did_insert: false,
old_entry: Some(std::mem::replace(self, new)), old_entry: Some(std::mem::replace(self, new)),
}; }
} else { } else {
match file.entry.state { match file.entry.state {
// Only replace the current file with a directory if it's
// marked as `Removed`
EntryState::Removed => { EntryState::Removed => {
self.kind = NodeKind::Directory(Directory { unreachable!("Removed file turning into a directory was dealt with earlier")
was_file: Some(Box::from(file.clone())),
children: Default::default(),
})
} }
_ => { _ => {
return Node::insert_in_file( Node::insert_in_file(
file, new_entry, head, tail, file, new_entry, head, tail,
) )
} }
} }
} }
} }
match &mut self.kind {
NodeKind::Directory(directory) => {
Node::insert_in_directory(directory, new_entry, head, tail)
}
NodeKind::File(_) => {
unreachable!("The file case has already been handled")
}
} }
} }
/// The current file still exists and is not marked as `Removed`. /// The current file still exists and is not marked as `Removed`.
/// Insert the entry in its `was_directory`. /// Insert the entry in its `was_directory`.
fn insert_in_file( fn insert_in_file(
file: &mut File, file: &mut File,
new_entry: DirstateEntry, new_entry: DirstateEntry,