# HG changeset patch # User Steve Losh # Date 1269702612 14400 # Node ID 22de90ef33ede6f9ca55419879723cba20b6b517 # Parent b1448ebe2909600e1a199902a650ce3160c55b89 Add identifiers to ReviewChangeset's and ReviewSignoff's. The identifier of a rcset/rsignoff is the filename it was saved as, which is the hash of its contents. This will be useful when we want to add replies to comments/signoffs. CLI support for this feature has also been added (using --verbose and --debug flags with 'hg review [-r REV]'), as well as some simple unit tests. diff -r b1448ebe2909 -r 22de90ef33ed review/api.py --- a/review/api.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/api.py Sat Mar 27 11:10:12 2010 -0400 @@ -305,6 +305,7 @@ data['lines'] = data['lines'].split(',') data['lines'] = map(int, filter(None, data['lines'])) data['hgdate'] = util.parsedate(data['hgdate']) + data['identifier'] = _split_path_dammit(fn)[-1] self.comments.append(ReviewComment(**data)) self.comments.sort(key=operator.attrgetter('local_datetime')) @@ -312,6 +313,7 @@ for fn in signofffns: data = _parse_data(self.repo['tip'][fn].data()) data['hgdate'] = util.parsedate(data['hgdate']) + data['identifier'] = _split_path_dammit(fn)[-1] self.signoffs.append(ReviewSignoff(**data)) self.signoffs.sort(key=operator.attrgetter('local_datetime')) else: @@ -654,6 +656,7 @@ comment.lines comment.message comment.local_datetime + comment.identifier Each item is a string, except for lines, hgdate, and local_datetime. @@ -667,7 +670,7 @@ was added. """ - def __init__(self, author, hgdate, node, filename, lines, message, **extra): + def __init__(self, author, hgdate, node, filename, lines, message, identifier=None, **extra): """Initialize a ReviewComment. You shouldn't need to create these directly -- use a ReviewChangeset @@ -688,6 +691,7 @@ self.filename = filename self.lines = lines self.message = message + self.identifier = identifier def _render_data(self): """Render the data of this comment into a string for writing to disk. @@ -728,6 +732,7 @@ signoff.node signoff.opinion signoff.message + signoff.identifier Each item is a string, except for hgdate and local_datetime. @@ -739,7 +744,7 @@ was added. """ - def __init__(self, author, hgdate, node, opinion, message, **extra): + def __init__(self, author, hgdate, node, opinion, message, identifier=None, **extra): """Initialize a ReviewSignoff. You shouldn't need to create these directly -- use a ReviewChangeset @@ -760,6 +765,7 @@ self.node = node self.opinion = opinion self.message = message + self.identifier = identifier def _render_data(self): """Render the data of this signoff into a string for writing to disk. diff -r b1448ebe2909 -r 22de90ef33ed review/extension_ui.py --- a/review/extension_ui.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/extension_ui.py Sat Mar 27 11:10:12 2010 -0400 @@ -112,18 +112,35 @@ ) ui.write(messages.REVIEW_LOG_COMMENTS % (comment_count, author_count)) + def _build_item_header(item, author_template, author_extra=None): + author = templatefilters.person(item.author) + author_args = (author,) + if author_extra: + author_args = author_args + author_extra + author_part = author_template % author_args + + age = templatefilters.age(item.hgdate) + age_part = messages.REVIEW_LOG_AGE % age + if ui.debugflag: + hash_part = messages.REVIEW_LOG_IDENTIFIER % item.identifier + elif ui.verbose: + hash_part = messages.REVIEW_LOG_IDENTIFIER % item.identifier[:12] + else: + hash_part = '' + detail_part = age_part + hash_part + + spacing = 80 - (len(author_part) + len(detail_part)) + if spacing <= 0: + spacing = 1 + spacing = ' ' * spacing + + return author_part + spacing + detail_part + '\n' + + def _print_comment(comment, before='', after=''): ui.write(before) - - author = templatefilters.person(comment.author) - author_part = messages.REVIEW_LOG_COMMENT_AUTHOR % author + ui.write(_build_item_header(comment, messages.REVIEW_LOG_COMMENT_AUTHOR)) - age = templatefilters.age(comment.hgdate) - age_part = messages.REVIEW_LOG_AGE % age - - spacing = ' ' * (80 - (len(author_part) + len(age_part))) - - ui.write(author_part + spacing + age_part + '\n') for line in comment.message.splitlines(): ui.write(messages.REVIEW_LOG_COMMENT_LINE % line) @@ -132,16 +149,9 @@ def _print_signoff(signoff, before='', after=''): ui.write(before) - author = templatefilters.person(signoff.author) opinion = signoff.opinion or 'neutral' - author_part = messages.REVIEW_LOG_SIGNOFF_AUTHOR % (author, opinion) + ui.write(_build_item_header(signoff, messages.REVIEW_LOG_SIGNOFF_AUTHOR, (opinion,))) - age = templatefilters.age(signoff.hgdate) - age_part = messages.REVIEW_LOG_AGE % age - - spacing = ' ' * (80 - (len(author_part) + len(age_part))) - - ui.write(author_part + spacing + age_part + '\n') for line in signoff.message.splitlines(): ui.write(messages.REVIEW_LOG_SIGNOFF_LINE % line) @@ -342,6 +352,12 @@ If any FILEs are given, only those diffs will be shown. If --quiet is used no diffs will be shown. + If --verbose is used the short identifier of each comment/signoff will + also be shown. + + If --debug is used the full identifier of each comment/signoff will also + be shown. + """)), (['review-comment'], ('Adding code review comments for changesets'), diff -r b1448ebe2909 -r 22de90ef33ed review/messages.py --- a/review/messages.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/messages.py Sat Mar 27 11:10:12 2010 -0400 @@ -94,6 +94,7 @@ """ REVIEW_LOG_AGE = """(%s)""" +REVIEW_LOG_IDENTIFIER = """ (%s)""" REVIEW_LOG_FILE_HEADER = """changes in %s""" REVIEW_LOG_SKIPPED = """\ diff -r b1448ebe2909 -r 22de90ef33ed review/tests/test_comment.py --- a/review/tests/test_comment.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/tests/test_comment.py Sat Mar 27 11:10:12 2010 -0400 @@ -1,9 +1,10 @@ from nose import * from util import * -from .. import messages +from .. import api, messages import os from mercurial import util as hgutil +from mercurial.node import hex a1, a2 = (messages.REVIEW_LOG_COMMENT_AUTHOR % '|').split('|') @@ -223,3 +224,37 @@ assert a2 not in output assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.' not in output + +@with_setup(setup_reviewed_sandbox, teardown_sandbox) +def test_comment_identifiers(): + review(comment=True, message='Test comment one.', rev='1', files=['file_one']) + + rd = api.ReviewDatastore(get_ui(), get_sandbox_repo()) + dsr = get_datastore_repo() + + comment = rd['1'].comments[0] + + identifier = comment.identifier + short_identifier = identifier[:12] + + comment_filename = api._split_path_dammit(dsr['tip'].files()[0])[-1] + comment_cset = hex(dsr['tip'].node()) + + assert identifier == comment_filename + assert identifier != comment_cset + + verbose_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier[:12] + debug_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier + + normal_output = review(rev='1') + assert verbose_identifier not in normal_output + assert debug_identifier not in normal_output + + verbose_output = review(rev='1', verbose=True) + assert verbose_identifier in verbose_output + assert debug_identifier not in verbose_output + + debug_output = review(rev='1', debug=True) + assert verbose_identifier not in debug_output + assert debug_identifier in debug_output + diff -r b1448ebe2909 -r 22de90ef33ed review/tests/test_signoff.py --- a/review/tests/test_signoff.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/tests/test_signoff.py Sat Mar 27 11:10:12 2010 -0400 @@ -4,6 +4,7 @@ import os from mercurial import util as hgutil +from mercurial.node import hex s1, s2 = (messages.REVIEW_LOG_SIGNOFF_AUTHOR % ('|', 'neutral')).split('|') @@ -91,3 +92,37 @@ assert sn1 in output assert messages.REVIEW_LOG_SIGNOFF_LINE % 'Test signoff one.' in output + +@with_setup(setup_reviewed_sandbox, teardown_sandbox) +def test_signoff_identifiers(): + review(signoff=True, message='Test signoff one.', rev='0') + + rd = api.ReviewDatastore(get_ui(), get_sandbox_repo()) + dsr = get_datastore_repo() + + signoff = rd['0'].signoffs[0] + + identifier = signoff.identifier + short_identifier = identifier[:12] + + signoff_filename = api._split_path_dammit(dsr['tip'].files()[0])[-1] + signoff_cset = hex(dsr['tip'].node()) + + assert identifier == signoff_filename + assert identifier != signoff_cset + + verbose_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier[:12] + debug_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier + + normal_output = review(rev='0') + assert verbose_identifier not in normal_output + assert debug_identifier not in normal_output + + verbose_output = review(rev='0', verbose=True) + assert verbose_identifier in verbose_output + assert debug_identifier not in verbose_output + + debug_output = review(rev='0', debug=True) + assert verbose_identifier not in debug_output + assert debug_identifier in debug_output + diff -r b1448ebe2909 -r 22de90ef33ed review/tests/util.py --- a/review/tests/util.py Fri Mar 05 07:05:44 2010 -0500 +++ b/review/tests/util.py Sat Mar 27 11:10:12 2010 -0400 @@ -9,12 +9,16 @@ _ui = ui.ui() def review(init=False, comment=False, signoff=False, yes=False, no=False, force=False, message='', rev='.', remote_path='', lines='', files=None, - unified='5', web=False): + unified='5', web=False, verbose=False, debug=False): if not files: files = [] _ui.pushbuffer() + if debug: + _ui.debugflag = True + elif verbose: + _ui.verbose = True extension_ui.review( _ui, get_sandbox_repo(), *files, **dict( @@ -23,6 +27,7 @@ lines=lines, unified=unified, web=web ) ) + _ui.verbose, _ui.debugflag = False, False output = _ui.popbuffer() print output @@ -88,5 +93,9 @@ def clone_sandbox_repo(): hg.clone(cmdutil.remoteui(_ui, {}), sandbox_repo_path, sandbox_clone_path) -def get_datastore_repo(path): +def get_datastore_repo(path=api.DEFAULT_DATASTORE_DIRNAME): return hg.repository(_ui, path) + +def get_ui(): + return _ui +