# HG changeset patch # User Steve Losh # Date 1265474196 18000 # Node ID c5195b16e7e4cce43be961647ada1b6a004baf7e # Parent b599ca22418dc1a5d7f268877c996329a50c37d4# Parent 649e19f4c2e8b8324dd7f69d9250ee8470920fcd Merge the branch closing. diff -r b599ca22418d -r c5195b16e7e4 README --- a/README Thu Oct 22 18:27:31 2009 -0400 +++ b/README Sat Feb 06 11:36:36 2010 -0500 @@ -2,10 +2,12 @@ This is a work in progress. I wouldn't try it out on anything important... +You can look at some [screenshots](http://www.flickr.com/photos/sjl7678/sets/72157622593551064/), but be warned that they might be a little out of date! + Installing ========== -`hg-review` requires Mercurial (probably 1.3.1+) and **Python 2.6+**. Seriously, you need the latest version of Python. I'm not going to rewrite `os.relpath()` to support older versions, sorry. +`hg-review` requires Mercurial (probably 1.3.1+). Get it: @@ -26,6 +28,7 @@ If you want to get a quick taste of how it works: cd path/to/wherever/you/cloned/hg-review + hg update webui hg review --init hg review --web diff -r b599ca22418d -r c5195b16e7e4 bundled/webpy/web/utils.py --- a/bundled/webpy/web/utils.py Thu Oct 22 18:27:31 2009 -0400 +++ b/bundled/webpy/web/utils.py Sat Feb 06 11:36:36 2010 -0500 @@ -1,3 +1,4 @@ + #!/usr/bin/env python """ General Utilities diff -r b599ca22418d -r c5195b16e7e4 review/api.py --- a/review/api.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/api.py Sat Feb 06 11:36:36 2010 -0500 @@ -6,8 +6,31 @@ import file_templates, messages from mercurial import cmdutil, error, hg, patch, util from mercurial.node import hex +from mercurial import ui as _ui +try: + from os.path import relpath +except ImportError: # python < 2.6 + from os.path import curdir, abspath, sep, commonprefix, pardir, join + def relpath(path, start=curdir): + """Return a relative version of a path""" + + if not path: + raise ValueError("no path specified") + + start_list = abspath(start).split(sep) + path_list = abspath(path).split(sep) + + # Work out how much of the filepath is shared by start and path. + i = len(commonprefix([start_list, path_list])) + + rel_list = [pardir] * (len(start_list)-i) + path_list[i:] + if not rel_list: + return curdir + return join(*rel_list) + + DEFAULT_DATASTORE_DIRNAME = 'code-review' class PreexistingDatastore(Exception): @@ -156,7 +179,7 @@ """ if repo: - p = os.path.relpath(os.path.realpath(p), start=repo.root) + p = relpath(os.path.realpath(p), start=repo.root) return '/'.join(_split_path_dammit(p)) @@ -191,7 +214,7 @@ datastore_root = os.path.join(self.target.root, self.lpath) try: - self.repo = hg.repository(ui, datastore_root) + self.repo = hg.repository(_ui.ui(), datastore_root) except error.RepoError: raise UninitializedDatastore(True) elif '.hgreview' in repo['tip']: @@ -568,7 +591,7 @@ return '\n'.join(map(str, [ self.author, - self.datetime, + self.hgdate, self.node, self.filename, self.lines, diff -r b599ca22418d -r c5195b16e7e4 review/extension_ui.py --- a/review/extension_ui.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/extension_ui.py Sat Feb 06 11:36:36 2010 -0500 @@ -6,16 +6,18 @@ """ import operator, os +import re import messages from api import * from mercurial import help, templatefilters, util from mercurial.node import short +from mercurial import commands, extensions def _web_command(ui, repo, **opts): ui.note(messages.WEB_START) import web_ui - web_ui.load_interface(ui, repo) + web_ui.load_interface(ui, repo, open=True) def _init_command(ui, repo, **opts): ui.note(messages.INIT_START) @@ -207,7 +209,57 @@ if previous_n < max_line: skipped = max_line - previous_n ui.write(messages.REVIEW_LOG_SKIPPED % skipped) + skipped_comments = filter( + lambda c: max(c.lines) in range(previous_n + 1, max_line), + line_level_comments + ) + for comment in skipped_comments: + _print_comment(comment) + + +_review_effects = {'deleted': ['red'], + 'inserted': ['green'], + 'comments': ['cyan'],} + +_review_re = [(re.compile(r'^(?P *\d+: )(?P[-].*)'), 'deleted'), + (re.compile(r'^(?P *\d+: )(?P[+].*)'), 'inserted'), + (re.compile(r'^(?P#.*)'), 'comments'),] + +def colorwrap(orig, *args): + '''wrap ui.write for colored diff output''' + def _colorize(s): + lines = s.split('\n') + for i, line in enumerate(lines): + if not line: + continue + else: + 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])) + break + return '\n'.join(lines) + orig(*[_colorize(s) for s in args]) + +def colorreview(orig, ui, repo, *fnames, **opts): + '''colorize review command output''' + oldwrite = extensions.wrapfunction(ui, 'write', colorwrap) + try: + orig(ui, repo, *fnames, **opts) + finally: + ui.write = oldwrite + +def extsetup(ui): + try: + color = extensions.find('color') + color._setupcmd(ui, 'review', cmdtable, colorreview, + _review_effects) + global render_effects + render_effects = color.render_effects + except KeyError: + pass def review(ui, repo, *fnames, **opts): @@ -363,4 +415,4 @@ hg review --signoff --yes --force -m 'Nevermind, this is fine.' """)), -) \ No newline at end of file +) diff -r b599ca22418d -r c5195b16e7e4 review/messages.py --- a/review/messages.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/messages.py Sat Feb 06 11:36:36 2010 -0500 @@ -93,7 +93,7 @@ $ %s """ -REVIEW_LOG_AGE = """(%s ago)""" +REVIEW_LOG_AGE = """(%s)""" REVIEW_LOG_FILE_HEADER = """changes in %s""" REVIEW_LOG_SKIPPED = """\ diff -r b599ca22418d -r c5195b16e7e4 review/tests/test_init.py --- a/review/tests/test_init.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/tests/test_init.py Sat Feb 06 11:36:36 2010 -0500 @@ -1,3 +1,4 @@ +from __future__ import with_statement from nose import * from util import * from .. import messages diff -r b599ca22418d -r c5195b16e7e4 review/tests/util.py --- a/review/tests/util.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/tests/util.py Sat Feb 06 11:36:36 2010 -0500 @@ -1,5 +1,5 @@ """Utilities for writing unit tests for hg-review.""" - +from __future__ import with_statement import os, shutil import sample_data from mercurial import cmdutil, commands, hg, ui @@ -11,13 +11,14 @@ force=False, message='', rev='.', local_path='', remote_path='', lines='', files=None, unified='5', web=False): - files = files if files else [] + if not files: + files = [] _ui.pushbuffer() extension_ui.review(_ui, get_sandbox_repo(), *files, - init=init, comment=comment, signoff=signoff, yes=yes, no=no, + **dict(init=init, comment=comment, signoff=signoff, yes=yes, no=no, force=force, message=message, rev=rev, local_path=local_path, - remote_path=remote_path, lines=lines, unified=unified, web=web) + remote_path=remote_path, lines=lines, unified=unified, web=web)) output = _ui.popbuffer() print output diff -r b599ca22418d -r c5195b16e7e4 review/web_media/comments.js --- a/review/web_media/comments.js Thu Oct 22 18:27:31 2009 -0400 +++ b/review/web_media/comments.js Sat Feb 06 11:36:36 2010 -0500 @@ -1,11 +1,18 @@ $(function() { - $("form").hide(); + $("div > form").hide(); + $("tr:has(form)").hide(); $("p.comment-activate a").click(function(event) { $(event.target).hide(); $(event.target).closest("div").children("form").fadeIn("fast"); return false; }); + + $("tr.rem, tr.add, tr.con").click(function(event) { + $(event.target).closest("tr").addClass("comment-line-selected"); + $(event.target).closest("tr").next("tr.comment-line").fadeIn("fast"); + return false; + }); }); \ No newline at end of file diff -r b599ca22418d -r c5195b16e7e4 review/web_media/style.css --- a/review/web_media/style.css Thu Oct 22 18:27:31 2009 -0400 +++ b/review/web_media/style.css Sat Feb 06 11:36:36 2010 -0500 @@ -1,29 +1,52 @@ /* Basic layout and typography. */ body { - background: #f5f5f5; + background: #999; +} +div#content-wrap { + background-color: #fafafa; } div#main-wrap { - width: 65em; + width: 75em; margin: 0em auto; padding: 3em; } div#head-wrap { text-align: center; padding: 1.5em 0em .5em; - background-color: #ccc; - border-bottom: 1px solid #bbb; + color: #fff; + background-color: #111; + border-bottom: 6px solid #3C659A; +} +div#head-wrap h1 a, div#head-wrap h1 { + font-weight: normal; +} +div#remote-wrap { + text-align: center; + padding-top: 1.5em; + margin-bottom: -1.5em; +} +div#remote-wrap span.remote-section h3 { + display: inline; +} +div#remote-wrap span.remote-section { + margin: 0em 1em; +} +div#remote-wrap form { + display: inline; } div#footer { - color: #666; + border-top: 6px solid #666; + color: #fff; text-align: center; font-style: italic; + padding-top: 1.5em; } /* Links. */ a { text-decoration: none; font-weight: bold; - color: #297E00; + color: #3C659A; } a:hover { color: #EA0076; @@ -47,20 +70,26 @@ table tr td.last { text-align: right; } +table tr form * { + margin: 0em; +} /* Review pages. */ div.filename-header { background-color: #ccc; border: 1px solid #c5c5c5; padding: 1em; - width: 75em; - margin-left: -5em; margin-bottom: 1.5em; margin-top: 1.5em; } div.filename-header h3 { margin: 0em; } +div.filename-header a.fold-file, div.filename-header a.unfold-file { + float: right; + font-weight: bold; + font-size: 1.5em; +} /* Comments. */ .comment { @@ -85,6 +114,9 @@ } div#comment-file form { } +tr.comment-line-selected { + background-color: #FBEAD0; +} /* Diffs. */ div.diff { @@ -93,6 +125,9 @@ div.diff table tr { white-space: pre; } +div.diff table tr.comment-line { + white-space: normal; +} table tr.add { background: #DBF3D1; } diff -r b599ca22418d -r c5195b16e7e4 review/web_media/ui.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/review/web_media/ui.js Sat Feb 06 11:36:36 2010 -0500 @@ -0,0 +1,14 @@ +$(function() { + + $("a.fold-file").toggle(function(event) { + $(event.target).closest("div.file-review").find("div.file-review-contents").slideUp("fast"); + $(event.target).html("   ←").addClass("unfold-file").removeClass("fold-file"); + return false; + }, + function(event) { + $(event.target).closest("div.file-review").find("div.file-review-contents").slideDown("fast"); + $(event.target).html("   ↓").addClass("fold-file").removeClass("unfold-file"); + return false; + }); + +}); \ No newline at end of file diff -r b599ca22418d -r c5195b16e7e4 review/web_templates/base.html --- a/review/web_templates/base.html Thu Oct 22 18:27:31 2009 -0400 +++ b/review/web_templates/base.html Sat Feb 06 11:36:36 2010 -0500 @@ -1,4 +1,4 @@ -$def with (rd, content) +$def with (rd, content, title) @@ -9,18 +9,39 @@ + -
- $:{ content } -
- + +

