[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 #2 from Eric Seidel <eric at webkit.org>  2010-04-13 15:16:49 PST ---
(From update of attachment 53224)
+        except OSError, e:
+            # No pretty diff.  :(
+            self._tool.user.page(diff)

Do we need to show an error message?
Do we need to catch ScriptError as well?  Should we be logging inside that
function and returning a bool instead?

Why + instead of %?

+        url = "file://" + urllib.quote(self._pretty_diff_file.name)

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?

Sometimes I really had 80c:
+        args = [
+            "ruby",
+            "-I",
+            pretty_patch_path,
+            prettify_path,
+        ]

Seems we may want to explicitly close/delete the temporary file:
+    def pretty_diff_file(self, diff):
+        pretty_diff = self.pretty_diff(diff)
+        diff_file = tempfile.NamedTemporaryFile(suffix=".html")
+        diff_file.write(pretty_diff)
+        diff_file.flush()
+        return diff_file

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