RESOLVED FIXED 26210
PrettyPatch should display added images inline
https://bugs.webkit.org/show_bug.cgi?id=26210
Summary PrettyPatch should display added images inline
Eric Seidel (no email)
Reported 2009-06-05 01:09:24 PDT
PrettyPatch should display added images inline Cause it'd be cool?
Attachments
Hackish first pass, currently svn only, png only (1.99 KB, patch)
2009-06-05 01:11 PDT, Eric Seidel (no email)
no flags
Add support for inline display of png files from svn-create-patch patches (2.64 KB, patch)
2009-06-05 01:19 PDT, Eric Seidel (no email)
aroben: review+
Eric Seidel (no email)
Comment 1 2009-06-05 01:11:23 PDT
Created attachment 30996 [details] Hackish first pass, currently svn only, png only 1 files changed, 16 insertions(+), 2 deletions(-)
Eric Seidel (no email)
Comment 2 2009-06-05 01:11:53 PDT
Comment on attachment 30996 [details] Hackish first pass, currently svn only, png only I didn't actually mean to mark this for review. Old habits...
Eric Seidel (no email)
Comment 3 2009-06-05 01:15:24 PDT
I guess this will have to be SVN only for now. git-send-bugzilla patches don't include image data: https://bugs.webkit.org/attachment.cgi?id=30012
Eric Seidel (no email)
Comment 4 2009-06-05 01:19:02 PDT
Created attachment 30997 [details] Add support for inline display of png files from svn-create-patch patches 2 files changed, 28 insertions(+), 2 deletions(-)
Adam Roben (:aroben)
Comment 5 2009-06-05 07:33:14 PDT
(In reply to comment #3) > I guess this will have to be SVN only for now. git-send-bugzilla patches don't > include image data: > https://bugs.webkit.org/attachment.cgi?id=30012 If git-send-bugzilla works like git-send-email, then it just sends whatever is in the .patch file generated by git-format-patch. So git-send-bugzilla has no opinion about image data. You just need to pass --binary to git-format-patch.
Adam Roben (:aroben)
Comment 6 2009-06-05 07:38:35 PDT
Comment on attachment 30997 [details] Add support for inline display of png files from svn-create-patch patches > +.image { > + border: 2px solid black; > +} Why not just key this off the img tag? > + > .context, .context .lineNumber { > color: #849; > background-color: #fef; > @@ -191,17 +197,25 @@ EOF > break > when BINARY_FILE_MARKER_FORMAT > @binary = true > + if (IMAGE_FILE_MARKER_FORMAT.match(lines[i + 1])) then This will probably do something bad on the last iteration of the loop when i == lines.length - 1. But maybe we know that isn't the case given that we just saw a binary file marker? > + lines_with_contents = lines[startOfSections...lines.length] > + @sections = DiffSection.parse(lines_with_contents) unless @binary > + @image_url = "data:image/png;base64," + lines_with_contents.join if @image I'm surprised you didn't have to chomp the newlines off each line. > + if @image then > + str += "<img class='image' src='" + @image_url + "' />" No need for the space or the slash before the right angle bracket. r=me
Brent Fulgham
Comment 7 2009-06-09 15:47:43 PDT
Landed in @r44547.
Nikolas Zimmermann
Comment 8 2009-06-09 15:52:09 PDT
Wow, this is so useful, thanks for that! :-)
Note You need to log in before you can comment on or make changes to this bug.