Patchwork D1732: revsetlang: add utility function to return hash like symbols from the tree

login
register
mail settings
Submitter phabricator
Date Dec. 19, 2017, 12:10 p.m.
Message ID <differential-rev-PHID-DREV-6zthbunbg2wy6g5er2gb-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26358/
State Superseded
Headers show

Comments

phabricator - Dec. 19, 2017, 12:10 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Functionalities like unhiding changesets whose rev/hash is passed by the user
  required the knowledge of rev/hashes in the user provided specs. This patch adds
  functions which can parse tree object and return a list of such values.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1732

AFFECTED FILES
  mercurial/revsetlang.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 19, 2017, 2:09 p.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> revsetlang.py:665
> +
> +hashre = util.re.compile('[0-9a-fA-F]{1,40}')
> +

Nit: `_hashre` to mark it a private.

> revsetlang.py:674
> +
> +    for example for the revset 3::abe3ff it will return ('3', 'abe3ff')"""
> +    if not tree:

Nit: could be a doctest.

> revsetlang.py:679
> +    results = []
> +    if tree[0] == "symbol":
> +        results.append(tree[1])

My two cents, test `if _ishashlikesymbol(s)` here and early-return
`[tree[1]]`.

The last `[s for s ...]` doesn't make sense.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1732

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 25, 2017, 1:06 p.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> revsetlang.py:665
> +
> +_hashre = util.re.compile('[0-9a-fA-F]{1,40}')
> +

Oops, one more thing. Needs `$` to match whole word.

Test something like `('symbol', 'abe3ffZ')`.

> revsetlang.py:691
> +            results += gethashlikesymbols(subtree)
> +        # return directly, we don't need to filter symbols again
> +        return results

Nit: this comment should be removed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1732

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -661,3 +661,26 @@ 
         if tree[0] == 'func':
             funcs.add(tree[1][1])
         return funcs
+
+hashre = util.re.compile('[0-9a-fA-F]{1,40}')
+
+def _ishashlikesymbol(symbol):
+    """returns true if the symbol looks like a hash"""
+    return hashre.match(symbol)
+
+def gethashlikesymbols(tree):
+    """returns the list of symbols of the tree that look like hashes
+
+    for example for the revset 3::abe3ff it will return ('3', 'abe3ff')"""
+    if not tree:
+        return []
+
+    results = []
+    if tree[0] == "symbol":
+        results.append(tree[1])
+    elif len(tree) >= 3:
+        for subtree in tree[1:]:
+            results += gethashlikesymbols(subtree)
+        # return directly, we don't need to filter symbols again
+        return results
+    return [s for s in results if _ishashlikesymbol(s)]