[RFC] different keyword extension architecture

Christian Ebert blacktrash at gmx.net
Thu Jan 31 09:05:17 CST 2008


Hi,

Comments on this patch and its rationale would be welcome, so I
know in which direction to work.

Personally I like that the stupid sys.argv parsing has gone
(which might cause more probs in the future), and that it's less
tied to filelog and more to the working dir.

It would be nice to keep expansion for web view (it works for
downloaded archives), at best only for filerevision and raw.
However I haven't found methods to do this except by
monkeypatching webcommands.rawfile and hgweb.filerevision -- the
latter would also have to wrap highlight's filerevision ... not
nice. Unfortunately a general approach like for kwcat had no
effect.

If I should continue in this direction I can split up the patch
if desired. See also <http://www.blacktrash.org/hg/hgkeyword/>
for a stepwise approach.

c



# HG changeset patch
# User Christian Ebert <blacktrash at gmx.net>
# Date 1201790639 -3600
# Node ID a2f69c24b1bc7bcfc2e679dcc5b241e47a1068f5
# Parent  6dcc190ffc36c0fc1b410b32f547324b10d2e27c
keyword: refactor overriding wread, wwrite, wwritedata repo methods

- restrict consistently to reading/writing in working dir
- no cludgy sys.argv parsing anymore
- extra kwcat command to output files with keywords expanded
  (falls back on "hg cat" if no filenames are configured)

Should ease collaboration with other extensions, or external tools
like TortoiseHg.

Changes in behaviour:
- hg cat does not expand; will be implemented with new kwcat command
- no expansion in web /display/, only in downloaded archives

Thanks to Jesse Glick for inciting this approach.

Internal changes:
- kwtemplater kwrepo attribute instead of global variable
- use kwcat in test

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -78,25 +78,17 @@
 "Log = {desc}" expands to the first line of the changeset description.
 '''
 
-from mercurial import commands, cmdutil, context, dispatch, filelog
-from mercurial import patch, localrepo, revlog, templater, util
+from mercurial import commands, cmdutil, context, filelog, localrepo
+from mercurial import patch, revlog, templater, util
 from mercurial.node import *
 from mercurial.i18n import _
-import re, shutil, sys, tempfile, time
+import re, shutil, tempfile, time
 
 commands.optionalrepo += ' kwdemo'
-
-# hg commands that trigger expansion only when writing to working dir,
-# not when reading filelog, and unexpand when reading from working dir
-restricted = ('diff1', 'record',
-              'qfold', 'qimport', 'qnew', 'qpush', 'qrefresh', 'qrecord')
 
 def utcdate(date):
     '''Returns hgdate in cvs-like UTC format.'''
     return time.strftime('%Y/%m/%d %H:%M:%S', time.gmtime(date[0]))
-
-
-_kwtemplater = None
 
 class kwtemplater(object):
     '''
@@ -113,13 +105,11 @@
         'Header': '{root}/{file},v {node|short} {date|utcdate} {author|user}',
     }
 
-    def __init__(self, ui, repo, inc, exc, hgcmd):
+    def __init__(self, ui, repo, inc, exc):
         self.ui = ui
         self.repo = repo
         self.matcher = util.matcher(repo.root, inc=inc, exc=exc)[1]
-        self.hgcmd = hgcmd
-        self.commitnode = None
-        self.path = ''
+        self.ctx = None
 
         kwmaps = self.ui.configitems('keywordmaps')
         if kwmaps: # override default templates
@@ -134,101 +124,84 @@
         self.ct = cmdutil.changeset_templater(self.ui, self.repo,
                                               False, '', False)
 
