A frequent, and especially troublesome to debug, cause for post-review issues
is embedded windows newlines. These are fine if they occur at the end of a
line, but when a \r isn't followed by a \n the patch command run on the
ReviewBoard host will usually choke due to misaligning line numbers in the
Pushed to the following branch...
Tested by making a perforce change riddled with erroneous windows newlines,
then posting a review for it (ReviewBoard wept bitterly when rendering the
diff, of course).
We've been using this change against RBTools 0.3. I've adapted the change
for the current master but this copy of it hasn't been exercised.
Bump? I've revised this patch a little and tested as follows...
% p4 edit README
% echo "more\rstuff\r\n" >> README
This produced a server-side stacktrace without this patch, and works with this
normalization. This can be pulled from my 'newline_normalization' branch...
I'm pretty hesitant to change any logic related to newlines, due to the subtle ways in which they can break.
We've seen all the following cases before:
* \r\n on every line
* \n on every line
* \r on every line
* \r\r\n on every line
* \r\n only on some lines
* \n only on some lines
* \r only on some lines
* \r\r\n only on some lines
We'd need to make sure we don't regress any of those. And yes, some simply fail today, as you've noticed, because these situations can't always be fixed.
There are files out there that have embedded newlines as part of the data set and not necessarily as line terminators. It complicates things, and I'm not sure what the best course of action is for it.
What I'd need to see are unit tests accompanying this that check each type of file and make sure that the results are correct.
I'm curious if you know how your files get into these situations. Usually, this indicates issues with repository configuration, Perforce doing newline normalization and then syncing those files from one platform to another, or editor issues.