Bug 25881 – Attachment causes PrettyPatch to fail

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

Created attachment 30489 [details]
Make PrettyPatch understand quoted filenames in git diffs

 2 files changed, 10 insertions(+), 1 deletions(-)

[Adam Roben (:aroben)]

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

[Eric Seidel (no email)]

Landed as r43884.  Thanks!

[Eric Seidel (no email)]

http://trac.webkit.org/changeset/43884

[Eric Seidel (no email)]

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.

[Adam Roben (:aroben)]

(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

([^"]+)