-    def substitute(self, node, data, subfunc):
-        '''Obtains file's changenode if commit node not given,
+    def substitute(self, path, data, node, subfunc):
+        '''Obtains file's changenode if node not given,
         and calls given substitution function.'''
-        if self.commitnode:
-            fnode = self.commitnode
-        else:
-            c = context.filectx(self.repo, self.path, fileid=node)
-            fnode = c.node()
+        if node is None:
+            # kwrepo.wwrite except when overwriting on commit
+            if self.ctx is None:
+                self.ctx = self.repo.changectx()
+            try:
+                fnode = self.ctx.filenode(path)
+                fl = self.repo.file(path)
+                c = context.filectx(self.repo, path, fileid=fnode, filelog=fl)
+                node = c.node()
+            except revlog.LookupError:
+                # eg: convert
+                return subfunc == self.re_kw.sub and data or (data, None)
+        elif subfunc == self.re_kw.sub:
+            # hg kwcat using kwfilelog.read
+            c = context.filectx(self.repo, path, fileid=node)
+            node = c.node()
 
         def kwsub(mobj):
             '''Substitutes keyword using corresponding template.'''
             kw = mobj.group(1)
             self.ct.use_template(self.templates[kw])
             self.ui.pushbuffer()
-            self.ct.show(changenode=fnode, root=self.repo.root, file=self.path)
+            self.ct.show(changenode=node, root=self.repo.root, file=path)
             return '$%s: %s $' % (kw, templater.firstline(self.ui.popbuffer()))
 
         return subfunc(kwsub, data)
 
-    def expand(self, node, data):
+    def expand(self, path, data, node):
         '''Returns data with keywords expanded.'''
-        if util.binary(data) or self.hgcmd in restricted:
+        if util.binary(data):
             return data
-        return self.substitute(node, data, self.re_kw.sub)
+        return self.substitute(path, data, node, self.re_kw.sub)
 
-    def process(self, node, data, expand):
+    def process(self, path, data, expand, ctx, node):
         '''Returns a tuple: data, count.
         Count is number of keywords/keyword substitutions,
         telling caller whether to act on file containing data.'''
         if util.binary(data):
             return data, None
         if expand:
-            return self.substitute(node, data, self.re_kw.subn)
-        return data, self.re_kw.search(data)
+            self.ctx = ctx
+            return self.substitute(path, data, node, self.re_kw.subn)
+        return self.re_kw.subn(r'$\1$', data)
 
-    def shrink(self, text):
+    def shrink(self, data):
         '''Returns text with all keyword substitutions removed.'''
-        if util.binary(text):
-            return text
-        return self.re_kw.sub(r'$\1$', text)
+        if util.binary(data):
+            return data
+        return self.re_kw.sub(r'$\1$', data)
 
 class kwfilelog(filelog.filelog):
     '''
