# HG changeset patch # User Steve Losh # Date 1276644195 14400 # Node ID 30864e67432fcfed028db50c0b47beeb242667e6 # Parent 9307f4023849bb805eb407c6a67fb72f21e9a09e# Parent d5280b38dd4c0908a9c573dd535ea81891fdb00f merge with default diff -r 9307f4023849 -r 30864e67432f review/extension_ui.py --- a/review/extension_ui.py Tue Jun 15 00:21:03 2010 -0400 +++ b/review/extension_ui.py Tue Jun 15 19:23:15 2010 -0400 @@ -6,12 +6,18 @@ """ import re -import helps, messages -from api import * +import api, helps, messages from mercurial import help, templatefilters, util from mercurial.node import short from mercurial import extensions + +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 _web_command(ui, repo, **opts): ui.note(messages.WEB_START) read_only = opts.pop('read_only') @@ -27,16 +33,16 @@ ui.note(messages.INIT_START) try: - ReviewDatastore(ui, repo, rpath=opts.pop('remote_path'), create=True) + api.ReviewDatastore(ui, repo, rpath=opts.pop('remote_path'), create=True) if '.hgreview' not in repo['tip'].manifest(): ui.status(messages.INIT_SUCCESS_UNCOMMITTED) else: ui.status(messages.INIT_SUCCESS_CLONED) - except RelativeRemotePath: + except api.RelativeRemotePath: raise util.Abort(messages.INIT_UNSUPPORTED_RELATIVE_RPATH) - except DatastoreRequiresRemotePath: + except api.DatastoreRequiresRemotePath: raise util.Abort(messages.INIT_REQUIRES_REMOTE_PATH) - except PreexistingDatastore, e: + except api.PreexistingDatastore, e: if e.committed: ui.note(messages.INIT_EXISTS) else: @@ -44,55 +50,55 @@ def _comment_command(ui, repo, *fnames, **opts): rev = opts.pop('rev') - message = opts.pop('message') lines = opts.pop('lines') + message = opts.pop('message').strip() - rd = ReviewDatastore(ui, repo) + rd = api.ReviewDatastore(ui, repo) rcset = rd[rev] if lines and not len(fnames) == 1: raise util.Abort(messages.COMMENT_LINES_REQUIRE_FILE) - if not message: - raise util.Abort(messages.COMMENT_REQUIRES_MESSAGE) + if lines: + lines = lines.split(',') + + fnames = map(lambda f: api.sanitize_path(f, repo), fnames) if fnames else [''] - if lines: - lines=lines.split(',') - - if fnames: - fnames = map(lambda f: sanitize_path(f, repo), fnames) - else: - fnames = [''] + if not message: + message = _get_message(ui, rd, messages.COMMENT_EDITOR_MESSAGE) + if not message: + raise util.Abort(messages.COMMENT_REQUIRES_MESSAGE) for fn in fnames: try: rcset.add_comment(message=message, filename=fn, lines=lines) - except FileNotInChangeset: - raise util.Abort( - messages.COMMENT_FILE_DOES_NOT_EXIST % (fn, repo[rev].rev()) - ) + except api.FileNotInChangeset: + raise util.Abort(messages.COMMENT_FILE_DOES_NOT_EXIST % ( + fn, repo[rev].rev())) def _signoff_command(ui, repo, **opts): - rd = ReviewDatastore(ui, repo) + rd = api.ReviewDatastore(ui, repo) rcset = rd[opts.pop('rev')] - message = opts.pop('message') - - if not message: - raise util.Abort(messages.SIGNOFF_REQUIRES_MESSAGE) + message = opts.pop('message').strip() + force = opts.pop('force') yes, no = opts.pop('yes'), opts.pop('no') if yes and no: raise util.Abort(messages.SIGNOFF_OPINION_CONFLICT) opinion = 'yes' if yes else ('no' if no else '') - try: - rcset.add_signoff(message=message, opinion=opinion, - force=opts.pop('force')) - except SignoffExists: + if rcset.signoffs_for_current_user() and not force: raise util.Abort(messages.SIGNOFF_EXISTS) + if not message: + message = _get_message(ui, rd, messages.SIGNOFF_EDITOR_MESSAGE) + if not message: + raise util.Abort(messages.SIGNOFF_REQUIRES_MESSAGE) + + rcset.add_signoff(message=message, opinion=opinion, force=force) + def _check_command(ui, repo, **opts): - rd = ReviewDatastore(ui, repo) + rd = api.ReviewDatastore(ui, repo) rcset = rd[opts.pop('rev')] if opts.pop('no_nos'): @@ -116,8 +122,8 @@ context = int(opts.pop('unified')) try: - rd = ReviewDatastore(ui, repo) - except UninitializedDatastore: + rd = api.ReviewDatastore(ui, repo) + except api.UninitializedDatastore: raise util.Abort(messages.NO_DATA_STORE) cset = repo[rev] rcset = rd[rev] @@ -201,7 +207,7 @@ if not fnames: fnames = rcset.files() - fnames = [sanitize_path(fname, repo) for fname in fnames] + fnames = [api.sanitize_path(fname, repo) for fname in fnames] fnames = [fname for fname in fnames if rcset.has_diff(fname)] for filename in fnames: @@ -251,8 +257,9 @@ for r, style in _review_re: m = r.match(line) if m: - lines[i] = "%s%s" % (m.groupdict().get('rest', ''), - render_effects(m.group('colorized'), _review_effects[style])) + lines[i] = "%s%s" % ( + m.groupdict().get('rest', ''), + render_effects(m.group('colorized'), _review_effects[style])) break return '\n'.join(lines) @@ -287,23 +294,23 @@ """code review changesets in the current repository To start using the review extension with a repository, you need to - initialize the code review data: + initialize the code review data:: hg help review-init - Once you've initialized it (and cloned the review data repo to a - place where others can get to it), you can start reviewing changesets. + Once you've initialized it (and cloned the review data repo to a place + where others can get to it) you can start reviewing changesets. See the following help topics if you want to use the command-line interface: - hg help review-review - hg help review-comment - hg help review-signoff - hg help review-check + - hg help review-review + - hg help review-comment + - hg help review-signoff + - hg help review-check - Once you've reviewed some changesets, don't forget to push your - comments and signoffs so other people can view them. + Once you've reviewed some changesets don't forget to push your comments and + signoffs so other people can view them. """ if opts.pop('web'): @@ -340,7 +347,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'), - ('', 'force', False, 'overwrite an existing signoff'), + ('f', 'force', False, 'overwrite an existing signoff'), ('w', 'web', False, 'launch the web interface'), ('', 'read-only', False, 'make the web interface read-only'), diff -r 9307f4023849 -r 30864e67432f review/helps.py --- a/review/helps.py Tue Jun 15 00:21:03 2010 -0400 +++ b/review/helps.py Tue Jun 15 19:23:15 2010 -0400 @@ -5,28 +5,26 @@ """ INIT = r""" -hg review --init --remote-path PATH - -Initialize code review for the current repository. +USAGE: hg review --init --remote-path PATH When run for the first time in a project, it will do two things: - * Create a new Mercurial repository to hold the review data at .hg/review/ +- Create a new Mercurial repository to hold the review data at ``.hg/review/`` - * Create and 'hg add' a .hgreview file in the current repository. You - will need to commit this file yourself with: - hg commmit .hgreview -m 'Initialize code review data.' +- Create and ``hg add`` a .hgreview file in the current repository. You will + need to commit this file yourself with: ``hg commmit .hgreview -m 'initialize + code review data'`` This repository contains code review data such as comments and signoffs. It is a normal Mercurial repository, so you can push and pull review data to and from other clones of it to share your comments and signoffs. -The --remote-path option is required, and specifies the path where the +The ``--remote-path`` option is required and specifies the path where the canonical code review data for this project will live. This is the path -that will be cloned when someone else runs 'hg review --init' on the +that will be cloned when someone else runs ``hg review --init`` on the project. -Examples: +Examples:: hg review --init --remote-path 'http://bitbucket.org/u/project-review' hg review --init --remote-path '../project-review' @@ -34,44 +32,43 @@ """ REVIEW = r""" -hg review [-r REV] [-U CONTEXT] [--quiet] [FILE] +USAGE: hg review [-r REV] [-U CONTEXT] [--quiet] [FILE] -Show code review information about a specific revision. Diffs of all -changed files will be shown, and the line numbers printed are the ones -that should be used with 'hg review --comment --lines LINES FILE'. +If no revision is given the current parent of the working directory will be +used. -If no revision is given, the current parent of the working directory -will be shown. +Diffs of all changed files will be shown with comments inline. The line numbers +printed are the ones that should be used with ``hg review --comment --lines +LINES FILE``. -The number of lines of context in diffs can be changed with the -U option. -If any FILEs are given, only those diffs will be shown. If --quiet is used +The number of lines of context in diffs can be changed with the ``-U`` option. +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 ``--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. +If ``--debug`` is used the full identifier of each comment/signoff will also be +shown. """ COMMENT = r""" -hg review --comment -m MESSAGE [-r REV] [-l LINES] [FILE] +USAGE: hg review --comment -m MESSAGE [-r REV] [-l LINES] [FILE] -If no revision is given, the current parent of the working directory -will be used. +If no revision is given the current parent of the working directory will be +used. -If no FILEs are given, the comment will be attached to the changeset -as a whole. +If no FILEs are given the comment will be attached to the changeset as a whole. -If one or more FILEs are given but no LINES are given, the comment will -be attached to the each file as a whole. +If one or more FILEs are given but no LINES are given, the comment will be +attached to the each file as a whole. -If a FILE is given and LINES is given the comment will be attached to -those specific lines. LINES should be a comma-separated list of line -numbers (as numbered in the output of 'hg review'), such as '3' or '2,3' +If a FILE is given and LINES are given the comment will be attached to those +specific lines. LINES should be a comma-separated list of line numbers (as +numbered in the output of ``hg review``), such as ``3`` or ``2,3``. -Examples: +Examples:: hg review --comment -m 'This changeset needs to go in branch X.' hg review --comment -m 'This file should just be deleted.' script.py @@ -80,20 +77,20 @@ """ SIGNOFF = r""" -hg review --signoff -m MESSAGE [--yes | --no] [-r REV] [--force] +USAGE: hg review --signoff -m MESSAGE [--yes | --no] [-r REV] [--force] -If no revision is given, the current parent of the working directory -will be used. +If no revision is given the current parent of the working directory will be +used. -The --yes and --no options can be used to indicate whether you think +The ``--yes`` and ``--no`` options can be used to indicate whether you think the changeset is "good" or "bad". It's up to the collaborators of each -individual project to decide exactly what that means. If neither option -is given the signoff will be marked as "neutral". +individual project to decide exactly what that means. If neither option is +given the signoff will be marked as "neutral". -If you've already signed off on a changeset, you can use --force to -overwrite your previous signoff with a new one. +If you've already signed off on a changeset you can use ``--force`` to replace +your previous signoff with a new one. -Examples: +Examples:: hg review --signoff -m 'I do not work on this part of the code.' hg review --signoff --yes -m 'Thanks, this change looks good.' @@ -103,10 +100,10 @@ """ CHECK = r""" -hg review --check [-r REV] [--no-nos] [--yeses NUM] [--seen] +USAGE: hg review --check [-r REV] [--no-nos] [--yeses NUM] [--seen] -If no revision is given, the current parent of the working directory -will be used. +If no revision is given the current parent of the working directory will be +used. Check that the changeset "passes" the given tests of review status. If no tests are given an error is returned. @@ -117,13 +114,13 @@ The following tests are available: - --no-nos ensures that there are not any "no" signoffs on the changeset. +- ``--no-nos`` ensures that there are not any "no" signoffs on the changeset. - --yeses NUM ensures that at least NUM people have signed off as "yes" on - the changeset. +- ``--yeses NUM`` ensures that at least NUM people have signed off as "yes" on + the changeset. - --seen ensures that someone has commented or signed off on the changeset, - and can be used to make sure someone has at least taken a look at it. Note - that a signoff of "no" will make this test succeed. +- ``--seen`` ensures that someone has commented or signed off on the changeset, + and can be used to make sure someone has at least taken a look at it. Note + that a signoff of "no" will make this test succeed. """ diff -r 9307f4023849 -r 30864e67432f review/messages.py --- a/review/messages.py Tue Jun 15 00:21:03 2010 -0400 +++ b/review/messages.py Tue Jun 15 19:23:15 2010 -0400 @@ -41,7 +41,7 @@ """ COMMENT_REQUIRES_MESSAGE = """\ -a message must be provided to add a comment! +empty comment """ COMMENT_FILE_DOES_NOT_EXIST = """\ @@ -52,8 +52,15 @@ you must give a filename to comment on specific lines! """ +COMMENT_EDITOR_MESSAGE = """\ + + +HG: Enter your comment. Lines beginning with 'HG:' are removed. +HG: Leave comment empty to abort comment.\ +""" + SIGNOFF_REQUIRES_MESSAGE = """\ -a message must be provided to sign off! +empty signoff message """ SIGNOFF_OPINION_CONFLICT = """\ @@ -61,7 +68,14 @@ """ SIGNOFF_EXISTS = """\ -you have already signed off on this changeset (use --force to overwrite)! +you have already signed off on this changeset (use -f to replace)! +""" + +SIGNOFF_EDITOR_MESSAGE = """\ + + +HG: Enter your signoff message. Lines beginning with 'HG:' are removed. +HG: Leave message empty to abort signoff.\ """ REVIEW_LOG_CSET = """\ diff -r 9307f4023849 -r 30864e67432f review/tests/test_comment.py --- a/review/tests/test_comment.py Tue Jun 15 00:21:03 2010 -0400 +++ b/review/tests/test_comment.py Tue Jun 15 19:23:15 2010 -0400 @@ -13,15 +13,45 @@ output = review() assert messages.REVIEW_LOG_COMMENTS % (0, 0) in output + +# TODO: Figure out how to handle external editors nicely with nose. +#@with_setup(setup_reviewed_sandbox, teardown_sandbox) +#def test_blank_comment(): + #try: + #review(comment=True, message=' \t\n') + #except hgutil.Abort, e: + #error = str(e) + #assert messages.COMMENT_REQUIRES_MESSAGE in error + #else: + #assert False, 'The correct error message was not printed.' + + #try: + #review(comment=True, message=messages.COMMENT_EDITOR_MESSAGE) + #except hgutil.Abort, e: + #error = str(e) + #assert messages.COMMENT_REQUIRES_MESSAGE in error + #else: + #assert False, 'The correct error message was not printed.' + @with_setup(setup_reviewed_sandbox, teardown_sandbox) -def test_blank_comment(): - try: - review(comment=True) - except hgutil.Abort, e: - error = str(e) - assert messages.COMMENT_REQUIRES_MESSAGE in error - else: - assert False, 'The correct error message was not printed.' +def test_comment_formatting(): + review(comment=True, message=' \tTest comment one.\t ') + output = review() + + assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.' in output + assert messages.REVIEW_LOG_COMMENT_LINE % ' \tTest comment one.' not in output + assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.\t ' not in output + assert messages.REVIEW_LOG_COMMENT_LINE % ' \tTest comment one.\t ' not in output + review(rev=0, comment=True, + message=' \tTest\n indented\n\ttabindented\noutdented \ndone\t ') + output = review(rev=0) + + assert messages.REVIEW_LOG_COMMENT_LINE % 'Test' in output + assert messages.REVIEW_LOG_COMMENT_LINE % ' indented' in output + assert messages.REVIEW_LOG_COMMENT_LINE % '\ttabindented' in output + assert messages.REVIEW_LOG_COMMENT_LINE % 'outdented ' in output + assert messages.REVIEW_LOG_COMMENT_LINE % 'done' in output + @with_setup(setup_reviewed_sandbox, teardown_sandbox) def test_add_comments_to_parent_rev(): diff -r 9307f4023849 -r 30864e67432f review/tests/test_signoff.py --- a/review/tests/test_signoff.py Tue Jun 15 00:21:03 2010 -0400 +++ b/review/tests/test_signoff.py Tue Jun 15 19:23:15 2010 -0400 @@ -13,15 +13,44 @@ output = review() assert messages.REVIEW_LOG_SIGNOFFS % (0, 0, 0, 0) in output +# TODO: Figure out how to handle external editors nicely with nose. +#@with_setup(setup_reviewed_sandbox, teardown_sandbox) +#def test_blank_signoff(): + #try: + #review(signoff=True, message=' \t\n') + #except hgutil.Abort, e: + #error = str(e) + #assert messages.SIGNOFF_REQUIRES_MESSAGE in error + #else: + #assert False, 'The correct error message was not printed.' + + #try: + #review(signoff=True, message=messages.SIGNOFF_EDITOR_MESSAGE) + #except hgutil.Abort, e: + #error = str(e) + #assert messages.SIGNOFF_REQUIRES_MESSAGE in error + #else: + #assert False, 'The correct error message was not printed.' + @with_setup(setup_reviewed_sandbox, teardown_sandbox) -def test_blank_signoff(): - try: - review(signoff=True) - except hgutil.Abort, e: - error = str(e) - assert messages.SIGNOFF_REQUIRES_MESSAGE in error - else: - assert False, 'The correct error message was not printed.' +def test_signoff_formatting(): + review(signoff=True, message=' \tTest signoff one.\t ') + output = review() + + assert messages.REVIEW_LOG_SIGNOFF_LINE % 'Test signoff one.' in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % ' \tTest signoff one.' not in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % 'Test signoff one.\t ' not in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % ' \tTest signoff one.\t ' not in output + review(rev=0, signoff=True, + message=' \tTest\n indented\n\ttabindented\noutdented \ndone\t ') + output = review(rev=0) + + assert messages.REVIEW_LOG_SIGNOFF_LINE % 'Test' in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % ' indented' in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % '\ttabindented' in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % 'outdented ' in output + assert messages.REVIEW_LOG_SIGNOFF_LINE % 'done' in output + @with_setup(setup_reviewed_sandbox, teardown_sandbox) def test_signoff_on_parent_rev():