Re: D126: phabricator: add status to revision query language

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: D126: phabricator: add status to revision query language

dsp (David Soria Parra)
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch adds status words (ex. `abandoned`, `accepted`, `needsreview`,
  `needsrevision`, `closed`) to the revision query language so people can
  select revision in a more flexible way.

TEST PLAN
  Try something like `phabread ':2 & accepted'`, `phabread ':105 - closed` and
  make sure they have desired outputs.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -439,6 +439,13 @@
 _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'),
                               (r'node', 'Node ID'), (r'parent', 'Parent ')])
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -455,19 +462,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -595,15 +602,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -722,8 +733,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: quark, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
quark updated this revision to Diff 488.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D126?vs=238&id=488

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -438,6 +438,13 @@
 _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'),
                               (r'node', 'Node ID'), (r'parent', 'Parent ')])
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -454,19 +461,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -594,15 +601,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -722,8 +733,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



To: quark, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
In reply to this post by dsp (David Soria Parra)
quark updated this revision to Diff 556.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D126?vs=488&id=556

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -455,6 +455,13 @@
 _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'),
                               (r'node', 'Node ID'), (r'parent', 'Parent ')])
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -471,19 +478,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -611,15 +618,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -739,8 +750,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



To: quark, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
In reply to this post by dsp (David Soria Parra)
quark updated this revision to Diff 571.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D126?vs=556&id=571

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -489,6 +489,13 @@
 
     return True
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -505,19 +512,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -645,15 +652,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -773,8 +784,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



To: quark, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
In reply to this post by dsp (David Soria Parra)
quark updated this revision to Diff 841.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D126?vs=571&id=841

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -488,6 +488,13 @@
 
     return True
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -504,19 +511,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -644,15 +651,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -772,8 +783,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



To: quark, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
In reply to this post by dsp (David Soria Parra)
durin42 accepted this revision.
durin42 added a comment.
This revision is now accepted and ready to land.


  queued, thanks

REPOSITORY
  rHG Mercurial

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

To: quark, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

D126: phabricator: add status to revision query language

dsp (David Soria Parra)
In reply to this post by dsp (David Soria Parra)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGfb59192b4981: phabricator: add status to revision query language (authored by quark).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D126?vs=841&id=960

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

AFFECTED FILES
  contrib/phabricator.py

CHANGE DETAILS

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -488,6 +488,13 @@
 
     return True
 
+_knownstatusnames = {'accepted', 'needsreview', 'needsrevision', 'closed',
+                     'abandoned'}
+
+def _getstatusname(drev):
+    """get normalized status name from a Differential Revision"""
+    return drev[r'statusName'].replace(' ', '').lower()
+
 # Small language to specify differential revisions. Support symbols: (), :X,
 # +, and -.
 
@@ -504,19 +511,19 @@
 }
 
 def _tokenize(text):
-    text = text.replace(' ', '') # remove space
     view = memoryview(text) # zero-copy slice
-    special = '():+-&'
+    special = '():+-& '
     pos = 0
     length = len(text)
     while pos < length:
         symbol = ''.join(itertools.takewhile(lambda ch: ch not in special,
                                              view[pos:]))
         if symbol:
             yield ('symbol', symbol, pos)
             pos += len(symbol)
-        else: # special char
-            yield (text[pos], None, pos)
+        else: # special char, ignore space
+            if text[pos] != ' ':
+                yield (text[pos], None, pos)
             pos += 1
     yield ('end', None, pos)
 
@@ -644,15 +651,19 @@
         tofetch.update(range(max(1, r - batchsize), r + 1))
     if drevs:
         fetch({r'ids': list(tofetch)})
-    getstack(list(ancestordrevs))
+    validids = sorted(set(getstack(list(ancestordrevs))) | set(drevs))
 
     # Walk through the tree, return smartsets
     def walk(tree):
         op = tree[0]
         if op == 'symbol':
             drev = _parsedrev(tree[1])
             if drev:
                 return smartset.baseset([drev])
+            elif tree[1] in _knownstatusnames:
+                drevs = [r for r in validids
+                         if _getstatusname(prefetched[r]) == tree[1]]
+                return smartset.baseset(drevs)
             else:
                 raise error.Abort(_('unknown symbol: %s') % tree[1])
         elif op in {'and_', 'add', 'sub'}:
@@ -772,8 +783,13 @@
     ``&``, ``(``, ``)`` for complex queries. Prefix ``:`` could be used to
     select a stack.
 
+    ``abandoned``, ``accepted``, ``closed``, ``needsreview``, ``needsrevision``
+    could be used to filter patches by status. For performance reason, they
+    only represent a subset of non-status selections and cannot be used alone.
+
     For example, ``:D6+8-(2+D4)`` selects a stack up to D6, plus D8 and exclude
-    D2 and D4.
+    D2 and D4. ``:D9 & needsreview`` selects "Needs Review" revisions in a
+    stack up to D9.
 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.



To: quark, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Loading...