-    Subclass of filelog to hook into its read, add, cmp methods.
-    Keywords are "stored" unexpanded, and processed on reading.
+    Subclass of filelog to hook into its read method for kwcat.
     '''
-    def __init__(self, opener, path):
+    def __init__(self, opener, path, kwt):
         super(kwfilelog, self).__init__(opener, path)
-        _kwtemplater.path = path
-
-    def kwctread(self, node, expand):
-        '''Reads expanding and counting keywords, called from _overwrite.'''
-        data = super(kwfilelog, self).read(node)
-        return _kwtemplater.process(node, data, expand)
+        self._kwt = kwt
+        self._path = path
 
     def read(self, node):
         '''Expands keywords when reading filelog.'''
         data = super(kwfilelog, self).read(node)
-        return _kwtemplater.expand(node, data)
-
-    def add(self, text, meta, tr, link, p1=None, p2=None):
-        '''Removes keyword substitutions when adding to filelog.'''
-        text = _kwtemplater.shrink(text)
-        return super(kwfilelog, self).add(text, meta, tr, link, p1=p1, p2=p2)
-
-    def cmp(self, node, text):
-        '''Removes keyword substitutions for comparison.'''
-        text = _kwtemplater.shrink(text)
-        if self.renamed(node):
-            t2 = super(kwfilelog, self).read(node)
-            return t2 != text
-        return revlog.revlog.cmp(self, node, text)
-
+        return self._kwt.expand(self._path, data, node)
 
 # store original patch.patchfile.__init__
 _patchfile_init = patch.patchfile.__init__
 
-def _kwpatchfile_init(self, ui, fname, missing=False):
-    '''Monkeypatch/wrap patch.patchfile.__init__ to avoid
-    rejects or conflicts due to expanded keywords in working dir.'''
-    _patchfile_init(self, ui, fname, missing=missing)
 
-    if _kwtemplater.matcher(self.fname):
-        # shrink keywords read from working dir
-        kwshrunk = _kwtemplater.shrink(''.join(self.lines))
-        self.lines = kwshrunk.splitlines(True)
-
-
-def _iskwfile(f, link):
-    return not link(f) and _kwtemplater.matcher(f)
+def _iskwfile(f, link, kwt):
+    return not link(f) and kwt.matcher(f)
 
 def _status(ui, repo, *pats, **opts):
     '''Bails out if [keyword] configuration is not active.
     Returns status of working directory.'''
-    if _kwtemplater:
+    if hasattr(repo, '_kwt'):
         files, match, anypats = cmdutil.matchpats(repo, pats, opts)
         return repo.status(files=files, match=match, list_clean=True)
     if ui.configitems('keyword'):
@@ -239,22 +212,22 @@
     '''Overwrites selected files expanding/shrinking keywords.'''
     ctx = repo.changectx(node)
     mf = ctx.manifest()
-    if node is not None:   # commit
-        _kwtemplater.commitnode = node
+    if node is not None:
+        # commit
         files = [f for f in ctx.files() if f in mf]
         notify = ui.debug
-    else:                  # kwexpand/kwshrink
+    else:
+        # kwexpand/kwshrink
         notify = ui.note
-    candidates = [f for f in files if _iskwfile(f, mf.linkf)]
+    candidates = [f for f in files if _iskwfile(f, mf.linkf, repo._kwt)]
     if candidates:
         candidates.sort()
         action = expand and 'expanding' or 'shrinking'
         for f in candidates:
-            fp = repo.file(f, kwmatch=True)
-            data, kwfound = fp.kwctread(mf[f], expand)
+            data, kwfound = repo._wreadkwct(f, expand, ctx, node)
             if kwfound:
                 notify(_('overwriting %s %s keywords\n') % (f, action))
-                repo.wwrite(f, data, mf.flags(f))
+                repo.wwrite(f, data, mf.flags(f), overwrite=True)
                 repo.dirstate.normal(f)
 
 def _kwfwrite(ui, repo, expand, *pats, **opts):
@@ -271,6 +244,26 @@
     finally:
         del wlock, lock
 
+def cat(ui, repo, file1, *pats, **opts):
+    '''output the current or given revision of files expanding keywords
+
+    Print the specified files as they were at the given revision.
+    If no revision is given, the parent of the working directory is used,
+    or tip if no revision is checked out.
+
+    Output may be to a file, in which case the name of the file is
+    given using a format string.  The formatting rules are the same as
+    for the export command, with the following additions:
+
+    %s   basename of file being printed
+    %d   dirname of file being printed, or '.' if in repo root
+    %p   root-relative path name of file being printed
+    '''
+    try:
+        repo.file = repo._kwfile
+    except AttributeError:
+        pass
+    commands.cat(ui, repo, file1, *pats, **opts)
 
 def demo(ui, repo, *args, **opts):
     '''print [keywordmaps] configuration and an expansion example
@@ -348,7 +341,7 @@
     repo.commit(text=msg)
     format = ui.verbose and ' in %s' % path or ''
     demostatus('%s keywords expanded%s' % (kwstatus, format))
-    ui.write(repo.wread(fn))
+    ui.write(repo.wopener(fn, 'r').read())
     ui.debug(_('\nremoving temporary repo %s\n') % tmpdir)
     shutil.rmtree(tmpdir, ignore_errors=True)
 
@@ -375,7 +368,7 @@
     if opts.get('untracked'):
         files += unknown
     files.sort()
-    kwfiles = [f for f in files if _iskwfile(f, repo._link)]
+    kwfiles = [f for f in files if _iskwfile(f, repo._link, repo._kwt)]
     cwd = pats and repo.getcwd() or ''
     kwfstats = not opts.get('ignore') and (('K', kwfiles),) or ()
     if opts.get('all') or opts.get('ignore'):
@@ -398,36 +391,10 @@
 
 
 def reposetup(ui, repo):
