Skip to content

Commit 600eb69

Browse files
committed
More aggressive mailmap substitution for better results (#364)
This is not what git seems to do, but it yields better results as more entries are unified and assigned to their respective authors. And it's valid, as emails are case-insensitive so they should match that way when unifying them.
1 parent 5052a4e commit 600eb69

File tree

2 files changed

+33
-11
lines changed

2 files changed

+33
-11
lines changed

git-mailmap/src/snapshot.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,15 @@ pub struct ResolvedSignature<'a> {
1414
}
1515

1616
impl<'a> ResolvedSignature<'a> {
17-
fn try_new(new_email: Option<&'a BString>, new_name: Option<&'a BString>) -> Option<Self> {
17+
fn try_new(
18+
new_email: Option<&'a BString>,
19+
matched_email: &'a BStr,
20+
current_email: &'_ BStr,
21+
new_name: Option<&'a BString>,
22+
) -> Option<Self> {
23+
let new_email = new_email
24+
.map(|n| n.as_bstr())
25+
.or_else(|| (matched_email != current_email).then(|| matched_email));
1826
match (new_email, new_name) {
1927
(None, None) => None,
2028
(new_email, new_name) => Some(ResolvedSignature {
@@ -244,6 +252,10 @@ impl Snapshot {
244252
/// Try to resolve `signature` by its contained email and name and provide resolved/mapped names as reference.
245253
/// Return `None` if no such mapping was found.
246254
///
255+
/// Note that opposed to what git seems to do, we also normalize the case of email addresses to match the one
256+
/// given in the mailmap. That is, if `Alex@example.com` is the current email, it will be matched and replaced with
257+
/// `alex@example.com`. This leads to better mapping results and saves entries in the mailmap.
258+
///
247259
/// This is the fastest possible lookup as there is no allocation.
248260
pub fn try_resolve_ref<'a>(&'a self, signature: git_actor::SignatureRef<'_>) -> Option<ResolvedSignature<'a>> {
249261
let email: EncodedStringRef<'_> = signature.email.into();
@@ -257,10 +269,20 @@ impl Snapshot {
257269

258270
match entry.entries_by_old_name.binary_search_by(|e| e.old_name.cmp_ref(name)) {
259271
Ok(pos) => {
260-
let entry = &entry.entries_by_old_name[pos];
261-
ResolvedSignature::try_new(entry.new_email.as_ref(), entry.new_name.as_ref())
272+
let name_entry = &entry.entries_by_old_name[pos];
273+
ResolvedSignature::try_new(
274+
name_entry.new_email.as_ref(),
275+
entry.old_email.as_bstr(),
276+
signature.email,
277+
name_entry.new_name.as_ref(),
278+
)
262279
}
263-
Err(_) => ResolvedSignature::try_new(entry.new_email.as_ref(), entry.new_name.as_ref()),
280+
Err(_) => ResolvedSignature::try_new(
281+
entry.new_email.as_ref(),
282+
entry.old_email.as_bstr(),
283+
signature.email,
284+
entry.new_name.as_ref(),
285+
),
264286
}
265287
}
266288

git-mailmap/tests/snapshot/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ fn try_resolve() {
66
let snapshot = Snapshot::from_bytes(&fixture_bytes("typical.txt"));
77
assert_eq!(
88
snapshot.try_resolve(signature("Foo", "Joe@example.com").to_ref()),
9-
Some(signature("Joe R. Developer", "Joe@example.com")),
10-
"resolved signatures contain all original fields, but normalizes only what's in the mapping, lookup is case-insensitive"
9+
Some(signature("Joe R. Developer", "joe@example.com")),
10+
"resolved signatures contain all original fields, and normalize the email as well to match the one that it was looked up with"
1111
);
1212
assert_eq!(
1313
snapshot.try_resolve(signature("Joe", "bugs@example.com").to_ref()),
@@ -56,7 +56,7 @@ fn try_resolve() {
5656
fn non_name_and_name_mappings_will_not_clash() {
5757
let entries = vec![
5858
// add mapping from email
59-
git_mailmap::Entry::change_name_and_email_by_email("new-name", "new-email", "old-email"),
59+
git_mailmap::Entry::change_name_by_email("new-name", "old-email"),
6060
// add mapping from name and email
6161
git_mailmap::Entry::change_name_and_email_by_name_and_email(
6262
"other-new-name",
@@ -69,12 +69,12 @@ fn non_name_and_name_mappings_will_not_clash() {
6969
let snapshot = Snapshot::new(entries);
7070

7171
assert_eq!(
72-
snapshot.try_resolve(signature("replace-by-email", "old-email").to_ref()),
73-
Some(signature("new-name", "new-email")),
74-
"it can match by email only"
72+
snapshot.try_resolve(signature("replace-by-email", "Old-Email").to_ref()),
73+
Some(signature("new-name", "old-email")),
74+
"it can match by email only, and the email is normalized"
7575
);
7676
assert_eq!(
77-
snapshot.try_resolve(signature("old-name", "old-email").to_ref()),
77+
snapshot.try_resolve(signature("old-name", "Old-Email").to_ref()),
7878
Some(signature("other-new-name", "other-new-email")),
7979
"it can match by email and name as well"
8080
);

0 commit comments

Comments
 (0)