Patchwork [2,of,2] localrepo: comment that revs() is internal only

login
register
mail settings
Submitter Gregory Szorc
Date Dec. 20, 2015, 6:32 p.m.
Message ID <85666a6ed6c8403a50f1.1450636355@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/12187/
State Accepted
Delegated to: Matt Mackall
Headers show

Comments

Gregory Szorc - Dec. 20, 2015, 6:32 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1450636340 28800
#      Sun Dec 20 10:32:20 2015 -0800
# Node ID 85666a6ed6c8403a50f1c851701aa2d2a3c79c6f
# Parent  fd6eb3903a016edc748053a5aa42c74cefa6a7fb
localrepo: comment that revs() is internal only

I didn't realize repo.revs() was only appropriate for internal use.
Add a comment to help prevent others from falling into this pit.
Sean Farley - Dec. 20, 2015, 7:03 p.m.
Gregory Szorc <gregory.szorc@gmail.com> writes:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1450636340 28800
> #      Sun Dec 20 10:32:20 2015 -0800
> # Node ID 85666a6ed6c8403a50f1c851701aa2d2a3c79c6f
> # Parent  fd6eb3903a016edc748053a5aa42c74cefa6a7fb
> localrepo: comment that revs() is internal only
>
> I didn't realize repo.revs() was only appropriate for internal use.
> Add a comment to help prevent others from falling into this pit.

Looks good to me. I'd push it if I could :-)
Matt Mackall - Dec. 20, 2015, 10:56 p.m.
On Sun, 2015-12-20 at 10:32 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1450636340 28800
> #      Sun Dec 20 10:32:20 2015 -0800
> # Node ID 85666a6ed6c8403a50f1c851701aa2d2a3c79c6f
> # Parent  fd6eb3903a016edc748053a5aa42c74cefa6a7fb
> localrepo: comment that revs() is internal only
> 
> I didn't realize repo.revs() was only appropriate for internal use.
> Add a comment to help prevent others from falling into this pit.

> +        Note: this should only be used on internal-facing revsets. If
> +        operating on revsets provided by users, use ``revset.match()``
> +        directly to avoid revset expression formatting and so revset
> +        aliases work.

Generally, the layer of stuff that's prepared to deal with user
nonsense is cmdutil or scmutil. The usual pattern here is
scmutil.revrange/pair/single.

Tweaked in flight.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -521,16 +521,21 @@  class localrepository(object):
     def revs(self, expr, *args):
         '''Find revisions matching a revset.
 
         The revset is specified as a string ``expr`` that may contain
         %-formatting to escape certain types. See ``revset.formatspec``.
 
         Return a revset.abstractsmartset, which is a list-like interface
         that contains integer revisions.
+
+        Note: this should only be used on internal-facing revsets. If
+        operating on revsets provided by users, use ``revset.match()``
+        directly to avoid revset expression formatting and so revset
+        aliases work.
         '''
         expr = revset.formatspec(expr, *args)
         m = revset.match(None, expr)
         return m(self)
 
     def set(self, expr, *args):
         '''Find revisions matching a revset and emit changectx instances.