1116886 - Revision set functions and template keywords for pushlog data

Status: RESOLVED FIXED

(Developer Services :: Mercurial: Pushlog, defect)

Description

[Gregory Szorc [:gps]]

Now that pushlog is implemented as an extension, we can implement template keywords and revision set functions to facilitate more advanced querying of pushlog data.

Some of this is kinda/sorta implemented in the mozext extension. We should move that to the official pushlog extension

Comment 1

[Gregory Szorc [:gps]]

Attachment: pushlog: implement revsets to query pushlog data ()

This patch implements the pushhead(), pushuser(USER), and pushdate(DATE)
revision set functions.

Performance tested against the mozilla-central pushlog database is
acceptable.

Comment 2

[Gregory Szorc [:gps]]

Attachment: pushlog: implement template keywords for showing pushlog data ()

This push introduces a handful of template keywords to make displaying
pushlog info via templates possible.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8543117 [details] [diff] [review]
pushlog: implement revsets to query pushlog data ()

Review of attachment 8543117 [details] [diff] [review]:
-----------------------------------------------------------------

::: hgext/pushlog/__init__.py
@@ +381,5 @@
> +    # simplicity wins.
> +    def getrevs():
> +        for pushid, who, when, nodes in repo.pushlog.pushes():
> +            yield repo[nodes[-1]].rev()
> +

getargs(x, 0, 0, "pushhead takes no arguments")

BTW, if you want to fix this upstream, it's dumb that all the functions that do that in revsets.py use a different string like _("foo takes no arguments"), adding a i18n comment that foo is a keyword, instead of doing something like _("%s takes no arguments") % "foo", which would work for every different "foo" (and that would make the string for pushhead translated like the others).

@@ +385,5 @@
> +
> +    return subset & revset.generatorset(getrevs())
> +
> +def revset_pushdate(repo, subset, x):
> +    """``pushdate(DATE)``

``pushdate(interval)``
Changesets that were pushed within the interval, see :hg:`help dates`.

@@ +387,5 @@
> +
> +def revset_pushdate(repo, subset, x):
> +    """``pushdate(DATE)``
> +    Changesets that were pushed according to the date spec provided.
> +    """

l = getargs(x, 1, 1, "pushdate requires one argument")

(same remark as above wrt i18n)

@@ +388,5 @@
> +def revset_pushdate(repo, subset, x):
> +    """``pushdate(DATE)``
> +    Changesets that were pushed according to the date spec provided.
> +    """
> +    ds = revset.getstring(x, 'pushdate() requires a string argument')

revset.getstring(l[0], 'pushdate requires a string')
(no "()", and no "argument" to match the error message of other expressions)

@@ +400,5 @@
> +
> +    return subset & revset.generatorset(getrevs())
> +
> +def revset_pushuser(repo, subset, x):
> +    """``pushuser(USER)``

I think this should match user(), so it would be:
``pushuser(string)``
User name that pushed the changeset contains string. The match is case-insensitive.

If `string` starts with `re:`, the remainder of the string is treated as
a regular expression. To match a user that actually contains `re:`, use
the prefix `literal:`.

@@ +402,5 @@
> +
> +def revset_pushuser(repo, subset, x):
> +    """``pushuser(USER)``
> +    Changesets that were pushed by the specified username.
> +    """

l = getargs(x, 1, 1, "pushuser requires one argument")

@@ +403,5 @@
> +def revset_pushuser(repo, subset, x):
> +    """``pushuser(USER)``
> +    Changesets that were pushed by the specified username.
> +    """
> +    user = revset.getstring(x, 'pushuser() requires a string argument')

n = encoding.lower(revset.getstring(l[0], 'pushuser requires a string'))
kind, pattern, matcher = revset._substringmatcher(n)

(that function has been there since 2.3)

@@ +407,5 @@
> +    user = revset.getstring(x, 'pushuser() requires a string argument')
> +
> +    def getrevs():
> +        for pushid, who, when, nodes in repo.pushlog.pushes():
> +            if who == user:

if matcher(encoding.lower(user)):

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8543118 [details] [diff] [review]
pushlog: implement template keywords for showing pushlog data ()

Review of attachment 8543118 [details] [diff] [review]:
-----------------------------------------------------------------

::: hgext/pushlog/__init__.py
@@ +431,5 @@
> +        pushes = {}
> +        nodetopush = {}
> +        for push in repo.pushlog.pushes():
> +            pushid, who, when, nodes = push
> +            pushes[pushid] = push

