( )⚙ D2904 templatefuncs: add mailmap template function

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2904

templatefuncs: add mailmap template function
ClosedPublic

Authored by sheehan on Mar 20 2018, 1:56 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG2a2ce93e12f4: templatefuncs: add mailmap template function
Summary

This commit adds a template function to support the .mailmap file
in Mercurial repositories. The .mailmap file comes from git, and
can be used to map new emails and names for old commits. The general
use case is that someone may change their name or author commits
under different emails and aliases, which would make these
commits appear as though they came from different persons. The
file allows you to specify the correct name that should be used
in place of the author field specified in the commit.

The mailmap file has 4 possible formats used to map old "commit"
names to new "proper" names:

  1. <proper@email.com> <commit@email.com>
  2. Proper Name <commit@email.com>
  3. Proper Name <proper@email.com> <commit@email.com>
  4. Proper Name <proper@email.com> Commit Name <commit@email.com>

Essentially there is a commit email present in each mailmap entry,
that maps to either an updated name, email, or both. The final
possible format allows commits authored by a person who used
both an old name and an old email to map to a new name and email.

To parse the file, we split by spaces and build a name out
of every element that does not start with "<". Once we find an element
that does start with "<" we concatenate all the name elements that preceded
and add that as a parsed name. We then add the email as the first
parsed email. We repeat the process until the end of the line, or
a comment is found. We will be left with all parsed names in a list,
and all parsed emails in a list, with the 0 index being the proper
values and the 1 index being the commit values (if they were specified
in the entry).

The commit values are added as the keys to a dict, and with the proper
fields as the values. The mapname function takes the mapping object and
the commit author field and attempts to look for a corresponding entry.
To do so we try (commit name, commit email) first, and if no results are
returned then (None, commit email) is also looked up. This is due to
format 4 from above, where someone may have a mailmap entry with both
name and email, and if they don't it is possible they have an entry that
uses only the commit email.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

sheehan created this revision.Mar 20 2018, 1:56 PM
yuja requested changes to this revision.Mar 21 2018, 9:31 AM
yuja added a subscriber: yuja.

Perhaps parsemailmap() and mapname() can be moved to new
utility module. And it's probably better to add some unit tests or doctests.

mercurial/templatefuncs.py
174

Current trend is to ditch namedtuple and use @attr.s instead.

178

Nit: avoid using map as variable name since there's map() function.

235

Perhaps util.email() and templatefilters.person() can be used instead.
In which case, person() should probably be moved to new utility
module.

248

Please use '%' formatting because bytes.format() isn't available
on Python 3.

254

Nit: **kwargs is unnecessary.

267

Perhaps repo.wvfs.tryread() can be used instead. That's what
hgext.lfs is doing to read .hglfs file.

274

No need to invalidate by mtime since the cache will be discarded
with the templating session .

And I think reading a file is much expensive than parsing.

tests/test-mailmap.t
8

Nit: -u can be used instead of HGUSER=

This revision now requires changes to proceed.Mar 21 2018, 9:31 AM
sheehan marked 5 inline comments as done.Mar 27 2018, 12:04 PM
sheehan updated this revision to Diff 7345.
sheehan marked 3 inline comments as done.Mar 29 2018, 10:10 AM
sheehan updated this revision to Diff 7361.
yuja requested changes to this revision.Mar 30 2018, 8:49 AM
yuja added inline comments.
mercurial/templatefuncs.py
186

Nit: .exists() isn't needed. tryread() handles it.

189

Still reading the file no matter if it is cached or not. This should be:

if 'mailmap' not in cache:
    data = repo.wvfs.tryread(...)
    cache['mailmap'] = parsemailmap(data)
194

Nit: try-except isn't needed thanks to tryread().

This revision now requires changes to proceed.Mar 30 2018, 8:49 AM
sheehan updated this revision to Diff 7381.Mar 30 2018, 11:11 AM
yuja accepted this revision.Mar 30 2018, 9:13 PM
This revision is now accepted and ready to land.Mar 30 2018, 9:13 PM
yuja added a comment.Mar 30 2018, 9:21 PM

Queued, thanks. Can you send a followup?

mercurial/templatefuncs.py
189

Perhaps the last or author wouldn't be necessary because that's
the default of mapname().

mercurial/utils/stringutil.py
380

An example to make it crash: b'>@<' (which is nice emoticon btw.)

Perhaps we can remove this any(...) and add if not email: continue instead.

