Before I get into actually reviewing the change, I'm trying to understand the need for the change. First, I don't think post-review should deal with anything involving newlines. It just shouldn't care. All logic should be in Review Board for this. We don't guarantee that people even will use post-review for uploading, as they may use the web UI or some third party app. Second, where does \r\r\n even come from? How is that a valid newline? Out of all the people that have used Review Board, I have never seen anyone hit an issue with a diff with \r\r\n. If this is some weirdness with a file originally using \r newlines later having \r\n newlines added on top of that, then that's something that needs to be fixed in the file and not Review Board, imho.
Review Board does not handle files with ^M characters at end of line.
Review Request #826 — Created April 20, 2009 and submitted
|Review Board SVN (deprecated)|
This is a patch for http://code.google.com/p/reviewboard/issues/detail?id=1071 that I had reported. The idea is to make post-review and diffviewer intelligent about lines ending with \r\r\n pattern. All we have to do is to convert those to a simple newline.
Tested this at my work, looks fine so far.
I have fixed all of the review comments except one. Sorry about being so late on this, I wanted to get this in early but was on a long vacation and didn't get time to complete this.
Revision 3 (+41 -35)
Thanks! Committed as r1988.