Skip to content

Commit 360bf38

Browse files
committed
refactor
- try to not force `clone_from()` into move semantics unless necessary
1 parent a3d92b4 commit 360bf38

File tree

2 files changed

+32
-35
lines changed

2 files changed

+32
-35
lines changed

gix-blame/src/file/function.rs

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,9 @@ where
172172
if more_than_one_parent {
173173
// None of the changes affected the file we’re currently blaming.
174174
// Copy blame to parent.
175-
hunks_to_blame = hunks_to_blame
176-
.into_iter()
177-
.map(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id))
178-
.collect();
175+
for unblamed_hunk in &mut hunks_to_blame {
176+
unblamed_hunk.clone_blame(suspect, parent_id);
177+
}
179178
} else {
180179
pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame);
181180
}
@@ -202,26 +201,20 @@ where
202201
}
203202
}
204203

205-
hunks_to_blame = hunks_to_blame
206-
.into_iter()
207-
.filter_map(|mut unblamed_hunk| {
208-
if unblamed_hunk.suspects.len() == 1 {
209-
if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) {
210-
// At this point, we have copied blame for every hunk to a parent. Hunks
211-
// that have only `suspect` left in `suspects` have not passed blame to any
212-
// parent and so they can be converted to a `BlameEntry` and moved to
213-
// `out`.
214-
out.push(entry);
215-
216-
return None;
217-
}
204+
hunks_to_blame.retain_mut(|unblamed_hunk| {
205+
if unblamed_hunk.suspects.len() == 1 {
206+
if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) {
207+
// At this point, we have copied blame for every hunk to a parent. Hunks
208+
// that have only `suspect` left in `suspects` have not passed blame to any
209+
// parent, and so they can be converted to a `BlameEntry` and moved to
210+
// `out`.
211+
out.push(entry);
212+
return false;
218213
}
219-
220-
unblamed_hunk.remove_blame(suspect);
221-
222-
Some(unblamed_hunk)
223-
})
224-
.collect();
214+
}
215+
unblamed_hunk.remove_blame(suspect);
216+
true
217+
});
225218

226219
// This block asserts that line ranges for each suspect never overlap. If they did overlap
227220
// this would mean that the same line in a *Source File* would map to more than one line in

gix-blame/src/file/mod.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ fn process_change(
6666

6767
// Nothing to do with `hunk` except shifting it,
6868
// but `unchanged` needs to be checked against the next hunk to catch up.
69-
new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset));
69+
new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset));
7070
(None, Some(Change::Unchanged(unchanged)))
7171
}
7272
(false, false) => {
@@ -95,7 +95,7 @@ fn process_change(
9595

9696
// Nothing to do with `hunk` except shifting it,
9797
// but `unchanged` needs to be checked against the next hunk to catch up.
98-
new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset));
98+
new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset));
9999
(None, Some(Change::Unchanged(unchanged)))
100100
}
101101
}
@@ -125,7 +125,7 @@ fn process_change(
125125
}
126126
Either::Right((before, after)) => {
127127
// requeue the left side `before` after offsetting it…
128-
new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset));
128+
new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset));
129129
// …and treat `after` as `new_hunk`, which contains the `added` range.
130130
after
131131
}
@@ -162,7 +162,7 @@ fn process_change(
162162
Either::Left(hunk) => hunk,
163163
Either::Right((before, after)) => {
164164
// Keep looking for the left side of the unblamed portion.
165-
new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset));
165+
new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset));
166166
after
167167
}
168168
};
@@ -220,7 +220,7 @@ fn process_change(
220220
// <----> (added)
221221

222222
// Retry `hunk` once there is overlapping changes to process.
223-
new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset));
223+
new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset));
224224

225225
// Let hunks catchup with this change.
226226
(
@@ -273,7 +273,7 @@ fn process_change(
273273
}
274274
Either::Right((before, after)) => {
275275
// `before` isn't affected by deletion, so keep it for later.
276-
new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset));
276+
new_hunks_to_blame.push(before.cloned_blame(suspect, parent).shift_by(parent, *offset));
277277
// after will be affected by offset, and we will see if there are more changes affecting it.
278278
after
279279
}
@@ -285,7 +285,7 @@ fn process_change(
285285
// | (line_number_in_destination)
286286

287287
// Catchup with changes.
288-
new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset));
288+
new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset));
289289

290290
(
291291
None,
@@ -295,7 +295,7 @@ fn process_change(
295295
}
296296
(Some(hunk), None) => {
297297
// nothing to do - changes are exhausted, re-evaluate `hunk`.
298-
new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset));
298+
new_hunks_to_blame.push(hunk.cloned_blame(suspect, parent).shift_by(parent, *offset));
299299
(None, None)
300300
}
301301
(None, Some(Change::Unchanged(_))) => {
@@ -409,13 +409,17 @@ impl UnblamedHunk {
409409
}
410410
}
411411

412-
// TODO
413-
// Should this also accept `&mut self` as the other functions do?
414-
fn clone_blame(mut self, from: ObjectId, to: ObjectId) -> Self {
412+
/// This is like [`Self::clone_blame()`], but easier to use in places
413+
/// where the cloning is done 'inline'.
414+
fn cloned_blame(mut self, from: ObjectId, to: ObjectId) -> Self {
415+
self.clone_blame(from, to);
416+
self
417+
}
418+
419+
fn clone_blame(&mut self, from: ObjectId, to: ObjectId) {
415420
if let Some(range_in_suspect) = self.suspects.get(&from) {
416421
self.suspects.insert(to, range_in_suspect.clone());
417422
}
418-
self
419423
}
420424

421425
fn remove_blame(&mut self, suspect: ObjectId) {

0 commit comments

Comments
 (0)