[PATCH 1 of 2] extensions: add revset support to most extensions

Matt Mackall mpm at selenic.com
Mon Nov 15 13:20:21 CST 2010


On Sat, 2010-11-13 at 13:06 -0600, Will Maier wrote:
> # HG changeset patch
> # User Will Maier <willmaier at ml1.net>
> # Date 1289478964 21600
> # Node ID 612f4471636dc602e5c1196bd3ad6fddb3edd6a6
> # Parent  91cbba199d8b1f1160f4b972fd15e1076b7edb5a
> extensions: add revset support to most extensions

Hmm, I see some problems here. There are -three- different APIs for
using revsets, depending on context.

a) revsingle - when only a single revision is meaningful (ie for
update_, we take the tipmost value of the revset

b) revpair - when we're interested in a pair of revisions (ie for diff),
we take the first and last members of the set

c) revrange - when we're interested in an arbitrary number of revisions,
we use the whole revset

Here you've applied (a) everywhere, and I spot a few places that ought
to have used (c) or possibly (b).

> -    rev = opts.get('rev')
> +    rev = cmdutil.revsingle(repo, opts.get('rev'))
>      if file_:
> -        ctx = repo.filectx(file_, changeid=rev)
> +        ctx = repo.filectx(file_, changeid=rev.rev())
>      else:
> -        ctx = repo[rev]
> +        ctx = rev
> 

This is bad style, as a 'rev' in the source is presumed to be an integer
or identifier, not a context. Move the .rev() to the 'rev =' line and
lose two weird changes.

Or better yet, use:

ctx = cmdutil.revsingle(...)
if file_:
    ctx = ctx[file]

> -                revs = [other.lookup(rev) for rev in opts['rev']]
> +                revs = [cmdutil.revsingle(other, rev).node() for rev in opts['rev']]

Here's a place that ought to use revrange.

> @@ -43,8 +43,8 @@ def difftree(ui, repo, node1=None, node2
>      """diff trees from two commits"""
>      def __difftree(repo, node1, node2, files=[]):
>          assert node2 is not None
> -        mmap = repo[node1].manifest()
> -        mmap2 = repo[node2].manifest()
> +        mmap = cmdutil.revsingle(repo, node1).manifest()
> +        mmap2 = cmdutil.revsingle(repo, node2).manifest()

Suspicious. Might want revpair.

> +        return revlog.hex(cmdutil.revsingle(repo, rev).node())

cmdutil.revsingle(..).hex()

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list