RESOLVED FIXED 52180
convert back to unified from sidebyside diff 
https://bugs.webkit.org/show_bug.cgi?id=52180
Summary convert back to unified from sidebyside diff
Ojan Vafai
Reported 2011-01-10 16:55:27 PST
convert back to unified from sidebyside diff
Attachments
Patch (11.57 KB, patch)
2011-01-10 16:58 PST, Ojan Vafai 		abarth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-01-10 16:58:00 PST
Created attachment 78473 [details] Patch
Adam Barth
Comment 2 2011-01-10 17:31:08 PST
Comment on attachment 78473 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review > Websites/bugs.webkit.org/code-review.js:885 > + $('.unify-link').live('click', handleUnifyLinkClick); > $('.ExpandLink').live('click', handleExpandLinkClick); These classes seem unnecessarily different.
Adam Barth
Comment 3 2011-01-10 17:31:29 PST
It's difficult to review this patch for correctness, but it seems fine.
Ojan Vafai
Comment 4 2011-01-10 18:48:12 PST
(In reply to comment #2) > (From update of attachment 78473 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review > > > Websites/bugs.webkit.org/code-review.js:885 > > + $('.unify-link').live('click', handleUnifyLinkClick); > > $('.ExpandLink').live('click', handleExpandLinkClick); > > These classes seem unnecessarily different. Yeah. I never know which style to follow in this file. The existing style in this file is some mashup of webkit/chromium/bugzilla style. At some point we should make it have a consistent style. Until then, I'm not sure what style to follow, so I end up doing some mix. :(
Adam Barth
Comment 5 2011-01-10 18:56:28 PST
Comment on attachment 78473 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78473&action=review >>> Websites/bugs.webkit.org/code-review.js:885 >>> $('.side-by-side-link').live('click', handleSideBySideLinkClick); >>> + $('.unify-link').live('click', handleUnifyLinkClick); >>> $('.ExpandLink').live('click', handleExpandLinkClick); >> >> These classes seem unnecessarily different. > > Yeah. I never know which style to follow in this file. The existing style in this file is some mashup of webkit/chromium/bugzilla style. At some point we should make it have a consistent style. Until then, I'm not sure what style to follow, so I end up doing some mix. :( Fair enough. :)
Ojan Vafai
Comment 6 2011-01-11 10:39:32 PST
Committed r75514: <http://trac.webkit.org/changeset/75514>
WebKit Review Bot
Comment 7 2011-01-11 12:07:12 PST
http://trac.webkit.org/changeset/75514 might have broken GTK Linux 64-bit Debug
Note You need to log in before you can comment on or make changes to this bug.