Closed by commit rHG2a2ce93e12f4: templatefuncs: add mailmap template function (authored by sheehan). · Explain WhyMar 30 2018, 9:22 PM
This revision was automatically updated to reflect the committed changes.
sheehan added a comment.Mar 31 2018, 10:11 AM
In D2904#48406, @yuja wrote:

Queued, thanks. Can you send a followup?

Thanks for the review, @yuja! I'll send a followup shortly. :)

Revision Contents
Changeset List

PathPackages
Mmercurial/templatefuncs.py (32 lines)
Mmercurial/utils/stringutil.py (127 lines)
AMtests/test-mailmap.t (67 lines)

Diff 7361

mercurial/templatefuncs.py

revset as revsetmod, revset as revsetmod,
revsetlang, revsetlang,
scmutil, scmutil,
templatefilters, templatefilters,
templatekw, templatekw,
templateutil, templateutil,
util, util,
) )
from .utils import dateutil from .utils import (
dateutil,
stringutil,
)
evalrawexp = templateutil.evalrawexp evalrawexp = templateutil.evalrawexp
evalfuncarg = templateutil.evalfuncarg evalfuncarg = templateutil.evalfuncarg
evalboolean = templateutil.evalboolean evalboolean = templateutil.evalboolean
evalinteger = templateutil.evalinteger evalinteger = templateutil.evalinteger
evalstring = templateutil.evalstring evalstring = templateutil.evalstring
evalstringliteral = templateutil.evalstringliteral evalstringliteral = templateutil.evalstringliteral
evalastype = templateutil.evalastype evalastype = templateutil.evalastype
raise error.ParseError(_("formatnode expects one argument")) raise error.ParseError(_("formatnode expects one argument"))
ui = context.resource(mapping, 'ui') ui = context.resource(mapping, 'ui')
node = evalstring(context, mapping, args[0]) node = evalstring(context, mapping, args[0])
if ui.debugflag: if ui.debugflag:
return node return node
return templatefilters.short(node) return templatefilters.short(node)
@templatefunc('mailmap(author)')
def mailmap(context, mapping, args):
yujaUnsubmitted
Done

Current trend is to ditch namedtuple and use @attr.s instead.

yuja: Current trend is to ditch namedtuple and use `@attr.s` instead.
"""Return the author, updated according to the value
set in the mailmap"""
if len(args) != 1:
raise error.ParseError(_("mailmap expects one argument"))
yujaUnsubmitted
Done

Nit: avoid using map as variable name since there's map() function.

yuja: Nit: avoid using `map` as variable name since there's `map()` function.
author = evalfuncarg(context, mapping, args[0])
try:
cache = context.resource(mapping, 'cache')
repo = context.resource(mapping, 'repo')
if not repo.wvfs.exists('.mailmap'):
yujaUnsubmitted
Not Done

Nit: .exists() isn't needed. tryread() handles it.

yuja: Nit: `.exists()` isn't needed. `tryread()` handles it.
return author
data = repo.wvfs.tryread('.mailmap')
yujaUnsubmitted
Not Done

Still reading the file no matter if it is cached or not. This should be:

if 'mailmap' not in cache:
    data = repo.wvfs.tryread(...)
    cache['mailmap'] = parsemailmap(data)