Files

+ $for filename, diff in rcset.diffs().iteritems(): -
-

${ filename }

-
- - $ file_level_comments = filter(lambda c: c.filename == filename and not c.lines, rcset.comments) - $for comment in file_level_comments: -
-
-
-
${ templatefilters.person(comment.author) } said:
-
${ comment.message }
+
+
+    ↓ +

${ filename }

+
+ +
+ $ file_level_comments = filter(lambda c: c.filename == filename and not c.lines, rcset.comments) + $for comment in file_level_comments: +
+
+ +
+
+
${ templatefilters.person(comment.author) } said:
+
${ comment.message }
+
+
+ + +
+

Add a comment on this file

+
+
+ + +
+
+ +
+ +
+
+ +
+ + $ max_line = diff['max'] + $ content = diff['content'] + $ line_level_comments = filter(lambda c: c.filename == filename and c.lines, rcset.comments) + $ previous_n = -1 + $for n, line in content: + $if n - 1 > previous_n: + $ skipped_count = n - previous_n + $if previous_n == -1: + $ skipped_count -= 1 + + + + $ skipped_comments = filter(lambda c: max(c.lines) in range(previous_n + 1, n), line_level_comments) + $for comment in skipped_comments: + + $ kind = 'rem' if line[0] == '-' else 'add' if line[0] == '+' else 'con' + + + + $ line_comments = filter(lambda c: max(c.lines) == n, line_level_comments) + $for comment in line_comments: + + + + + $ previous_n = n + $if previous_n < max_line: + $ skipped_count = max_line - previous_n + + + + $ skipped_comments = filter(lambda c: max(c.lines) in range(previous_n + 1, max_line), line_level_comments) + $for comment in skipped_comments: + +
… skipped ${ skipped_count } lines …
+
+
+
${ templatefilters.person(comment.author) } said (on a skipped line):
+
${ comment.message }
+
+
${ line[1:] or ' ' }
+
+
+
${ templatefilters.person(comment.author) } said:
+
${ comment.message }
+
+
+
+
+ + +
+
+ +
+ + +
+
… skipped ${ skipped_count } lines …
+
+
+
${ templatefilters.person(comment.author) } said (on a skipped line):
+
${ comment.message }
+
+
- -
-

