Bug 25881 - Attachment causes PrettyPatch to fail
Summary: Attachment causes PrettyPatch to fail
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL: https://bugs.webkit.org/attachment.cg...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-19 19:03 PDT by Eric Seidel (no email)
Modified: 2009-05-19 21:45 PDT (History)
2 users (show)

See Also:


Attachments
Make PrettyPatch understand quoted filenames in git diffs (983 bytes, patch)
2009-05-19 19:23 PDT, Eric Seidel (no email)
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-05-19 19:03:08 PDT
Attachment causes PrettyPatch to fail

./PrettyPatch/PrettyPatch.rb:72:in `split': can't convert nil into String (TypeError)
	from ./PrettyPatch/PrettyPatch.rb:72:in `find_url_and_path'
	from ./PrettyPatch/PrettyPatch.rb:85:in `linkifyFilename'
	from ./PrettyPatch/PrettyPatch.rb:203:in `to_html'
	from ./PrettyPatch/PrettyPatch.rb:14:in `prettify'
	from ./PrettyPatch/PrettyPatch.rb:14:in `collect'
	from ./PrettyPatch/PrettyPatch.rb:14:in `prettify'
	from PrettyPatch/prettify.rb:26
Comment 1 Eric Seidel (no email) 2009-05-19 19:19:29 PDT
Reduction:
diff --git "a/LayoutTests/editing/pasteboard/resources/File With Spaces! For Dra\314\210gging?.gif" "b/LayoutTests/editing/pasteboard/resources/File With Spaces! For Dra\314\210gging?.gif"
new file mode 100644
index 0000000..55844c8
Binary files /dev/null and "b/LayoutTests/editing/pasteboard/resources/File With Spaces! For Dra\314\210gging?.gif" differ

The quotes are throwing the diff parsing off.  Patch coming up.
Comment 2 Eric Seidel (no email) 2009-05-19 19:23:26 PDT
Created attachment 30489 [details]
Make PrettyPatch understand quoted filenames in git diffs

 2 files changed, 10 insertions(+), 1 deletions(-)
Comment 3 Adam Roben (:aroben) 2009-05-19 19:48:48 PDT
Comment on attachment 30489 [details]
Make PrettyPatch understand quoted filenames in git diffs

I guess this will allow mismatched quotes through (i.e., a/foo"), but that's probably fine.

r=me
Comment 4 Eric Seidel (no email) 2009-05-19 20:56:30 PDT
Landed as r43884.  Thanks!
Comment 5 Eric Seidel (no email) 2009-05-19 20:58:09 PDT
http://trac.webkit.org/changeset/43884
Comment 6 Eric Seidel (no email) 2009-05-19 21:03:23 PDT
Interesting.  Well, so the fix worked, but leaves a trailing " in the filename.  It's OK, but less than ideal.  I could find a way to use:

(")? and \1 I guess.
Comment 7 Adam Roben (:aroben) 2009-05-19 21:45:41 PDT
(In reply to comment #6)
> Interesting.  Well, so the fix worked, but leaves a trailing " in the filename.
>  It's OK, but less than ideal.  I could find a way to use:
> 
> (")? and \1 I guess.

Assuming the filename can't contain a quote, you could replace

(.+)

with

([^"]+)