yuja: Still reading the file no matter if it is cached or not. This should be: ``` if 'mailmap' not…
yujaUnsubmitted
Not Done

Perhaps the last or author wouldn't be necessary because that's
the default of mapname().

yuja: Perhaps the last `or author` wouldn't be necessary because that's the default of `mapname()`.
if 'mailmap' not in cache:
cache['mailmap'] = stringutil.parsemailmap(data)
except (error.ManifestLookupError, IOError):
yujaUnsubmitted
Not Done

Nit: try-except isn't needed thanks to tryread().

yuja: Nit: try-except isn't needed thanks to `tryread()`.
# Return the plain author if no mailmap file is found
return author
return stringutil.mapname(cache['mailmap'], author) or author
@templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])', @templatefunc('pad(text, width[, fillchar=\' \'[, left=False]])',
argspec='text width fillchar left') argspec='text width fillchar left')
def pad(context, mapping, args): def pad(context, mapping, args):
"""Pad text with a """Pad text with a
fill character.""" fill character."""
if 'text' not in args or 'width' not in args: if 'text' not in args or 'width' not in args:
# i18n: "pad" is a keyword # i18n: "pad" is a keyword
raise error.ParseError(_("pad() expects two to four arguments")) raise error.ParseError(_("pad() expects two to four arguments"))
return text return text
if left: if left:
return fillchar * fillwidth + text return fillchar * fillwidth + text
else: else:
return text + fillchar * fillwidth return text + fillchar * fillwidth
@templatefunc('indent(text, indentchars[, firstline])') @templatefunc('indent(text, indentchars[, firstline])')
def indent(context, mapping, args): def indent(context, mapping, args):
"""Indents all non-empty lines """Indents all non-empty lines
yujaUnsubmitted
Done

Perhaps util.email() and templatefilters.person() can be used instead.
In which case, person() should probably be moved to new utility
module.

yuja: Perhaps `util.email()` and `templatefilters.person()` can be used instead. In which case…
with the characters given in the indentchars string. An optional with the characters given in the indentchars string. An optional
third parameter will override the indent for the first line only third parameter will override the indent for the first line only
if present.""" if present."""
if not (2 <= len(args) <= 3): if not (2 <= len(args) <= 3):
# i18n: "indent" is a keyword # i18n: "indent" is a keyword
raise error.ParseError(_("indent() expects two or three arguments")) raise error.ParseError(_("indent() expects two or three arguments"))
text = evalstring(context, mapping, args[0]) text = evalstring(context, mapping, args[0])
indent = evalstring(context, mapping, args[1]) indent = evalstring(context, mapping, args[1])
if len(args) == 3: if len(args) == 3:
firstline = evalstring(context, mapping, args[2]) firstline = evalstring(context, mapping, args[2])
else: else:
yujaUnsubmitted
Done

Please use '%' formatting because bytes.format() isn't available
on Python 3.

yuja: Please use '%' formatting because bytes.format() isn't available on Python 3.
firstline = indent firstline = indent
# the indent function doesn't indent the first line, so we do it here # the indent function doesn't indent the first line, so we do it here
return templatefilters.indent(firstline + text, indent) return templatefilters.indent(firstline + text, indent)
@templatefunc('get(dict, key)') @templatefunc('get(dict, key)')
yujaUnsubmitted
Done

Nit: **kwargs is unnecessary.

yuja: Nit: `**kwargs` is unnecessary.
def get(context, mapping, args): def get(context, mapping, args):
"""Get an attribute/key from an object. Some keywords """Get an attribute/key from an object. Some keywords
are complex types. This function allows you to obtain the value of an are complex types. This function allows you to obtain the value of an
attribute on these types.""" attribute on these types."""
if len(args) != 2: if len(args) != 2:
# i18n: "get" is a keyword # i18n: "get" is a keyword
raise error.ParseError(_("get() expects two arguments")) raise error.ParseError(_("get() expects two arguments"))
dictarg = evalfuncarg(context, mapping, args[0]) dictarg = evalfuncarg(context, mapping, args[0])
if not util.safehasattr(dictarg, 'get'): if not util.safehasattr(dictarg, 'get'):
# i18n: "get" is a keyword # i18n: "get" is a keyword
raise error.ParseError(_("get() expects a dict as first argument")) raise error.ParseError(_("get() expects a dict as first argument"))
yujaUnsubmitted
Done

Perhaps repo.wvfs.tryread() can be used instead. That's what
hgext.lfs is doing to read .hglfs file.

yuja: Perhaps `repo.wvfs.tryread()` can be used instead. That's what `hgext.lfs` is doing to read `.
key = evalfuncarg(context, mapping, args[1]) key = evalfuncarg(context, mapping, args[1])
return templateutil.getdictitem(dictarg, key) return templateutil.getdictitem(dictarg, key)
@templatefunc('if(expr, then[, else])') @templatefunc('if(expr, then[, else])')
def if_(context, mapping, args): def if_(context, mapping, args):
"""Conditionally execute based on the result of """Conditionally execute based on the result of
an expression.""" an expression."""
yujaUnsubmitted
Done

No need to invalidate by mtime since the cache will be discarded
with the templating session .

And I think reading a file is much expensive than parsing.

yuja: No need to invalidate by mtime since the cache will be discarded with the templating session .
if not (2 <= len(args) <= 3): if not (2 <= len(args) <= 3):
# i18n: "if" is a keyword # i18n: "if" is a keyword
raise error.ParseError(_("if expects two or three arguments")) raise error.ParseError(_("if expects two or three arguments"))
test = evalboolean(context, mapping, args[0]) test = evalboolean(context, mapping, args[0])
if test: if test:
return evalrawexp(context, mapping, args[1]) return evalrawexp(context, mapping, args[1])
elif len(args) == 3: elif len(args) == 3:

