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