More patches that aren't handled by PrettyPatch.rb: Bug 19041 https://bugs.webkit.org/attachment.cgi?id=21119&action=prettypatch Bug 18465 https://bugs.webkit.org/attachment.cgi?id=20510&action=prettypatch https://bugs.webkit.org/attachment.cgi?id=20528&action=prettypatch
Bug 19043 https://bugs.webkit.org/attachment.cgi?id=21120&action=prettypatch https://bugs.webkit.org/attachment.cgi?id=21121&action=prettypatch
Bug 19067 https://bugs.webkit.org/attachment.cgi?id=21151&action=prettypatch
Bug 19068 https://bugs.webkit.org/attachment.cgi?id=21152&action=prettypatch
Bug 19069 https://bugs.webkit.org/attachment.cgi?id=21153&action=prettypatch
Note that all patches listed in Comment #0 through Comment #4 (except Bug 18465) are from the same contributor.
Created attachment 21388 [details] print exceptions from PrettyPatch to stdout
Created attachment 21389 [details] print exceptions from PrettyPatch to stdou Fixed a typo
Comment on attachment 21389 [details] print exceptions from PrettyPatch to stdou Committed in r34173
It looks like attachment 20510 [details] and attachment 20528 [details] are missing the "Index: " line that PrettyPatch uses to find the start of the diff. All the other attachments are causing PrettyPatch trouble because they have a leading / in the path of the files. This should be easy to fix.
Created attachment 21392 [details] Fix most of the patches listed in this bug
Created attachment 21393 [details] Fix most of the patches listed in this bug Fixed a typo
Comment on attachment 21393 [details] Fix most of the patches listed in this bug r=me!
Comment on attachment 21393 [details] Fix most of the patches listed in this bug Committed in r34175
The only remaining broken patches are: Attachment 20510 [details] Attachment 20528 [details]
Created attachment 21469 [details] Fix remaining patches Fix the remaining issue for patches with no "Index" or "diff" headers.
(In reply to comment #15) > Created an attachment (id=21469) [edit] > Fix remaining patches > > Fix the remaining issue for patches with no "Index" or "diff" headers. I should note that this change will NOT handle a patch with a mix of diffs that do and do not contain headers. I don't expect that case to ever happen, though.
Comment on attachment 21469 [details] Fix remaining patches I wonder if we should just always look for the "---" and "+++" lines instead of looking for "Index:" or "diff --git" lines? I think that's what the patch utility does anyway. It seems like it would simplify this code quite a bit. Care to take a stab at that, Dave?
(In reply to comment #17) > (From update of attachment 21469 [details] [edit]) > I wonder if we should just always look for the "---" and "+++" lines instead of > looking for "Index:" or "diff --git" lines? I think that's what the patch > utility does anyway. It seems like it would simplify this code quite a bit. > > Care to take a stab at that, Dave? Yes, maybe today. Bug mail is awfully slow (or Y!Mail delivery has slowed way down).
Comment on attachment 21469 [details] Fix remaining patches r=me
Comment on attachment 21469 [details] Fix remaining patches Sorry, Darin, I should have cleared the review? flag per Comment #17 and Comment #18.
Created attachment 21918 [details] Fix remaining patches v2 (In reply to comment #17) > (From update of attachment 21469 [details] [edit]) > I wonder if we should just always look for the "---" and "+++" lines instead of > looking for "Index:" or "diff --git" lines? I think that's what the patch > utility does anyway. It seems like it would simplify this code quite a bit. > > Care to take a stab at that, Dave? I don't think ignoring the "Index:" and "diff --git" lines will work. There are some patches with an "Index:" line that don't have "---" or "+++" lines (like binary patches), so this approach won't work with those cases. I did manage to simplify the patch from the previous attempt, at least for the first two changes.
Comment on attachment 21918 [details] Fix remaining patches v2 r=me
Remaining patches should have been fixed by: Committed revision 34797. But it doesn't appear that they were. :(
Comment on attachment 21918 [details] Fix remaining patches v2 Clearing review flag per Comment #23.
(In reply to comment #23) > Remaining patches should have been fixed by: > > Committed revision 34797. > > But it doesn't appear that they were. :( This was a server update issue. The issue is now resolved!