Two simple queries (since I can't find a mailing list for the coderev project)

[GoogleCodeExporter]

First, some well-deserved praise - I love the simplicity, usefulness, and 
vendor + language neutrality of coderev, and wish that more development 
utilities were as easy to find, use, and understand.

Two Questions (thanks in advance for comments, links, etc.):

1) After downloading the package I noticed some very minor glitches in 
codediff.py version 36 (some extra spaces were breaking the links in my "SVN 
diffs") and I decided to fix them add a switch that lets me render "one line 
changes" as "boring" (still shown, but in grey). How can I share these changes 
with other coderev users?

2) I like the "side by side" diff format which coderev generates so much that I 
would like to hijack it somehow for line-by-line source code annotation. My 
dream would be some way to associate blocks of code (displayed on the left) 
with blocks of commentary (displayed on the right) in an HTML document without 
having to spend hours tweaking the CSS in order to get it to look OK. Does 
anyone know of an existing tool that would allow me to do this?

Original issue reported on code.google.com by pwolfen...@qualys.com on 26 May 2011 at 2:03

[GoogleCodeExporter]

1) I do not understand the issue, maybe you are talking about similar to issue 
3?  Just send me your diff and I can patch it.

2) This tool can only generate static html page so far and I have no plan to 
add any dynamic stuff, if you are looking for a more powerful review tool, 
reviewboard may be your choice.

Matt

Original comment by matt...@gmail.com on 26 May 2011 at 9:03

[GoogleCodeExporter]

Thanks, Matt. Here's my diff for codediff.py:

