# HG changeset patch # User Steve Losh # Date 1276927417 14400 # Node ID e29d33ec47b19286693a9fce05c7767414b07db4 # Parent 537000a18d8652aa3c9698fb9ad6f4c5fdc56feb cli: add support for deleting items diff -r 537000a18d86 -r e29d33ec47b1 review/cli.py --- 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'), diff -r 537000a18d86 -r e29d33ec47b1 review/tests/test_delete.py --- /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') + diff -r 537000a18d86 -r e29d33ec47b1 review/tests/util.py --- 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