[PATCH 1 of 2] keyword: expand RCS/CVS-like keywords in local directory

[Alexis S. L. Carvalho]

I haven't really tried this, but some general comments:

Thus spake Christian Ebert:
> # HG changeset patch
> # User Christian Ebert <blacktrash at gmx.net>
> # Date 1170945749 -3600
> # Node ID 9a2007f392b4d95e115b7c1aaf8b30de0c89c2b5
> # Parent  5b1f663ef86d68ce11d70de8e5ab61d93341a18c
> keyword: expand RCS/CVS-like keywords in local directory
> 
> Configuration is done in the [keyword] and [keywordmaps] sections
> of the rcfile(s).
> Keywords and their expansions can be customized using
> hg template mappings.
> 
> This extension breaks the tabu of changing the working directory,
> but following a very defensive policy:
> a) candidates for keyword expansion must be configured explicitly.
> b) only text files are touched.
> c) state without expansion is always stored:
>    for trouble shooting just disable the extension.
> 

Does this play well with filters?  I'd expect so...

> +'''keyword expansion in local repositories
> +
> +This extension expands RCS/CVS-like or self-customized keywords in
> +the text files selected by your configuration.
> +
> +Keywords are only expanded in local repositories and not logged by
> +Mercurial internally. The mechanism can be regarded as a convenience
> +for the current user and may be turned off anytime.
> +
> +Substitution takes place on every commit and update of the working
> +repository.
> +
> +Configuration is done in the [keyword] and [keywordmaps] sections of
> +hgrc files.
> +'''

It'd probably be good to have some documentation about these two
sections available through hg help (maybe using a separate topic if you
think the above is already too long).

> +
> +from mercurial.i18n import _
> +from mercurial import cmdutil, templater, util
> +from mercurial import context, filelog, revlog
> +import os.path, re, time
> +
> +deftemplates = {
> +        'Revision': '{node|short}',
> +        'Author': '{author|user}',
> +        'Date': '{date|utcdate}',
> +        'RCSFile': '{file|basename},v',
> +        'Source': '{root}/{file},v',
> +        'Id': '{file|basename},v {node|short} {date|utcdate} {author|user}',
> +        'Header': '{root}/{file},v {node|short} {date|utcdate} {author|user}',
> +        }
> +
> +def utcdate(date):
> +    '''Returns hgdate in cvs-like UTC format.'''
> +    return time.strftime('%Y/%m/%d %H:%M:%S', time.gmtime(date[0]))

I'm guessing you're shooting for as much compatibility with CVS as
possible (hence the ugly ",v" in the filenames and this format).  Maybe
this filter could go into templater.py...  hmm...

> +class kwtemplater(object):
> +    '''
> +    Sets up keyword templates, corresponding keyword regex, and
> +    provides keyword expansion function.
> +    '''
> +    def __init__(self, ui, repo):
> +        self.ui = ui
> +        self.repo = repo
> +        self.templates = dict(ui.configitems('keywordmaps')) or deftemplates

Wouldn't it be better to add the user-defined templates to the set of
available templates instead of completely replacing them?


> +def reposetup(ui, repo):

> +    class kwrepo(repo.__class__):
> +        def file(self, f):
> +            if f[0] == '/':
> +                f = f[1:]
> +            return filelog.filelog(self.sopener, f, self, self.revlogversion)
> +
> +        def commit(self, files=None, text="", user=None, date=None,
> +                match=util.always, force=False, lock=None, wlock=None,
> +                force_editor=False, p1=None, p2=None, extra={}):
> +            '''Wraps commit, expanding keywords of committed and
> +            configured files in working directory.'''
> +
> +            node = super(kwrepo, self).commit(files=files,
> +                    text=text, user=user, date=date,
> +                    match=match, force=force, lock=lock, wlock=wlock,
> +                    force_editor=force_editor, p1=p1, p2=p2, extra=extra)
> +            if node is None:
> +                return node
> +
> +            candidates = self.changelog.read(node)[3]
> +            candidates = [f for f in candidates
> +                    if self.kwfmatcher(f) and os.path.isfile(self.wjoin(f))]

os.path.isfile also returns True for symlinks that point to a file,
which is probably not what you want.

> +            if not candidates:
> +                return node
> +
> +            kwt = kwtemplater(self.ui, self)
> +            overwrite = []
> +            for f in candidates:
> +                data = self.wfile(f).read()
> +                if not util.binary(data):
> +                    data, kwct = kwt.re_kw.subn(lambda m:
> +                            kwt.expand(m, f, node), data)
> +                    if kwct:
> +                        ui.debug(_('overwriting %s expanding keywords\n' % f))
> +                        self.wfile(f, 'w').write(data)
> +                        overwrite.append(f)
> +            self.dirstate.update(overwrite, 'n')
> +            return node

Since you're changing the dirstate, you want to grab the wlock before
calling super().commit(); something like:

if not wlock:
    wlock = self.wlock()
<do-stuff>

(Truthfully, we want to change hg from the above pattern into something
like

wrelease = False
if not wlock:
    wlock = self.wlock()
    wrelease = True
try:
    <do-stuff>
finally:
    if wrelease:
        wlock.release()

probably with the help of some decorators (without using the python2.4
syntax).

The main reason is to reduce (even eliminate if possible) reliance on
__del__ methods, which aren't called in some situations involving e.g.
reference cycles)

> +
> +    class kwfilelog(filelog.filelog):

Why do you define this inside reposetup?  AFAICS it doesn't use anything
from it...

> +        '''
> +        Superclass over filelog to customize it's read, add, cmp methods.
> +        Keywords are "stored" unexpanded, and expanded on reading.
> +        '''
> +        def __init__(self, opener, path, repo,
> +                     defversion=revlog.REVLOG_DEFAULT_VERSION):
> +            super(kwfilelog, self).__init__(opener, path, defversion)
> +            self._repo = repo

Saving a reference to the repo in the filelog makes me slightly uneasy
because of the possibility of reference cycles (even though they don't
happen right now).  Using the weakref module would be an alternative.
But as I said it's not really needed right now.

> +            self._path = path
> +            # only init kwtemplater if needed
> +            if not isinstance(repo, int) and repo.kwfmatcher(path):

This is ugly...

> +
> +    filelog.filelog = kwfilelog

Do you really have to overwrite filelog.filelog?  Since you're already
changing repo.file, you might as well have it call kwfilelog directly.
I'm guessing this would allow you to avoid the isinstance(repo, int)
above.

Also, since reposetup is called once for every repo created, the
filelog.filelog that will be used by the repo may not be the exact same
filelog.filelog that it set in its reposetup.

Hope this helps

Alexis