Seems to me you don't need this part of the cache...

@@ +433,5 @@
> +        for push in repo.pushlog.pushes():
> +            pushid, who, when, nodes = push
> +            pushes[pushid] = push
> +            for node in nodes:
> +                nodetopush[node] = pushid

... if you just do nodetopush[node] = push

@@ +438,5 @@
> +
> +        cache['pushlog-pushes'] = pushes
> +        cache['pushlog-nodetopush'] = nodetopush
> +
> +    pushid = cache['pushlog-nodetopush'].get(ctx.hex())

return cache['pushlog-node-topush'].get(ctx.hex(), (None, None, None, None))

@@ +452,5 @@
> +    pushid, who, when, nodes = _getpushinfo(repo, ctx, cache)
> +    return pushid
> +
> +def template_pushuser(repo, ctx, templ, cache, **args):
> +    """:pushuser: String. The user who pushed this commit."""

s/commit/changeset/

@@ +462,5 @@
> +    pushid, who, when, nodes = _getpushinfo(repo, ctx, cache)
> +    return util.makedate(when) if when else None
> +
> +def template_pushbasenode(repo, ctx, templ, cache, **args):
> +    """:pushbasenode: String. The changeset identification hash, as a 40 digit

s/pushbasenode/pushbase/? Although, is that really useful?

@@ +470,5 @@
> +    pushid, who, when, nodes = _getpushinfo(repo, ctx, cache)
> +    return nodes[0] if nodes else None
> +
> +def template_pushheadnode(repo, ctx, templ, cache, **args):
> +    """:pushheadnode: String. The changeset identification hash, as a 40 digit

s/pushheadnode/pushhead/?

@@ +491,5 @@
>      revset.symbols['pushhead'] = revset_pushhead
>      revset.symbols['pushdate'] = revset_pushdate
>      revset.symbols['pushuser'] = revset_pushuser
>  
> +    templatekw.keywords['pushid'] = template_pushid

keywords = {
  ...
}
templatekw.keywords.update(keywords)
templatekw.dockeywords.update(keywords)

Comment 5

[Gregory Szorc [:gps]]

Attachment: pushlog: implement revsets to query pushlog data ()

This patch implements the pushhead(), pushuser(USER), and pushdate(DATE)
revision set functions.

Performance tested against the mozilla-central pushlog database is
acceptable.

Comment 6

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #4)
> @@ +462,5 @@
> > +    pushid, who, when, nodes = _getpushinfo(repo, ctx, cache)
> > +    return util.makedate(when) if when else None
> > +
> > +def template_pushbasenode(repo, ctx, templ, cache, **args):
> > +    """:pushbasenode: String. The changeset identification hash, as a 40 digit
> 
> s/pushbasenode/pushbase/? Although, is that really useful?

I figured symmetry was good in case anybody wanted to create a user interface for displaying pushes in the X::Y form. There are, of course, other keywords we could add like {pushnodes} and {pushparentnode}. I imagine these will be implemented eventually. I'm lazy today :)

> @@ +470,5 @@
> > +    pushid, who, when, nodes = _getpushinfo(repo, ctx, cache)
> > +    return nodes[0] if nodes else None
> > +
> > +def template_pushheadnode(repo, ctx, templ, cache, **args):
> > +    """:pushheadnode: String. The changeset identification hash, as a 40 digit
> 
> s/pushheadnode/pushhead/?

I wanted to maintain naming consistency with core. It uses "node" to differentiate from "rev".

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8544688 [details] [diff] [review]
pushlog: implement revsets to query pushlog data ()

Review of attachment 8544688 [details] [diff] [review]:
-----------------------------------------------------------------

Please change the commit message to match the docstrings.

You may want to add tests for pushuser substring matching, possibly re: matching, and pushdate intervals.

::: hgext/pushlog/__init__.py
@@ +409,5 @@
> +
> +    return subset & revset.generatorset(getrevs())
> +
> +def revset_pushuser(repo, subset, x):
> +    """``pushuser(USER)``

s/USER/string/, since you refer to `string` below (and that matches user()).

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/62a6caf5d03c
https://hg.mozilla.org/hgcustom/version-control-tools/rev/72429fe8dc51

Comment 9

[Gregory Szorc [:gps]]

Made requested changes, including addition of new tests.

Unfortunately, the date tests aren't terrific because dates in pushlog are always using time.time(). This is a deficiency in the testing framework that should eventually be addressed. I did add a "pushdate('yesterday to today')" test to verify intervals work.