Bug 21222

Summary:

Formatted Diff for attachment 23920 is mangled

Product:

WebKit

Reporter:

Mark Rowe (bdash) <mrowe>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, aroben, cmarcelo, commit-queue, dbates, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

URL:

https://bugs.webkit.org/attachment.cgi?id=23920&action=prettypatch

Attachments:

Description	Flags
patch	aroben: review-
patch v2, moved normalization to lower layer	none

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-	Details Formatted Diff Diff
patch v2, moved normalization to lower layer (2.27 KB, patch) 2011-03-09 11:04 PST, Caio Marcelo de Oliveira Filho	no flags	Details Formatted Diff Diff
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.