# HG changeset patch # User Steve Losh # Date 1254871667 14400 # Node ID 34caeeba9ae2c0bbb8e5cb4d69e7a9ce1a2c40cc # Parent 1ef154bb1a4fb54d8612ce4bc257b317dadd788f Add line-level commenting. diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/api.py --- a/review/api.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/api.py Tue Oct 06 19:27:47 2009 -0400 @@ -106,14 +106,17 @@ commentfns = filter(_match('%s/comments' % node), relevant) signofffns = filter(_match('%s/signoffs' % node), relevant) - self.comments = [ - ReviewComment(**_parse_data(self.repo['tip'][fn].data())) - for fn in commentfns - ] - self.signoffs = [ - ReviewSignoff(**_parse_data(self.repo['tip'][fn].data())) - for fn in signofffns - ] + self.comments = [] + for fn in commentfns: + data = _parse_data(self.repo['tip'][fn].data()) + data['lines'] = data['lines'].split(',') + data['lines'] = map(int, filter(None, data['lines'])) + self.comments.append(ReviewComment(**data)) + + self.signoffs = [] + for fn in signofffns: + data = _parse_data(self.repo['tip'][fn].data()) + self.signoffs.append(ReviewSignoff(**data)) else: self.comments = [] self.signoffs = [] @@ -235,6 +238,17 @@ return templates.COMMENT_FILE_TEMPLATE % ( self.author, datetime, self.node, self.filename, lines, self.message ) + def __str__(self): + return '\n'.join(map(str, [ + self.author, + self.datetime, + self.node, + self.filename, + self.lines, + self.message, + '\n', + ])) + class ReviewSignoff(_ReviewObject): '''A single review signoff.''' diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/extension_ui.py --- a/review/extension_ui.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/extension_ui.py Tue Oct 06 19:27:47 2009 -0400 @@ -6,6 +6,7 @@ from mercurial import util from mercurial.node import short + def _init_command(ui, repo, **opts): ui.note(messages.INIT_START) try: @@ -23,6 +24,7 @@ rev = opts.pop('rev') filename = fnames[0] if fnames else '' message = opts.pop('message') + lines = opts.pop('lines') rd = ReviewDatastore(ui, repo) rcset = rd[rev] @@ -35,10 +37,13 @@ messages.COMMENT_FILE_DOES_NOT_EXIST % (filename, repo[rev].rev()) ) + if lines and not filename: + raise util.Abort(messages.COMMENT_LINES_REQUIRE_FILE) + if not message: raise util.Abort(messages.COMMENT_REQUIRES_MESSAGE) - rcset.add_comment(message=message, filename=filename) + rcset.add_comment(message=message, filename=filename, lines=lines.split(',')) def _signoff_command(ui, repo, **opts): rd = ReviewDatastore(ui, repo) @@ -76,13 +81,18 @@ ui.write(messages.REVIEW_LOG_SIGNOFFS % len(rcset.signoffs)) ui.write(messages.REVIEW_LOG_COMMENTS % (comment_count, author_count)) + def _print_comment(comment, before='', after=''): + ui.write(before) + ui.write(messages.REVIEW_LOG_COMMENT_AUTHOR % comment.author) + for line in comment.message.splitlines(): + ui.write(messages.REVIEW_LOG_COMMENT_LINE % line) + ui.write(after) + review_level_comments = filter(lambda c: not c.filename, rcset.comments) if review_level_comments: ui.write('\n') for comment in review_level_comments: - ui.write('\n' + messages.REVIEW_LOG_COMMENT_AUTHOR % comment.author) - for line in comment.message.splitlines(): - ui.write(messages.REVIEW_LOG_COMMENT_LINE % line) + _print_comment(comment, before='\n') diffs = rcset.full_diffs(fnames, opts) @@ -90,18 +100,26 @@ header = messages.REVIEW_LOG_FILE_HEADER % filename print '\n\n%s %s' % (header, '-'*(80-(len(header)+1))) - file_level_comments = filter(lambda c: filename == c.filename, rcset.comments) + file_level_comments = filter( + lambda c: filename == c.filename and not c.lines, rcset.comments + ) for comment in file_level_comments: - ui.write(messages.REVIEW_LOG_COMMENT_AUTHOR % comment.author) - for line in comment.message.splitlines(): - ui.write(messages.REVIEW_LOG_COMMENT_LINE % line) - + _print_comment(comment) content = diffs[filename].splitlines()[2:] + line_level_comments = filter( + lambda c: filename == c.filename and c.lines, rcset.comments + ) prefix = '%%%dd: ' % len(str(len(content)-1)) for n, line in enumerate(content): ui.write('%s %s\n' % (prefix % n, line)) + line_comments = filter( + lambda c: max(c.lines) == n, line_level_comments + ) + for comment in line_comments: + _print_comment(comment) + def review(ui, repo, *fnames, **opts): @@ -130,6 +148,7 @@ ('', 'force', False, 'overwrite an existing signoff'), ('f', 'file', '', 'comment on '), ('r', 'rev', '.', 'the revision to review'), + ('l', 'lines', '', 'the line(s) of the file to comment on'), ], 'hg review') } \ No newline at end of file diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/messages.py --- a/review/messages.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/messages.py Tue Oct 06 19:27:47 2009 -0400 @@ -26,6 +26,10 @@ file %s was not changed in revision %s! ''' +COMMENT_LINES_REQUIRE_FILE = '''\ +you must give a filename to comment on specific lines! +''' + SIGNOFF_REQUIRES_MESSAGE = '''\ a message must be provided to sign off! ''' diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/tests/sample_data.py --- a/review/tests/sample_data.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/tests/sample_data.py Tue Oct 06 19:27:47 2009 -0400 @@ -2,7 +2,7 @@ { 'file_one': 'hello\nworld', 'file_two': 'this is another test file', }, - { 'file_one': 'hello again\nworld', + { 'file_one': 'hello again\nworld\nfoo\nbar', 'file_three': 'this is a new file\nfor testing', }, ] \ No newline at end of file diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/tests/test_comment.py --- a/review/tests/test_comment.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/tests/test_comment.py Tue Oct 06 19:27:47 2009 -0400 @@ -115,3 +115,61 @@ assert False, 'The correct error message was not printed.' + +@with_setup(setup_reviewed_sandbox, teardown_sandbox) +def test_add_comments_to_file_line(): + sandbox = get_sandbox_repo() + + try: + review(comment=True, rev='1', message='Test bad comment.', lines='1') + except hgutil.Abort, e: + error = str(e) + assert messages.COMMENT_LINES_REQUIRE_FILE in error + else: + assert False, 'The correct error message was not printed.' + + author_line = messages.REVIEW_LOG_COMMENT_AUTHOR % '|' + a1, _, a2 = author_line.partition('|') + + review(comment=True, rev='1', message='Test comment one.', + files=['file_one'], lines='1') + + output = review(rev='1', files=['file_one']) + + # Make sure the comment is present at all. + assert a1 in output + 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 + + +@with_setup(setup_reviewed_sandbox, teardown_sandbox) +def test_add_comments_to_file_lines(): + sandbox = get_sandbox_repo() + + author_line = messages.REVIEW_LOG_COMMENT_AUTHOR % '|' + a1, _, a2 = author_line.partition('|') + + review(comment=True, rev='1', message='Test comment one.', + files=['file_one'], lines='1,2') + + output = review(rev='1', files=['file_one']) + + # Make sure the comment is present at all. + assert a1 in output + 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 + diff -r 1ef154bb1a4f -r 34caeeba9ae2 review/tests/util.py --- a/review/tests/util.py Mon Oct 05 20:50:06 2009 -0400 +++ b/review/tests/util.py Tue Oct 06 19:27:47 2009 -0400 @@ -6,7 +6,8 @@ _ui = ui.ui() def review(init=False, comment=False, signoff=False, yes=False, no=False, - force=False, message='', rev='.', local_path='', remote_path='', files=None): + force=False, message='', rev='.', local_path='', remote_path='', lines='', + files=None): files = files if files else [] @@ -14,7 +15,7 @@ extension_ui.review(_ui, get_sandbox_repo(), *files, init=init, comment=comment, signoff=signoff, yes=yes, no=no, force=force, message=message, rev=rev, - local_path=local_path, remote_path=remote_path ) + local_path=local_path, remote_path=remote_path, lines=lines ) output = _ui.popbuffer() print output