e29d33ec47b1

cli: add support for deleting items
[view raw] [browse files]
author Steve Losh <steve@stevelosh.com>
date Sat, 19 Jun 2010 02:03:37 -0400
parents 537000a18d86
children 40e13965af72
branches/tags (none)
files review/cli.py review/tests/test_delete.py review/tests/util.py

Changes

--- a/review/cli.py	Sat Jun 19 02:03:29 2010 -0400
+++ b/review/cli.py	Sat Jun 19 02:03:37 2010 -0400
@@ -239,6 +239,26 @@
             for comment in line['comments']:
                 _print_comment(comment)
 
+def _delete_command(ui, repo, *identifiers, **opts):
+    # TODO: require -f to delete some else's item
+    force = opts.pop('force')
+    
+    if not identifiers:
+        raise util.Abort(messages.DELETE_REQUIRES_IDS)
+
+    try:
+        rd = api.ReviewDatastore(ui, repo)
+    except api.UninitializedDatastore:
+        raise util.Abort(messages.NO_DATA_STORE)
+
+    for i in identifiers:
+        try:
+            rd.remove_item(i)
+        except api.UnknownIdentifier:
+            raise util.Abort(messages.DELETE_UNKNOWN_ID % i)
+        except api.AmbiguousIdentifier:
+            raise util.Abort(messages.DELETE_AMBIGUOUS_ID % i)
+
 
 _review_effects = {
     'deleted': ['red'],
@@ -297,7 +317,7 @@
         pass
 
 
-def review(ui, repo, *fnames, **opts):
+def review(ui, repo, *args, **opts):
     """code review changesets in the current repository
 
     To start using the review extension with a repository, you need to
@@ -325,13 +345,15 @@
     elif opts.pop('init'):
         return _init_command(ui, repo, **opts)
     elif opts.pop('comment'):
-        return _comment_command(ui, repo, *fnames, **opts)
+        return _comment_command(ui, repo, *args, **opts)
     elif opts.pop('signoff'):
         return _signoff_command(ui, repo, **opts)
     elif opts.pop('check'):
         return _check_command(ui, repo, **opts)
+    elif opts.pop('delete'):
+        return _delete_command(ui, repo, *args, **opts)
     else:
-        return _review_command(ui, repo, *fnames, **opts)
+        return _review_command(ui, repo, *args, **opts)
 
 
 cmdtable = {
@@ -341,6 +363,8 @@
         ('',  'mdown',       False, 'use Markdown to format the comment or signoff message'),
         ('r', 'rev',         '.',   'the revision to review'),
 
+        ('d', 'delete',      False, 'delete a comment or signoff'),
+
         ('',  'check',       False, 'check the review status of the given revision'),
         ('',  'no-nos',      False, 'ensure this revision does NOT have signoffs of "no"'),
         ('',  'yeses',       '',    'ensure this revision has at least NUM signoffs of "yes"'),
@@ -355,7 +379,7 @@
         ('s', 'signoff',     False, 'sign off'),
         ('',  'yes',         False, 'sign off as stating the changeset is good'),
         ('',  'no',          False, 'sign off as stating the changeset is bad'),
-        ('f', 'force',       False, 'overwrite an existing signoff'),
+        ('f', 'force',       False, 'force an action'),
 
         ('w', 'web',         False,       'launch the web interface'),
         ('',  'read-only',   False,       'make the web interface read-only'),
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/review/tests/test_delete.py	Sat Jun 19 02:03:37 2010 -0400
@@ -0,0 +1,144 @@
+from nose import *
+from util import *
+from .. import api, messages
+
+from mercurial import util as hgutil
+
+a1, a2 = (messages.REVIEW_LOG_COMMENT_AUTHOR % '|').split('|')
+s1, s2 = (messages.REVIEW_LOG_SIGNOFF_AUTHOR % ('|', 'neutral')).split('|')
+
+def _get_identifiers(rev='.'):
+    return [l.split(' ')[-1].strip('()\n')
+            for l in review(rev=rev, verbose=True).splitlines()
+            if (a1 in l and a2 in l) or (s1 in l and s2 in l)]
+
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_delete_invalid():
+    # TODO: rename files.
+
+    try:
+        review(delete=True)
+    except hgutil.Abort, e:
+        error = str(e)
+        assert messages.DELETE_REQUIRES_IDS in error
+    else:
+        assert False, 'The correct error message was not printed.'
+
+    try:
+        review(delete=True, files=['a'])
+    except hgutil.Abort, e:
+        error = str(e)
+        assert messages.DELETE_UNKNOWN_ID % 'a' in error
+    else:
+        assert False, 'The correct error message was not printed.'
+
+    review(comment=True, message='test')
+
+    try:
+        review(delete=True, files=['z'])
+    except hgutil.Abort, e:
+        error = str(e)
+        assert messages.DELETE_UNKNOWN_ID % 'z' in error
+    else:
+        assert False, 'The correct error message was not printed.'
+
+    # Use the pidgeonhole princicple to create ambiguous identifiers.
+    for i in range(17):
+        review(comment=True, message='test%d' % i)
+
+    ids = _get_identifiers()
+    id_map = {}
+    for i in ids:
+        id_map[i[0]] = id_map.get(i[0], 0) + 1
+    i = str(filter(lambda k: id_map[k] > 1, id_map.keys())[0])
+
+    try:
+        review(delete=True, files=[i])
+    except hgutil.Abort, e:
+        error = str(e)
+        print error
+        assert messages.DELETE_AMBIGUOUS_ID % i in error
+    else:
+        assert False, 'The correct error message was not printed.'
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_delete_comment():
+    def t(rev):
+        review(rev=rev, comment=True, message='test')
+        i = _get_identifiers(rev)[0]
+
+        output = review(rev=rev, delete=True, files=[i])
+        assert not output
+        output = review(rev=rev, verbose=True)
+        assert '(%s)\n' % i not in output
+
+        review(rev=rev, comment=True, message='test2')
+        review(rev=rev, comment=True, message='test3')
+        i1, i2 = _get_identifiers(rev)
+
+        output = review(rev=rev, delete=True, files=[i1])
+        assert not output
+        output = review(rev=rev, verbose=True)
+        assert '(%s)\n' % i1 not in output
+        assert '(%s)\n' % i2 in output
+
+        review(rev=rev, comment=True, message='test4')
+        i1, i2 = _get_identifiers(rev)
+
+        output = review(rev=rev, delete=True, files=[i1, i2])
+        assert not output
+        output = review(rev=rev, verbose=True)
+        assert '(%s)\n' % i1 not in output
+        assert '(%s)\n' % i2 not in output
+    t('.')
+    t('0')
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_delete_signoff():
+    # TODO: test multiple signoff deletions
+    review(signoff=True, message='test')
+    i = _get_identifiers()[0]
+
+    output = review(delete=True, files=[i])
+    assert not output
+    output = review(verbose=True)
+    assert '(%s)\n' % i not in output
+
+    review(comment=True, message='test2')
+    review(signoff=True, message='test3')
+    i1, i2 = _get_identifiers()
+
+    output = review(delete=True, files=[i2])
+    assert not output
+    output = review(verbose=True)
+    assert '(%s)\n' % i1 in output
+    assert '(%s)\n' % i2 not in output
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_delete_both():
+    def t(rev):
+        review(rev=rev, signoff=True, message='test')
+        review(rev=rev, comment=True, message='test')
+        ids = _get_identifiers(rev)
+
+        output = review(rev=rev, delete=True, files=ids)
+        assert not output
+        output = review(rev=rev, verbose=True)
+        assert '(%s)\n' % ids[0] not in output
+        assert '(%s)\n' % ids[1] not in output
+
+        review(rev=rev, signoff=True, message='test2')
+        review(rev=rev, comment=True, message='test3')
+        review(rev=rev, comment=True, message='test4')
+        ids = _get_identifiers(rev)
+
+        output = review(rev=rev, delete=True, files=ids[:2])
+        assert not output
+        output = review(rev=rev, verbose=True)
+        assert '(%s)\n' % ids[0] not in output
+        assert '(%s)\n' % ids[1] not in output
+        assert '(%s)\n' % ids[2] in output
+    t('.')
+    t('0')
+
--- a/review/tests/util.py	Sat Jun 19 02:03:29 2010 -0400
+++ b/review/tests/util.py	Sat Jun 19 02:03:37 2010 -0400
@@ -12,7 +12,7 @@
 def review(init=False, comment=False, signoff=False, check=False, yes=False,
     no=False, force=False, message='', rev='.', remote_path='', lines='',
     files=None, unified='5', web=False, verbose=False, debug=False, mdown=False,
-    seen=False, yeses='', no_nos=False):
+    seen=False, yeses='', no_nos=False, delete=False):
 
     if not files:
         files = []
@@ -27,7 +27,7 @@
             init=init, comment=comment, signoff=signoff, check=check, yes=yes,
             no=no, force=force, message=message, rev=rev, remote_path=remote_path,
             lines=lines, unified=unified, web=web, mdown=mdown, seen=seen,
-            yeses=yeses, no_nos=no_nos
+            yeses=yeses, no_nos=no_nos, delete=delete
         )
     )
     _ui.verbose, _ui.debugflag = False, False