mercurial/utils/stringutil.py

# stringutil.py - utility for generic string formatting, parsing, etc. # stringutil.py - utility for generic string formatting, parsing, etc.
# #
# Copyright 2005 K. Thananchayan <thananck@yahoo.com> # Copyright 2005 K. Thananchayan <thananck@yahoo.com>
# Copyright 2005-2007 Matt Mackall <mpm@selenic.com> # Copyright 2005-2007 Matt Mackall <mpm@selenic.com>
# Copyright 2006 Vadim Gelfer <vadim.gelfer@gmail.com> # Copyright 2006 Vadim Gelfer <vadim.gelfer@gmail.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import codecs import codecs
import re as remod import re as remod
import textwrap import textwrap
from ..i18n import _ from ..i18n import _
from ..thirdparty import attr
from .. import ( from .. import (
encoding, encoding,
error, error,
pycompat, pycompat,
) )
_DATA_ESCAPE_MAP = {pycompat.bytechr(i): br'\x%02x' % i for i in range(256)} _DATA_ESCAPE_MAP = {pycompat.bytechr(i): br'\x%02x' % i for i in range(256)}
""" """
if '@' not in author: if '@' not in author:
return author return author
f = author.find('<') f = author.find('<')
if f != -1: if f != -1:
return author[:f].strip(' "').replace('\\"', '"') return author[:f].strip(' "').replace('\\"', '"')
f = author.find('@') f = author.find('@')
return author[:f].replace('.', ' ') return author[:f].replace('.', ' ')
@attr.s(hash=True)
class mailmapping(object):
'''Represents a username/email key or value in
a mailmap file'''
email = attr.ib()
name = attr.ib(default=None)
def parsemailmap(mailmapcontent):
"""Parses data in the .mailmap format
>>> mmdata = b"\\n".join([
... b'# Comment',
... b'Name <commit1@email.xx>',
... b'<name@email.xx> <commit2@email.xx>',
... b'Name <proper@email.xx> <commit3@email.xx>',
... b'Name <proper@email.xx> Commit <commit4@email.xx>',
... ])
>>> mm = parsemailmap(mmdata)
>>> for key in sorted(mm.keys()):
... print(key)
mailmapping(email='commit1@email.xx', name=None)
mailmapping(email='commit2@email.xx', name=None)
mailmapping(email='commit3@email.xx', name=None)
mailmapping(email='commit4@email.xx', name='Commit')
>>> for val in sorted(mm.values()):
... print(val)
mailmapping(email='commit1@email.xx', name='Name')
mailmapping(email='name@email.xx', name=None)
mailmapping(email='proper@email.xx', name='Name')
mailmapping(email='proper@email.xx', name='Name')
"""
mailmap = {}
for line in mailmapcontent.splitlines():
# Don't bother checking the line if it is a comment or
# is an improperly formed author field
if line.lstrip().startswith('#') or any(c not in line for c in '<>@'):
continue
# name, email hold the parsed emails and names for each line
# name_builder holds the words in a persons name
yujaUnsubmitted
Not Done

An example to make it crash: b'>@<' (which is nice emoticon btw.)

Perhaps we can remove this any(...) and add if not email: continue instead.