-    '''Sets up repo as kwrepo for keyword substitution.
-    Overrides file method to return kwfilelog instead of filelog
-    if file matches user configuration.
-    Wraps commit to overwrite configured files with updated
-    keyword substitutions.
-    This is done for local repos only, and only if there are
-    files configured at all for keyword substitution.'''
-
     if not repo.local():
         return
 
-    nokwcommands = ('add', 'addremove', 'bundle', 'clone', 'copy',
-                    'export', 'grep', 'identify', 'incoming', 'init',
-                    'log', 'outgoing', 'push', 'remove', 'rename',
-                    'rollback', 'tip',
-                    'convert')
-    hgcmd, func, args, opts, cmdopts = dispatch._parse(ui, sys.argv[1:])
-    if hgcmd in nokwcommands:
-        return
-
-    if hgcmd == 'diff':
-        # only expand if comparing against working dir
-        node1, node2 = cmdutil.revpair(repo, cmdopts.get('rev'))
-        if node2 is not None:
-            return
-        # shrink if rev is not current node
-        if node1 is not None and node1 != repo.changectx().node():
-            hgcmd = 'diff1'
-
-    inc, exc = [], ['.hgtags']
+    inc, exc = [], ['.hgtags', '.hg_archival.txt']
     for pat, opt in ui.configitems('keyword'):
         if opt != 'ignore':
             inc.append(pat)
@@ -436,26 +403,40 @@
     if not inc:
         return
 
-    global _kwtemplater
-    _kwtemplater = kwtemplater(ui, repo, inc, exc, hgcmd)
-
     class kwrepo(repo.__class__):
-        def file(self, f, kwmatch=False):
+        def _kwfile(self, f):
+            '''Returns filelog expanding keywords on read (for kwcat).'''
             if f[0] == '/':
                 f = f[1:]
-            if kwmatch or _kwtemplater.matcher(f):
-                return kwfilelog(self.sopener, f)
+            if self._kwt.matcher(f):
+                return kwfilelog(self.sopener, f, self._kwt)
             return filelog.filelog(self.sopener, f)
+
+        def _wreadkwct(self, filename, expand, ctx, node):
+            '''Reads filename and returns tuple of data with keywords
+            expanded/shrunk and count of keywords (for _overwrite).'''
+            data = super(kwrepo, self).wread(filename)
+            return self._kwt.process(filename, data, expand, ctx, node)
 
         def wread(self, filename):
             data = super(kwrepo, self).wread(filename)
-            if hgcmd in restricted and _kwtemplater.matcher(filename):
-                return _kwtemplater.shrink(data)
+            if self._kwt.matcher(filename):
+                return self._kwt.shrink(data)
             return data
+
+        def wwrite(self, filename, data, flags, overwrite=False):
+            if not overwrite and self._kwt.matcher(filename):
+                data = self._kwt.expand(filename, data, None)
+            super(kwrepo, self).wwrite(filename, data, flags)
+
+        def wwritedata(self, filename, data):
+            if self._kwt.matcher(filename):
+                data = self._kwt.expand(filename, data, None)
+            return super(kwrepo, self).wwritedata(filename, data)
 
         def commit(self, files=None, text='', user=None, date=None,
                    match=util.always, force=False, force_editor=False,
-                   p1=None, p2=None, extra={}):
+                   p1=None, p2=None, extra={}, empty_ok=False):
             wlock = lock = None
             _p1 = _p2 = None
             try:
@@ -483,7 +464,8 @@
                              self).commit(files=files, text=text, user=user,
                                           date=date, match=match, force=force,
                                           force_editor=force_editor,
-                                          p1=p1, p2=p2, extra=extra)
+                                          p1=p1, p2=p2, extra=extra,
+                                          empty_ok=empty_ok)
 
                 # restore commit hooks
                 for name, cmd in commithooks.iteritems():
@@ -495,11 +477,26 @@
             finally:
                 del wlock, lock
 
