Skip to content

Commit

Permalink
CommitRewriter::rewrite_parents(): Take IntoIterator instead of `&[…
Browse files Browse the repository at this point in the history
…CommitId]`

This makes the API more flexible for cases where the caller doesn't already
have a Vec or array of owned CommitIds. CommitIds are often manipulated by
reference, so this makes the API more useable in many practical cases.

In many cases `rewrite_parents()` does not even need to clone the input
CommitIds.  This refactor allows the clone to be avoided if it's unnecessary.

There might be other APIs that would benefit from a similar change. In general,
it seems like there are a lot of places where we're writing
`&[commit_x.id().clone, commit_y.id().clone()]` and similiar.

- [Rust API Guidelines](https://rust-lang.github.io/api-guidelines/flexibility.html#functions-minimize-assumptions-about-parameters-by-using-generics-c-generic)
  • Loading branch information
emesterhazy committed Apr 21, 2024
1 parent dd643c9 commit 32f6f59
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,14 @@ impl<'repo> CommitRewriter<'repo> {

/// Update the intended new parents by replacing any occurrence of
/// `old_parent` by `new_parents`.
pub fn replace_parent(&mut self, old_parent: &CommitId, new_parents: &[CommitId]) {
pub fn replace_parent<'a>(
&mut self,
old_parent: &CommitId,
new_parents: impl IntoIterator<Item = &'a CommitId>,
) {
if let Some(i) = self.new_parents.iter().position(|p| p == old_parent) {
self.new_parents
.splice(i..i + 1, new_parents.iter().cloned());
.splice(i..i + 1, new_parents.into_iter().cloned());
let mut unique = HashSet::new();
self.new_parents.retain(|p| unique.insert(p.clone()));
}
Expand Down

0 comments on commit 32f6f59

Please sign in to comment.