Skip to content

Commit

Permalink
lib: mild refactoring made possible by previous commit
Browse files Browse the repository at this point in the history
Inline `create_descendant_commits`, move some functionality of
`DescendantRebaser::rebase_next` to `rebase_all`, a seemingly more logical
location.
  • Loading branch information
ilyagr committed Jan 2, 2024
1 parent 3c2cad5 commit 2624f18
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 24 deletions.
22 changes: 6 additions & 16 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,21 +882,6 @@ impl MutableRepo {
!(self.rewritten_commits.is_empty() && self.abandoned_commits.is_empty())
}

/// Creates a `DescendantRebaser` to rebase descendants of the recorded
/// rewritten and abandoned commits.
// TODO(ilyagr): Inline this. It's only used in tests.
fn create_descendant_rebaser<'settings, 'repo>(
&'repo mut self,
settings: &'settings UserSettings,
) -> DescendantRebaser<'settings, 'repo> {
DescendantRebaser::new(
settings,
self,
self.rewritten_commits.clone(),
self.abandoned_commits.clone(),
)
}

/// After the rebaser returned by this function is dropped,
/// self.clear_descendant_rebaser_plans() needs to be called.
fn rebase_descendants_return_rebaser<'settings, 'repo>(
Expand All @@ -908,7 +893,12 @@ impl MutableRepo {
// Optimization
return Ok(None);
}
let mut rebaser = self.create_descendant_rebaser(settings);
let mut rebaser = DescendantRebaser::new(
settings,
self,
self.rewritten_commits.clone(),
self.abandoned_commits.clone(),
);
*rebaser.mut_options() = options;
rebaser.rebase_all()?;
Ok(Some(rebaser))
Expand Down
14 changes: 6 additions & 8 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,12 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
new_commit,
}));
}
// TODO: As the TODO above says, we should probably change the API. Even if we
// don't, we should at least make this code not do any work if you call
// rebase_next() after we've returned None.
Ok(None)
}

pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> {
while self.rebase_next()?.is_some() {}
// TODO: As the TODO above says, we should probably change the API.
let mut view = self.mut_repo.view().store_view().clone();
for commit_id in &self.heads_to_remove {
view.head_ids.remove(commit_id);
Expand All @@ -600,11 +603,6 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
self.heads_to_remove.clear();
self.heads_to_add.clear();
self.mut_repo.set_view(view);
Ok(None)
}

pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> {
while self.rebase_next()?.is_some() {}
Ok(())
}
}
Expand Down

0 comments on commit 2624f18

Please sign in to comment.