--- a/review/cli.py Sat Jun 19 17:39:57 2010 -0400
+++ b/review/cli.py Sat Jun 19 17:40:05 2010 -0400
@@ -11,18 +11,58 @@
from mercurial.node import short
+def _get_datastore(ui, repo):
+ try:
+ return api.ReviewDatastore(ui, repo)
+ except api.UninitializedDatastore:
+ raise util.Abort(messages.NO_DATA_STORE)
+
def _get_message(ui, rd, initial):
message = ui.edit(initial, rd.repo.ui.username())
return '\n'.join(l for l in message.splitlines()
if not l.startswith('HG: ')).strip()
+def _edit_comment(rd, comment, *fnames, **opts):
+ lines = opts.pop('lines')
+ message = opts.pop('message').strip()
+ mdown = opts.pop('mdown')
+
+ if len(fnames) > 1:
+ raise util.Abort(messages.EDIT_REQUIRES_ONE_OR_LESS_FILES)
+
+ if not lines:
+ lines = comment.lines
+ else:
+ lines = [int(l.strip()) for l in lines.split(',')]
+
+ if not message:
+ message = comment.message
+
+ if not fnames:
+ fnames = [comment.filename]
+ else:
+ fnames = [api.sanitize_path(fnames[0])]
+
+ style = 'markdown' if mdown or comment.style == 'markdown' else ''
+
+ try:
+ rd.edit_comment(comment.identifier, message, fnames[0], lines, style)
+ except api.FileNotInChangeset:
+ raise util.Abort(messages.COMMENT_FILE_DOES_NOT_EXIST % (
+ fnames[0], rd.target[comment.node].rev()))
+
+def _edit_signoff(rd, signoff, **opts):
+ raise util.Abort('unimplemented, sorry')
+
+
def _web_command(ui, repo, **opts):
ui.note(messages.WEB_START)
read_only = opts.pop('read_only')
allow_anon = opts.pop('allow_anon')
address = opts.pop('address')
port = int(opts.pop('port'))
+ rd = _get_datastore(ui, repo)
import web
web.load_interface(ui, repo, read_only=read_only, allow_anon=allow_anon,
@@ -52,8 +92,8 @@
lines = opts.pop('lines')
message = opts.pop('message').strip()
mdown = opts.pop('mdown')
+ rd = _get_datastore(ui, repo)
- rd = api.ReviewDatastore(ui, repo)
rcset = rd[rev]
if lines and not len(fnames) == 1:
@@ -80,11 +120,11 @@
fn, repo[rev].rev()))
def _signoff_command(ui, repo, **opts):
- rd = api.ReviewDatastore(ui, repo)
- rcset = rd[opts.pop('rev')]
message = opts.pop('message').strip()
force = opts.pop('force')
mdown = opts.pop('mdown')
+ rd = _get_datastore(ui, repo)
+ rcset = rd[opts.pop('rev')]
yes, no = opts.pop('yes'), opts.pop('no')
if yes and no:
@@ -105,7 +145,7 @@
rcset.add_signoff(message=message, opinion=opinion, force=force, style=style)
def _check_command(ui, repo, **opts):
- rd = api.ReviewDatastore(ui, repo)
+ rd = _get_datastore(ui, repo)
rcset = rd[opts.pop('rev')]
if opts.pop('no_nos'):
@@ -127,11 +167,8 @@
def _review_command(ui, repo, *fnames, **opts):
rev = opts.pop('rev')
context = int(opts.pop('unified'))
+ rd = _get_datastore(ui, repo)
- try:
- rd = api.ReviewDatastore(ui, repo)
- except api.UninitializedDatastore:
- raise util.Abort(messages.NO_DATA_STORE)
cset = repo[rev]
rcset = rd[rev]
@@ -242,22 +279,37 @@
def _delete_command(ui, repo, *identifiers, **opts):
# TODO: require -f to delete some else's item
force = opts.pop('force')
+ rd = _get_datastore(ui, repo)
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)
+ raise util.Abort(messages.REQUIRES_IDS)
for i in identifiers:
try:
rd.remove_item(i)
except api.UnknownIdentifier:
- raise util.Abort(messages.DELETE_UNKNOWN_ID % i)
+ raise util.Abort(messages.UNKNOWN_ID % i)
except api.AmbiguousIdentifier:
- raise util.Abort(messages.DELETE_AMBIGUOUS_ID % i)
+ raise util.Abort(messages.AMBIGUOUS_ID % i)
+
+def _edit_command(ui, repo, *args, **opts):
+ # TODO: require -f to edit some else's item
+ # TODO: support forcing of plain text
+ force = opts.pop('force')
+ identifier = opts.pop('edit')
+ rd = _get_datastore(ui, repo)
+
+ items = rd.get_items(identifier)
+ if len(items) == 0:
+ raise util.Abort(messages.UNKNOWN_ID % identifier)
+ elif len(items) > 1:
+ raise util.Abort(messages.AMBIGUOUS_ID % identifier)
+ item = items[0]
+
+ if item.itemtype == 'comment':
+ _edit_comment(rd, item, *args, **opts)
+ elif item.itemtype == 'signoff':
+ _edit_signoff(rd, item, **opts)
_review_effects = {
@@ -350,6 +402,8 @@
return _signoff_command(ui, repo, **opts)
elif opts.pop('check'):
return _check_command(ui, repo, **opts)
+ elif opts.get('edit'):
+ return _edit_command(ui, repo, *args, **opts)
elif opts.pop('delete'):
return _delete_command(ui, repo, *args, **opts)
else:
@@ -364,6 +418,7 @@
('r', 'rev', '.', 'the revision to review'),
('d', 'delete', False, 'delete a comment or signoff'),
+ ('', 'edit', '', 'edit 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"'),
--- a/review/tests/sample_data.py Sat Jun 19 17:39:57 2010 -0400
+++ b/review/tests/sample_data.py Sat Jun 19 17:40:05 2010 -0400
@@ -3,11 +3,13 @@
'file_two': 'this is another test file',
'long_file': 'a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\no\np\nq\nr\ns\nt',
'always_changing': 'this\nfile\nalways\nchanges',
+ 'always_changing2': 'this\nfile\nalways\nchanges',
},
{ 'file_one': 'hello again\nworld\nfoo\nbar',
'file_three': 'this is a new file\nfor testing\npurposes\nonly',
'test_dir/test_file': 'this file is inside\nof a directory\n\nponies!',
'long_file': 'a\nb\nc\nd\ne\nf\nX\nh\ni\nj\nk\nl\nY\no\np\nq\nr\ns\nt',
- 'always_changing': 'this\nfile\nALWAYS\nchanges',
+ 'always_changing': 'THIS\nFILE\nALWAYS\nCHANGES',
+ 'always_changing2': 'THIS\nFILE\nALWAYS\nCHANGES',
},
-]
\ No newline at end of file
+]
--- a/review/tests/test_comment.py Sat Jun 19 17:39:57 2010 -0400
+++ b/review/tests/test_comment.py Sat Jun 19 17:40:05 2010 -0400
@@ -3,6 +3,7 @@
from nose import with_setup
from util import setup_reviewed_sandbox, teardown_sandbox, review, should_fail_with
from util import get_datastore_repo, get_sandbox_repo, get_ui
+from util import check_comment_exists_on_line
from .. import api, messages
@@ -135,12 +136,7 @@
assert a2 in output
assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.' in output
- # Make sure it's in the correct place
- output = output.splitlines()
- for n, line in enumerate(output):
- if line.startswith('#'):
- assert output[n-1].strip().startswith('1')
- break
+ check_comment_exists_on_line(1, files=['file_one'], rev='1')
@with_setup(setup_reviewed_sandbox, teardown_sandbox)
def test_add_comments_to_file_lines():
@@ -154,12 +150,7 @@
assert a2 in output
assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.' in output
- # Make sure it's in the correct place
- output = output.splitlines()
- for n, line in enumerate(output):
- if line.startswith('#'):
- assert output[n-1].strip().startswith('2')
- break
+ check_comment_exists_on_line(2, files=['file_one'], rev='1')
@with_setup(setup_reviewed_sandbox, teardown_sandbox)
def test_add_comments_to_file_in_subdir():
--- a/review/tests/test_delete.py Sat Jun 19 17:39:57 2010 -0400
+++ b/review/tests/test_delete.py Sat Jun 19 17:40:05 2010 -0400
@@ -31,7 +31,7 @@
review(rev=rev, comment=True, message='test')
i = get_identifiers(rev)[0]
- output = review(rev=rev, delete=True, args=[i])
+ output = review(delete=True, args=[i])
assert not output
output = review(rev=rev, verbose=True)
assert '(%s)\n' % i not in output
@@ -40,7 +40,7 @@
review(rev=rev, comment=True, message='test3')
i1, i2 = get_identifiers(rev)
- output = review(rev=rev, delete=True, args=[i1])
+ output = review(delete=True, args=[i1])
assert not output
output = review(rev=rev, verbose=True)
assert '(%s)\n' % i1 not in output
@@ -49,7 +49,7 @@
review(rev=rev, comment=True, message='test4')
i1, i2 = get_identifiers(rev)
- output = review(rev=rev, delete=True, args=[i1, i2])
+ output = review(delete=True, args=[i1, i2])
assert not output
output = review(rev=rev, verbose=True)
assert '(%s)\n' % i1 not in output
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/review/tests/test_edit.py Sat Jun 19 17:40:05 2010 -0400
@@ -0,0 +1,133 @@
+import time
+
+from nose import with_setup
+from util import setup_reviewed_sandbox, teardown_sandbox, review, should_fail_with
+from util import get_identifiers, check_comment_exists_on_line
+
+from .. import messages
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_edit_invalid():
+ should_fail_with(messages.UNKNOWN_ID % 'z', edit='z')
+
+ review(comment=True, message='test')
+
+ should_fail_with(messages.UNKNOWN_ID % 'z', edit='z')
+
+ # 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])
+
+ should_fail_with(messages.AMBIGUOUS_ID % i, edit=i)
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_touch_comment():
+ def t(rev):
+ review(rev=rev, comment=True, message='test', args=['always_changing'], lines='1')
+ i = get_identifiers(rev)[0]
+
+ # This sucks, but we need to do it to support testing "touch" edits.
+ time.sleep(1.1)
+
+ output = review(edit=i)
+ assert not output
+ output = review(rev=rev, verbose=True)
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev)) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' in output
+
+ check_comment_exists_on_line(1, files=['always_changing'], rev='1')
+ t('1')
+ t('0')
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_edit_comment_message():
+ def t(rev):
+ review(rev=rev, comment=True, message='test')
+ i = get_identifiers(rev)[0]
+
+ output = review(edit=i, message='edited')
+ assert not output
+ output = review(rev=rev, verbose=True)
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev)) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' not in output
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'edited' in output
+ t('.')
+ t('0')
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_edit_comment_lines():
+ def t(rev):
+ review(rev=rev, comment=True, message='test', args=['always_changing'], lines='1')
+ i = get_identifiers(rev)[0]
+
+ output = review(edit=i, lines='3')
+ assert not output
+ output = review(rev=rev, verbose=True)
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev)) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' in output
+ check_comment_exists_on_line(3, files=['always_changing'], rev=rev)
+
+ i = get_identifiers(rev)[0]
+
+ output = review(edit=i, lines='1,2')
+ assert not output
+ output = review(rev=rev, verbose=True)
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev)) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' in output
+ check_comment_exists_on_line(2, files=['always_changing'], rev=rev)
+ t('1')
+ t('0')
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_edit_comment_filename():
+ def t(rev):
+ review(rev=rev, comment=True, message='test', args=['always_changing'], lines='1')
+ i = get_identifiers(rev)[0]
+
+ output = review(edit=i, args=['always_changing2'])
+ assert not output
+
+ output = review(rev=rev, verbose=True, args=['always_changing'])
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev, files=['always_changing'])) == 0
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' not in output
+
+ output = review(rev=rev, verbose=True, args=['always_changing2'])
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev, files=['always_changing2'])) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' in output
+ t('1')
+ t('0')
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_edit_comment_everything():
+ def t(rev):
+ review(rev=rev, comment=True, message='test', args=['always_changing'], lines='1')
+ i = get_identifiers(rev)[0]
+
+ output = review(edit=i, args=['always_changing2'], message='edited', lines='2')
+ assert not output
+
+ output = review(rev=rev, verbose=True, args=['always_changing'])
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev, files=['always_changing'])) == 0
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' not in output
+
+ output = review(rev=rev, verbose=True, args=['always_changing2'])
+ assert '(%s)' % i not in output
+ assert len(get_identifiers(rev, files=['always_changing2'])) == 1
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'test' not in output
+ assert messages.REVIEW_LOG_COMMENT_LINE % 'edited' in output
+ check_comment_exists_on_line(2, files=['always_changing'], rev=rev)
+ t('1')
+ t('0')
--- a/review/tests/util.py Sat Jun 19 17:39:57 2010 -0400
+++ b/review/tests/util.py Sat Jun 19 17:40:05 2010 -0400
@@ -134,7 +134,24 @@
a1, a2 = (messages.REVIEW_LOG_COMMENT_AUTHOR % '|').split('|')
s1, s2 = (messages.REVIEW_LOG_SIGNOFF_AUTHOR % ('|', 'neutral')).split('|')
-def get_identifiers(rev='.'):
+def get_identifiers(rev='.', files=[]):
return [l.split(' ')[-1].strip('()\n')
- for l in review(rev=rev, verbose=True).splitlines()
+ for l in review(rev=rev, verbose=True, args=files).splitlines()
if (a1 in l and a2 in l) or (s1 in l and s2 in l)]
+
+
+COMMENT_LINE_ERROR = '''\
+Expected a comment on line %d:
+
+%s
+%s
+'''
+def check_comment_exists_on_line(n, files=[], rev='.'):
+ output = review(rev=rev, args=files).splitlines()
+ for i, line in enumerate(output):
+ if line.startswith('#'):
+ assert output[i-1].strip().startswith(str(n)), \
+ COMMENT_LINE_ERROR % (n, output[i-1].rstrip(),
+ output[i].rstrip())
+ break
+