[Webkit-unassigned] [Bug 37489] Use pretty patch for confirming webkit-patch diffs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=37489





--- Comment #3 from Adam Barth <abarth at webkit.org>  2010-04-14 08:49:13 PST ---
> Do we need to show an error message?

Fixed.

> Do we need to catch ScriptError as well?

When would a ScriptError be thrown?  It seems like that would only be in case
of a bug in webkit-patch or PrettyPatch.

> Should we be logging inside that function and returning a bool instead?

Done.  We actually need to return the file because we need to keep it alive
until the user is done viewing the diff.

> Why + instead of %?

Fixed.

> Seems kinda hacky:
> +        if url.startswith("file://"):
> +            log("MOCK: user.open_url: file://...")
> +            return
> 
> Why not just change what the tmp path is to make it consistent?

I couldn't find a clean way of doing this.  The problem is ConfirmDiff uses
PrettyPatch statically, and PrettyPatch uses tempfile statically.

> Sometimes I really had 80c:

+dpranke :)

> Seems we may want to explicitly close/delete the temporary file:

I've added an explicit close.  Python should do the deleting for us.  In fact,
we have to work around the fact that Python is super eager to delete the file.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.