$ diff coderev-read-only-as-of-this-morning/codediff.py my-coderev/codediff.py
5a6,16
> # CHANGELOG:
> # - 2011/03/25 <pjw@qualys.com> fix some inline typos, add CHANGELOG
> # - 2011/03/25 <pjw@qualys.com> treat "one line changes" as "boring" (grey)
> # - 2011/04/07 <pjw@qualys.com> drop leading space from paths to fix SVN diffs
> # - 2011/04/07 <pjw@qualys.com> integrate into XXXX, add EXAMPLES.txt
> #
> # TODO:
> # - <pjw@qualys.com> control "boring" logic via a (new) command line option
> #       (see OptionParser logic)
> # - <pjw@qualys.com> reduce duplicated code by adding parameters to HTML
> #       templates
202a214
>             .boring {color:gray; font:9pt;}
244a257
>             .boring {color:gray; font:9pt;}
402a416,433
> # TODO: avoid code duplication by adding a "class" parameter to the above 
template
>     _boring_diff_data_row_template = """
>     <tr class="boring">
>         <td>%(pathname)s</td>
>         <td><abbr title="Changed/Deleted/Added">\
>                 %(changed)s/%(deleted)s/%(added)s</abbr></td>
>         <td><a href="%(pathname_url)s.cdiff.html" title="context 
diff">Cdiff</a>\
>                 </td>
>         <td><a href="%(pathname_url)s.udiff.html" title="unified 
diff">Udiff</a>\
>                 </td>
>         <td><a href="%(pathname_url)s.sdiff.html" title="side-by-side context 
diff">\
>                 Sdiff</a></td>
>         <td><a href="%(pathname_url)s.fdiff.html" title="side-by-side full 
diff">\
>                 Fdiff</a></td>
>         <td><a href="%(pathname_url)s-.html" title="old file">Old</a></td>
>         <td><a href="%(pathname_url)s.html" title="new file">New</a></td>
>     </tr>"""
> 
527a559,562
>             f = f.lstrip() # drop leading spaces
>             # print ' PJW DEBUG: * f=[%-20s]' % f,
>             # print
> 
614a650
>                 # Emit toplevel index line:
623c659
<                 data_row = self._diff_data_row_template % dict(
---
>                 change_vars = dict(
630c666,674
<                 summary['changed'] += 1
---
>                 # "One line changes" are boring, so display them,
>                 # but don't count them as "real" changes:
>                 if ((file_summary['changed'] == 1)
>                     and (file_summary['deleted'] == 0)
>                     and (file_summary['added'] == 0)):
>                     data_row = self._boring_diff_data_row_template % 
change_vars
>                 else:
>                     data_row = self._diff_data_row_template % change_vars
>                     summary['changed'] += 1
648c692
<         # now wirte index page
---
>         # now write index page
697c741
<         'Warnning for overwriting, return True if answered yes, False if no'
---
>         'Warning for overwriting, return True if answered yes, False if no'

Original comment by pwolfen...@qualys.com on 26 May 2011 at 1:53

[GoogleCodeExporter]

Attaching the patch as a file, to reduce the chances of copy/paste artifacts.

Original comment by pwolfen...@qualys.com on 26 May 2011 at 1:57

Attachments:

• coderev_diff.patch

[GoogleCodeExporter]

Regarding "static" vs. "dynamic", I wasn't so much thinking of a full WYSIWYG 
system (thank you for the reviewboard recommendation, though) as something that 
would take the raw source code and combine it with some "annotation" 
information, for example:

{
 "path/to/file.php": {
   "notes" : [
     { "lines": ["3", "12-15"], "text" : "Replace \"foo\" with
\"bar\" here." },
     { "lines": ["78-190"], "text": "This is obsolete - remove it!" },
     { "lines": ["386"], "text": "What does this do?" },
     { "text": "Consider using the new XXX API instead of YYY." } // applies to whole file
   ]
 },
 "path/to/another_file.php": {
   "notes" : [ { "text": "Integrate all of these methods into the ZZZ
class, and drop this class entirely." } ]
 }
}

to make the (static) HTML output, in much the same spirit as coderev in its 
current form.

But even this relatively constrained problem is hardly trivial to solve - I'm 
not sure what would be most useful to display, for example, in the case of 
"overlapping" notes (i.e. where some lines have more than one associated block 
of commentary). It might make sense to show "minimized" comments on the right 
and then highlight the associated lines (if any) on the left when each comment 
is expanded (and then re-minimized) in turn. Or it may simply not be practical 
(yet) to make something like this work in a reasonably attractive and 
browser-independent way.

Original comment by pwolfen...@qualys.com on 26 May 2011 at 3:34

[GoogleCodeExporter]

Hi, I read the CHANGELOG in the diff in comment 3, see comments inline.

> # CHANGELOG:
> # - 2011/03/25 <pjw@qualys.com> fix some inline typos, add CHANGELOG

This is good.

> # - 2011/03/25 <pjw@qualys.com> treat "one line changes" as "boring" (grey)

I personally don't agree to treat one line changes as boring, I've seen a lot 
important change but still in one line in my experience :)

> # - 2011/04/07 <pjw@qualys.com> drop leading space from paths to fix SVN diffs

What do you mean "leading space from paths"? How this happen?

> # - 2011/04/07 <pjw@qualys.com> integrate into XXXX, add EXAMPLES.txt

No idea...

Original comment by matt...@gmail.com on 30 May 2011 at 3:40

[GoogleCodeExporter]

Comments for comment 5:

IMHO, the most possible enhancement to the static pages would be using 
javascript to hide/show lines in context in the side-by-side diff page.  I've 
seen similar feature in "wx" - one teamwork tool of Sun Microsystems.  However 
coderev is too much glued to python difflib, and I am not so practised on js, I 
don't plan to do further efforts on this area.

Original comment by matt...@gmail.com on 30 May 2011 at 3:45

[GoogleCodeExporter]

This tool is moved to https://github.com/ymattw/coderev/, github gives you 
flexibility of "forking" :)

Original comment by matt...@gmail.com on 12 Jan 2013 at 2:08

• Changed state: WontFix

[GoogleCodeExporter]

This tool is moved to https://github.com/ymattw/coderev/, github gives you 
flexibility of "forking" :)

Original comment by matt...@gmail.com on 12 Jan 2013 at 2:08