I'm a bit confused as to this segment. Why are we special-casing this? If it's necessary, a comment should be added describing why.
I reverted the previous CRLF change due to several bugs that it caused. To prevent that here, I'd like to request some thorough unit tests covering diffs with different line terminators and testing any revision info that would be affected. We should test that each can still be parsed and that each can be applied.
Fix CRLF issues
Review Request #83 — Created June 19, 2007 and submitted
|Review Board SVN (deprecated)|
This is quite an hack, but at least it will start the ball rolling on this issue. There are several problems, all related to CRLF. The first problem I had, and that the simpler patch I posted on issue 78 solved, was creating a diff on Linux to a file with CRLF endings. The next problem, that this patch solves, is posting a patch created on windows (with svn diff or tortoisesvn). There were two problems: * the parsing of the revision number, caused by the first part of the patch. * after correcting revision parsing, another problem is caused by patch itself. If patch (in Unix, at least) gets a patch file with CRLF line endings, it assumes it was meant to have LF line endings and throws the CR away. This makes the patch fail because the file being patched has CRLF. This second problem is solved by converting the header lines to end with LF. By the way, I think this is the cause for the issue mentioned by someone on the mailing list using tortoise svn to generate the diff. The code is a hack, I just started looking at python code this week, and I don't really know what happens with review-board installed on Windows, but it solves my problem.
It works for patches to files containing CRLF, generated both on Windows and Linux, at least when the server is running on Linux