RESOLVED FIXED 21222
Formatted Diff for attachment 23920 [details] is mangled 
https://bugs.webkit.org/show_bug.cgi?id=21222
Summary Formatted Diff for attachment 23920 is mangled
Mark Rowe (bdash)
Reported 2008-09-29 15:00:20 PDT
The formatted diff for attachment 23920 [details] is mangled. The raw diff is at <https://bugs.webkit.org/attachment.cgi?id=23920&action=view>, with the mangled formatting at <https://bugs.webkit.org/attachment.cgi?id=23920&action=prettypatch>.
Attachments
patch (2.26 KB, patch)
2011-03-08 20:50 PST, Caio Marcelo de Oliveira Filho 		aroben: review-
DetailsFormatted DiffDiff
patch v2, moved normalization to lower layer (2.27 KB, patch)
2011-03-09 11:04 PST, Caio Marcelo de Oliveira Filho 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1 2008-10-01 18:48:10 PDT
*** Bug 21291 has been marked as a duplicate of this bug. ***
Caio Marcelo de Oliveira Filho
Comment 2 2011-03-08 20:50:30 PST
Created attachment 85129 [details] patch
Eric Seidel (no email)
Comment 3 2011-03-08 21:14:33 PST
Comment on attachment 85129 [details] patch Shouldn't this be done at a lower layer? Or shouldn't prettyify ASSERT that it has the proper line endings?
Eric Seidel (no email)
Comment 4 2011-03-08 21:14:59 PST
How do we unit test this? (I'm not sure we have unit tests for the ruby code?)
Adam Roben (:aroben)
Comment 5 2011-03-09 05:23:04 PST
Comment on attachment 85129 [details] patch I agree with Eric that the line-ending normalization should happen in PrettyPatch itself. prettify.rb is just supposed to be a thin wrapper around PrettyPatch.
Adam Roben (:aroben)
Comment 6 2011-03-09 05:23:43 PST
(In reply to comment #4) > How do we unit test this? (I'm not sure we have unit tests for the ruby code?) We don't have unit tests. It would be pretty easy to add some, though!
Caio Marcelo de Oliveira Filho
Comment 7 2011-03-09 11:04:50 PST
Created attachment 85197 [details] patch v2, moved normalization to lower layer
Caio Marcelo de Oliveira Filho
Comment 8 2011-03-09 11:13:06 PST
(In reply to comment #6) > We don't have unit tests. It would be pretty easy to add some, though! What kind of tests are you thinking about, one that gets a set of patches to run prettify and compare with some expected files? Or something different? Any guidance on where to put those is welcome too :)
Eric Seidel (no email)
Comment 9 2011-03-09 11:14:20 PST
(In reply to comment #8) > (In reply to comment #6) > > We don't have unit tests. It would be pretty easy to add some, though! > > What kind of tests are you thinking about, one that gets a set of patches to run prettify and compare with some expected files? Or something different? > > Any guidance on where to put those is welcome too :) I was thinking more along the lines of the python unit tests: *_unittest.py under http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/
WebKit Commit Bot
Comment 10 2011-03-10 16:57:03 PST
Comment on attachment 85197 [details] patch v2, moved normalization to lower layer Clearing flags on attachment: 85197 Committed r80795: <http://trac.webkit.org/changeset/80795>
WebKit Commit Bot
Comment 11 2011-03-10 16:57:07 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.