+    kwt = kwrepo._kwt = kwtemplater(ui, repo, inc, exc)
+
+    def kwpatchfile_init(self, ui, fname, missing=False):
+        '''Monkeypatch/wrap patch.patchfile.__init__ to avoid
+        rejects or conflicts due to expanded keywords in working dir.'''
+        _patchfile_init(self, ui, fname, missing=missing)
+
+        if kwt.matcher(self.fname):
+            # shrink keywords read from working dir
+            kwshrunk = kwt.shrink(''.join(self.lines))
+            self.lines = kwshrunk.splitlines(True)
+
     repo.__class__ = kwrepo
-    patch.patchfile.__init__ = _kwpatchfile_init
+    patch.patchfile.__init__ = kwpatchfile_init
 
 
 cmdtable = {
+    'kwcat':
+        (cat, commands.table['cat'][1],
+         _('hg kwcat [OPTION]... FILE...')),
     'kwdemo':
         (demo,
          [('d', 'default', None, _('show default keyword template maps')),
diff --git a/tests/test-keyword b/tests/test-keyword
--- a/tests/test-keyword
+++ b/tests/test-keyword
@@ -57,8 +57,8 @@
 hg --quiet identify
 echo % cat
 cat sym a b
-echo % hg cat
-hg cat sym a b
+echo % hg kwcat
+hg kwcat sym a b
 
 echo
 echo % diff a hooktest
@@ -144,8 +144,8 @@
 
 echo % cat
 cat sym a b
-echo % hg cat
-hg cat sym a b
+echo % hg kwcat
+hg kwcat sym a b
 
 echo
 echo '$Xinfo$' >> a
@@ -167,8 +167,8 @@
 
 echo % cat
 cat sym a b
-echo % hg cat
-hg cat sym a b
+echo % hg kwcat
+hg kwcat sym a b
 echo
 
 echo % remove
@@ -242,12 +242,6 @@
 hg --verbose kwshrink
 echo % cat
 cat sym a b
-echo % hg cat
-hg cat sym a b
+echo % hg kwcat
+hg kwcat sym a b
 echo
-rm $HGRCPATH
-echo % cat
-cat sym a b
-echo % hg cat
-hg cat sym a b
-echo
diff --git a/tests/test-keyword.out b/tests/test-keyword.out
--- a/tests/test-keyword.out
+++ b/tests/test-keyword.out
@@ -46,6 +46,7 @@
 
 list of commands:
 
+ kwcat      output the current or given revision of files expanding keywords
  kwdemo     print [keywordmaps] configuration and an expansion example
  kwexpand   expand keywords in working directory
  kwfiles    print files currently configured for keyword expansion
@@ -130,7 +131,7 @@
 do not process $Id:
 xxx $
 ignore $Id$
-% hg cat
+% hg kwcat
 expand $Id: a,v f782df5f9602 1970/01/01 00:00:00 user $
 do not process $Id:
 xxx $
@@ -239,7 +240,7 @@
 do not process $Id:
 xxx $
 ignore $Id$
-% hg cat
+% hg kwcat
 expand $Id: a f782df5f9602 Thu, 01 Jan 1970 00:00:00 +0000 user $
 do not process $Id:
 xxx $
@@ -266,7 +267,7 @@
 xxx $
 $Xinfo: User Name <user at example.com>: firstline $
 ignore $Id$
-% hg cat
+% hg kwcat
 expand $Id: a 0729690beff6 Thu, 01 Jan 1970 00:00:02 +0000 user $
 do not process $Id:
 xxx $
@@ -359,27 +360,10 @@
 xxx $
 $Xinfo$
 ignore $Id$
-% hg cat
+% hg kwcat
 expand $Id: a 0729690beff6 Thu, 01 Jan 1970 00:00:02 +0000 user $
 do not process $Id:
 xxx $
 $Xinfo: User Name <user at example.com>: firstline $
 ignore $Id$
 a
-% cat
-expand $Id$
-do not process $Id:
-xxx $
-$Xinfo$
-expand $Id$
-do not process $Id:
-xxx $
-$Xinfo$
-ignore $Id$
-% hg cat
-expand $Id$
-do not process $Id:
-xxx $
-$Xinfo$
-ignore $Id$
-a


More information about the Mercurial-devel mailing list