yuja: An example to make it crash: `b'>@<'` (which is nice emoticon btw.) Perhaps we can remove this…
name, email = [], []
namebuilder = []
for element in line.split():
if element.startswith('#'):
# If we reach a comment in the mailmap file, move on
break
elif element.startswith('<') and element.endswith('>'):
# We have found an email.
# Parse it, and finalize any names from earlier
email.append(element[1:-1]) # Slice off the "<>"
if namebuilder:
name.append(' '.join(namebuilder))
namebuilder = []
# Break if we have found a second email, any other
# data does not fit the spec for .mailmap
if len(email) > 1:
break
else:
# We have found another word in the committers name
namebuilder.append(element)
mailmapkey = mailmapping(
email=email[-1],
name=name[-1] if len(name) == 2 else None,
)
mailmap[mailmapkey] = mailmapping(
email=email[0],
name=name[0] if name else None,
)
return mailmap
def mapname(mailmap, author):
"""Returns the author field according to the mailmap cache, or
the original author field.
>>> mmdata = b"\\n".join([
... b'# Comment',
... b'Name <commit1@email.xx>',
... b'<name@email.xx> <commit2@email.xx>',
... b'Name <proper@email.xx> <commit3@email.xx>',
... b'Name <proper@email.xx> Commit <commit4@email.xx>',
... ])
>>> m = parsemailmap(mmdata)
>>> mapname(m, b'Commit <commit1@email.xx>')
'Name <commit1@email.xx>'
>>> mapname(m, b'Name <commit2@email.xx>')
'Name <name@email.xx>'
>>> mapname(m, b'Commit <commit3@email.xx>')
'Name <proper@email.xx>'
>>> mapname(m, b'Commit <commit4@email.xx>')
'Name <proper@email.xx>'
>>> mapname(m, b'Unknown Name <unknown@email.com>')
'Unknown Name <unknown@email.com>'
"""
# If the author field coming in isn't in the correct format,
# just return the original author field
if not isauthorwellformed(author):
return author
# Turn the user name into a mailmaptup
commit = mailmapping(name=person(author), email=email(author))
try:
# Try and use both the commit email and name as the key
proper = mailmap[commit]
except KeyError:
# If the lookup fails, use just the email as the key instead
# We call this commit2 as not to erase original commit fields
commit2 = mailmapping(email=commit.email)
proper = mailmap.get(commit2, mailmapping(None, None))
# Return the author field with proper values filled in
return '%s <%s>' % (
proper.name if proper.name else commit.name,
proper.email if proper.email else commit.email,
)

tests/test-mailmap.t

  • This file was added.
Create a repo and add some commits
$ hg init mm
$ cd mm
$ echo "Test content" > testfile1
$ hg add testfile1
$ hg commit -m "First commit" -u "Proper <commit@m.c>"
$ echo "Test content 2" > testfile2
yujaUnsubmitted
Done

Nit: -u can be used instead of HGUSER=

yuja: Nit: `-u` can be used instead of `HGUSER=`
$ hg add testfile2
$ hg commit -m "Second commit" -u "Commit Name 2 <commit2@m.c>"
$ echo "Test content 3" > testfile3
$ hg add testfile3
$ hg commit -m "Third commit" -u "Commit Name 3 <commit3@m.c>"
$ echo "Test content 4" > testfile4
$ hg add testfile4
$ hg commit -m "Fourth commit" -u "Commit Name 4 <commit4@m.c>"
Add a .mailmap file with each possible entry type plus comments
$ cat > .mailmap << EOF
> # Comment shouldn't break anything
> <proper@m.c> <commit@m.c> # Should update email only
> Proper Name 2 <commit2@m.c> # Should update name only
> Proper Name 3 <proper@m.c> <commit3@m.c> # Should update name, email due to email
> Proper Name 4 <proper@m.c> Commit Name 4 <commit4@m.c> # Should update name, email due to name, email
> EOF
$ hg add .mailmap
$ hg commit -m "Add mailmap file" -u "Testuser <test123@m.c>"
Output of commits should be normal without filter
$ hg log -T "{author}\n" -r "all()"
Proper <commit@m.c>
Commit Name 2 <commit2@m.c>
Commit Name 3 <commit3@m.c>
Commit Name 4 <commit4@m.c>
Testuser <test123@m.c>
Output of commits with filter shows their mailmap values
$ hg log -T "{mailmap(author)}\n" -r "all()"
Proper <proper@m.c>
Proper Name 2 <commit2@m.c>
Proper Name 3 <proper@m.c>
Proper Name 4 <proper@m.c>
Testuser <test123@m.c>
Add new mailmap entry for testuser
$ cat >> .mailmap << EOF
> <newmmentry@m.c> <test123@m.c>
> EOF
Output of commits with filter shows their updated mailmap values
$ hg log -T "{mailmap(author)}\n" -r "all()"
Proper <proper@m.c>
Proper Name 2 <commit2@m.c>
Proper Name 3 <proper@m.c>
Proper Name 4 <proper@m.c>
Testuser <newmmentry@m.c>
A commit with improperly formatted user field should not break the filter
$ echo "some more test content" > testfile1
$ hg commit -m "Commit with improper user field" -u "Improper user"
$ hg log -T "{mailmap(author)}\n" -r "all()"
Proper <proper@m.c>
Proper Name 2 <commit2@m.c>
Proper Name 3 <proper@m.c>
Proper Name 4 <proper@m.c>
Testuser <newmmentry@m.c>
Improper user