Bug 26210 – PrettyPatch should display added images inline

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

PrettyPatch should display added images inline

Cause it'd be cool?

[Eric Seidel (no email)]

Created attachment 30996 [details]
Hackish first pass, currently svn only, png only

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

[Eric Seidel (no email)]

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)]

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)]

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)]

(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 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]

Landed in @r44547.

[Nikolas Zimmermann]

Wow, this is so useful, thanks for that! :-)