Add a comment on this file

-
-
- - -
-
- -
- -
-
- -
- - $ max_line = diff['max'] - $ content = diff['content'] - $ line_level_comments = filter(lambda c: c.filename == filename and c.lines, rcset.comments) - $ previous_n = -1 - $for n, line in content: - $if n - 1 > previous_n: - $ skipped_count = n - previous_n - $if previous_n == -1: - $ skipped_count -= 1 - - - - $ kind = 'rem' if line[0] == '-' else 'add' if line[0] == '+' else '' - - - - $ line_comments = filter(lambda c: max(c.lines) == n, line_level_comments) - $for comment in line_comments: - - $ previous_n = n - $if previous_n < diff['max']: - $ skipped_count = diff['max'] - previous_n - - - -
… skipped ${ skipped_count } lines …
${ line[1:] or ' ' }
-
-
-
${ templatefilters.person(comment.author) } said:
-
${ comment.message }
-
-
… skipped ${ skipped_count } lines …
\ No newline at end of file diff -r b599ca22418d -r c5195b16e7e4 review/web_ui.py --- a/review/web_ui.py Thu Oct 22 18:27:31 2009 -0400 +++ b/review/web_ui.py Sat Feb 06 11:36:36 2010 -0500 @@ -1,8 +1,9 @@ """The review extension's web UI.""" +from __future__ import with_statement import sys, os import api -from mercurial import cmdutil +from mercurial import cmdutil, hg package_path = os.path.split(os.path.realpath(__file__))[0] template_path = os.path.join(package_path, 'web_templates') @@ -20,6 +21,8 @@ '/', 'index', '/media/([^/]*)', 'media', '/review/([\da-f]{12})/?', 'review', + '/push/', 'push', + '/pull/', 'pull', ) @@ -40,8 +43,8 @@ def render_in_base(fn): def _fn(*args, **kwargs): - content = fn(*args, **kwargs) - return render.base(_rd, content) + title, content = fn(*args, **kwargs) + return render.base(_rd, content, title) return _fn class index: @@ -50,26 +53,57 @@ rev_max = _rd.target['tip'].rev() rev_min = rev_max - LOG_PAGE_LEN if rev_max >= LOG_PAGE_LEN else 0 revs = (_rd.target[r] for r in xrange(rev_max, rev_min, -1)) - return render.index(_rd, revs) + return ('', render.index(_rd, revs)) class review: @render_in_base def GET(self, node_short): - return render.review(_rd, _rd[node_short]) + title = '/ %s:%s – %s' % ( + _rd[node_short].target[node_short].rev(), + node_short, + _rd[node_short].target[node_short].description().splitlines()[0]) + return (title, render.review(_rd, _rd[node_short])) def POST(self, node_short): i = web.input() body = i['body'] filename = i['filename'] if 'filename' in i else '' + lines = i['lines'].split(',') if 'lines' in i else '' + print filename, lines if body: rcset = _rd[node_short] - rcset.add_comment(body, filename) + rcset.add_comment(body, filename, lines) raise web.seeother('/review/%s/' % node_short) +class push: + def GET(self): + path = web.input()['path'] + dest, revs, checkout = hg.parseurl(_rd.repo.ui.expandpath(path, path), None) + other = hg.repository(cmdutil.remoteui(_rd.repo, {}), dest) + + _rd.repo.push(other, True, revs=revs) + + raise web.seeother('/') + + +class pull: + def GET(self): + path = web.input()['path'] + source, revs, checkout = hg.parseurl(_rd.repo.ui.expandpath(path, path), None) + other = hg.repository(cmdutil.remoteui(_rd.repo, {}), source) + + modheads = _rd.repo.pull(other, heads=revs, force=True) + + if modheads: + hg.update(_rd.repo, 'tip') + + raise web.seeother('/') + + class media: def GET(self, fname): if '..' in fname: @@ -80,10 +114,15 @@ return content -def load_interface(ui, repo): +def load_interface(ui, repo, open=False): global _rd _rd = api.ReviewDatastore(ui, repo) sys.argv = sys.argv[:1] # Seriously, web.py? This is such a hack. app = web.application(urls, globals()) + + if open: + import webbrowser + webbrowser.open(app.browser().url) + app.run()