-
-
-
trunk/reviewboard/diffviewer/parser.py (Diff revision 2) Trailing whitespace. There's a lot of this, so can you go through the diff viewer and make sure it's all been removed?
DiffParser class, subclass for SCM specific diff parse
Review Request #99 — Created June 24, 2007 and submitted
| Information | |
|---|---|
| trickie | |
| Review Board SVN (deprecated) | |
| trunk | |
| Reviewers | |
| reviewboard | |
Update: - only change is to make the regexp for hunting out binary changes is now a class member variable ================================================================ Update: These changes preserve the same behaviour of the previous functional approach to diff parsing, it just wraps it all in a class. - removed useless whitespace - compiled binary-change-matching regexp in __init__ ================================================================ Update: Split diff parsing into smaller methods, makes it easier to tweak per SCM implementation ================================================================ I have split this review request from the GitTool in http://reviews.review-board.org/r/80/ This patch will create a DiffParser class to allow SCMTool's to override the diff parsing operation, and (hopefully) lead to less code duplication. It is pretty much just jamming the previous functional approach into a class.
Unittests updated, tests pass
-
Looks reasonable. Aside from the cosmetic changes that Christian suggested, my only request is that you compile the regexps and store them in a class data member for the parser.
-
-
trunk/reviewboard/diffviewer/parser.py (Diff revision 4) This can be class data instead of member data, so we only have one for the entire app: class DiffParser: binregexp = re.compile(...) def __init__(self, data): ...