D2904: templatefuncs: add mailmap template function
yuja (Yuya Nishihara)
phabricator at mercurial-scm.org
Wed Mar 21 09:32:12 EDT 2018
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.
Perhaps `parsemailmap()` and `mapname()` can be moved to new
utility module. And it's probably better to add some unit tests or doctests.
INLINE COMMENTS
> templatefuncs.py:172
> +# Represents mailmap keys/values
> +mailmaptup = collections.namedtuple('mailmaptup', ['name', 'email'])
> +
Current trend is to ditch namedtuple and use `@attr.s` instead.
> templatefuncs.py:176
> + """Parses data in the .mailmap format"""
> + map = {}
> + for line in mailmapcontent.splitlines():
Nit: avoid using `map` as variable name since there's `map()` function.
> templatefuncs.py:233
> + # Turn the user name into a mailmaptup
> + commit = mailmaptup(*(l.strip('> ') for l in author.split('<')))
> +
Perhaps `util.email()` and `templatefilters.person()` can be used instead.
In which case, `person()` should probably be moved to new utility
module.
> templatefuncs.py:246
> + # Return the author field with proper values filled in
> + return '{name} <{email}>'.format(
> + name=proper.name if proper.name else commit.name,
Please use '%' formatting because bytes.format() isn't available
on Python 3.
> templatefuncs.py:252
> + at templatefunc('mailmap(author)')
> +def mailmap(context, mapping, args, **kwargs):
> + """Return the author, updated according to the value
Nit: `**kwargs` is unnecessary.
> templatefuncs.py:265
> + wctx = repo[None]
> + wfctx = wctx.filectx('.mailmap')
> +
Perhaps `repo.wvfs.tryread()` can be used instead. That's what
`hgext.lfs` is doing to read `.hglfs` file.
> templatefuncs.py:272
> +
> + if 'mailmap' not in cache or cache['mailmap']['date'] < filedate:
> + cache['mailmap'] = {
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.
> test-mailmap.t:7
> + $ hg add testfile1
> + $ HGUSER="Proper <commit at m.c>" hg commit -m "First commit"
> + $ echo "Test content 2" > testfile2
Nit: `-u` can be used instead of `HGUSER=`
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2904
To: sheehan, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
More information about the Mercurial-devel
mailing list