63963 2011-07-05 17:58:58 -0700 Would be nice if the review-tool offered to show the corresponding header 2011-10-13 18:39:36 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 eric abarth abarth aroben ojan webkit.review.bot oldest_to_newest 432509 0 eric 2011-07-05 17:58:58 -0700 Would be nice if the review-tool offered to show the corresponding header Similar to how XCode has a "show me the corresponding file" button for headers/cpps, would be nice if the review-tool knew how to show the corresponding header/cpp (probably in an iframe) to the current file under review. 432513 1 ojan 2011-07-05 18:04:43 -0700 Sounds great. We already have all the code in there to pull source files from trak. I'm trying to think what the UI would look like. We could add the header file to the diff, but just not have it allow comments? 432605 2 abarth 2011-07-05 20:58:32 -0700 In the links that appear on the right (like "blame") you could have a link called "header" that went to trac. 432626 3 99793 abarth 2011-07-05 22:23:33 -0700 Created attachment 99793 Untested, probably broken 432666 4 99793 ojan 2011-07-06 00:18:59 -0700 Comment on attachment 99793 Untested, probably broken View in context: https://bugs.webkit.org/attachment.cgi?id=99793&action=review Code seems fine to me. Do we also want to show link to the .cpp file for .h files? > Websites/bugs.webkit.org/code-review.js:628 > + stem = file_name.substr(0, file_name.length - 'cpp'.length); Need to declare "stem". 432668 5 abarth 2011-07-06 00:28:13 -0700 We'll probably also want to handle ".mm" and possibly FooMac.mm => Foo.h. I'm not sure how much of that to bite off in the first iteration. 483647 6 110933 abarth 2011-10-13 16:49:47 -0700 Created attachment 110933 Patch 483655 7 110933 ojan 2011-10-13 16:55:35 -0700 Comment on attachment 110933 Patch Code looks fine. Can you add a test? code-review-test.html 483658 8 abarth 2011-10-13 17:02:32 -0700 > Can you add a test? What fun would that be! :) 483671 9 110940 abarth 2011-10-13 17:16:38 -0700 Created attachment 110940 Patch 483717 10 110940 webkit.review.bot 2011-10-13 18:39:31 -0700 Comment on attachment 110940 Patch Clearing flags on attachment: 110940 Committed r97435: <http://trac.webkit.org/changeset/97435> 483718 11 webkit.review.bot 2011-10-13 18:39:36 -0700 All reviewed patches have been landed. Closing bug. 99793 2011-07-05 22:23:33 -0700 2011-10-13 16:49:42 -0700 Untested, probably broken bug-63963-20110705222332.patch text/plain 844 abarth SW5kZXg6IFdlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9jb2RlLXJldmlldy5qcwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBXZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZpZXcuanMJKHJldmlzaW9uIDkw NDMzKQorKysgV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL2NvZGUtcmV2aWV3LmpzCSh3b3JraW5n IGNvcHkpCkBAIC02MjMsNiArNjIzLDExIEBAIHZhciBDT0RFX1JFVklFV19VTklUVEVTVDsKICAg ICB2YXIgdHJhY19saW5rcyA9ICQoJzxhIHRhcmdldD0iX2JsYW5rIj5hbm5vdGF0ZTwvYT48YSB0 YXJnZXQ9Il9ibGFuayI+cmV2aXNpb24gbG9nPC9hPicpOwogICAgIHRyYWNfbGlua3NbMF0uaHJl ZiA9ICdodHRwOi8vdHJhYy53ZWJraXQub3JnL2Jyb3dzZXIvdHJ1bmsvJyArIGZpbGVfbmFtZSAr ICc/YW5ub3RhdGU9YmxhbWUnICsgdXJsX2hhc2g7CiAgICAgdHJhY19saW5rc1sxXS5ocmVmID0g J2h0dHA6Ly90cmFjLndlYmtpdC5vcmcvbG9nL3RydW5rLycgKyBmaWxlX25hbWU7CisgICAgaWYg KC9bLl1jcHAkLy50ZXN0KGZpbGVfbmFtZSkpIHsKKyAgICAgIHRyYWNfbGlua3MucHJlcGVuZCgn PGEgdGFyZ2V0PSJfYmxhbmsiPmhlYWRlcjwvYT4nKTsKKyAgICAgIHN0ZW0gPSBmaWxlX25hbWUu c3Vic3RyKDAsIGZpbGVfbmFtZS5sZW5ndGggLSAnY3BwJy5sZW5ndGgpOworICAgICAgdHJhY19s aW5rc1swXS5ocmVmPSAnaHR0cDovL3RyYWMud2Via2l0Lm9yZy9sb2cvdHJ1bmsvJyArIHN0ZW0g KyAnLmgnOworICAgIH0KICAgICByZXR1cm4gdHJhY19saW5rczsKICAgfQogCg== 110933 2011-10-13 16:49:47 -0700 2011-10-13 17:16:34 -0700 Patch bug-63963-20111013164945.patch text/plain 2010 abarth U3VidmVyc2lvbiBSZXZpc2lvbjogOTc0MTgKZGlmZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Vi a2l0Lm9yZy9DaGFuZ2VMb2cgYi9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvQ2hhbmdlTG9nCmlu ZGV4IGQxNDg5YTk2NDY1OGFhNjQ3ZDM2ZTA5ZjEzNjUxN2VkZWM3MTk3NWUuLmIwZjg1OGQ5NTRi YTJjM2ZkOWI4OGFiNDQzYjVmNzM3OWJkNThhMDAgMTAwNjQ0Ci0tLSBhL1dlYnNpdGVzL2J1Z3Mu d2Via2l0Lm9yZy9DaGFuZ2VMb2cKKysrIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL0NoYW5n ZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDExLTEwLTEzICBBZGFtIEJhcnRoICA8YWJhcnRoQHdl YmtpdC5vcmc+CisKKyAgICAgICAgV291bGQgYmUgbmljZSBpZiB0aGUgcmV2aWV3LXRvb2wgb2Zm ZXJlZCB0byBzaG93IHRoZSBjb3JyZXNwb25kaW5nIGhlYWRlcgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjM5NjMKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIGlzIGEgZmlyc3QgY3V0IGF0IGFkZGlu ZyBsaW5rcyB0byB0aGUgaGVhZGVyIGZpbGUgaW4gdGhlIHJldmlldworICAgICAgICB0b29sLiAg SWYgZm9sa3MgbGlrZSB0aGlzIGZlYXR1cmUsIHdlIGNhbiBjb3ZlciBtb3JlIGNhc2VzIGluIHRo ZQorICAgICAgICBmdXR1cmUuCisKKyAgICAgICAgKiBjb2RlLXJldmlldy5qczoKKwogMjAxMS0w OS0xNCAgQmVuIFdlbGxzICA8YmVud2VsbHNAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFByZXR0 eVBhdGNoIHNob3VsZCBoYW5kbGUgImRlbHRhIiBwYXRjaCBtZWNoYW5pc20gaW4gZ2l0IGJpbmFy eSBwYXRjaGVzCmRpZmYgLS1naXQgYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZp ZXcuanMgYi9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZpZXcuanMKaW5kZXggOThl NDYwNmY3NWE1NjMxZjcyODhlMDdjZmI4ZGE3MzdkMDNhMTVjNS4uOGU5ZWU5YjFhNDIwZmVjNTA3 YzVlODE4NWY1OWRjY2UwNjliMTQ5NyAxMDA2NDQKLS0tIGEvV2Vic2l0ZXMvYnVncy53ZWJraXQu b3JnL2NvZGUtcmV2aWV3LmpzCisrKyBiL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9jb2RlLXJl dmlldy5qcwpAQCAtNjIzLDYgKzYyMywxNSBAQCB2YXIgQ09ERV9SRVZJRVdfVU5JVFRFU1Q7CiAg ICAgdmFyIHRyYWNfbGlua3MgPSAkKCc8YSB0YXJnZXQ9Il9ibGFuayI+YW5ub3RhdGU8L2E+PGEg dGFyZ2V0PSJfYmxhbmsiPnJldmlzaW9uIGxvZzwvYT4nKTsKICAgICB0cmFjX2xpbmtzWzBdLmhy ZWYgPSAnaHR0cDovL3RyYWMud2Via2l0Lm9yZy9icm93c2VyL3RydW5rLycgKyBmaWxlX25hbWUg KyAnP2Fubm90YXRlPWJsYW1lJyArIHVybF9oYXNoOwogICAgIHRyYWNfbGlua3NbMV0uaHJlZiA9 ICdodHRwOi8vdHJhYy53ZWJraXQub3JnL2xvZy90cnVuay8nICsgZmlsZV9uYW1lOworICAgIHZh ciBpbXBsZW1lbnRhdGlvbl9zdWZmaXhfbGlzdCA9IFsnLmNwcCcsICcubW0nXTsKKyAgICBmb3Ig KHZhciBpID0gMDsgaSA8IGltcGxlbWVudGF0aW9uX3N1ZmZpeF9saXN0Lmxlbmd0aDsgKytpKSB7 CisgICAgICAgIHZhciBzdWZmaXggPSBpbXBsZW1lbnRhdGlvbl9zdWZmaXhfbGlzdFtpXTsKKyAg ICAgICAgaWYgKGZpbGVfbmFtZS5sYXN0SW5kZXhPZihzdWZmaXgpID09IGZpbGVfbmFtZS5sZW5n dGggLSBzdWZmaXgubGVuZ3RoKSB7CisgICAgICAgICAgICB0cmFjX2xpbmtzLnByZXBlbmQoJzxh IHRhcmdldD0iX2JsYW5rIj5oZWFkZXI8L2E+Jyk7CisgICAgICAgICAgICB2YXIgc3RlbSA9IGZp bGVfbmFtZS5zdWJzdHIoMCwgZmlsZV9uYW1lLmxlbmd0aCAtIHN1ZmZpeC5sZW5ndGgpOworICAg ICAgICAgICAgdHJhY19saW5rc1swXS5ocmVmPSAnaHR0cDovL3RyYWMud2Via2l0Lm9yZy9sb2cv dHJ1bmsvJyArIHN0ZW0gKyAnLmgnOworICAgICAgICB9CisgICAgfQogICAgIHJldHVybiB0cmFj X2xpbmtzOwogICB9CiAK 110940 2011-10-13 17:16:38 -0700 2011-10-13 18:39:31 -0700 Patch bug-63963-20111013171637.patch text/plain 3767 abarth U3VidmVyc2lvbiBSZXZpc2lvbjogOTc0MjUKZGlmZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Vi a2l0Lm9yZy9DaGFuZ2VMb2cgYi9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvQ2hhbmdlTG9nCmlu ZGV4IGQxNDg5YTk2NDY1OGFhNjQ3ZDM2ZTA5ZjEzNjUxN2VkZWM3MTk3NWUuLmIwZjg1OGQ5NTRi YTJjM2ZkOWI4OGFiNDQzYjVmNzM3OWJkNThhMDAgMTAwNjQ0Ci0tLSBhL1dlYnNpdGVzL2J1Z3Mu d2Via2l0Lm9yZy9DaGFuZ2VMb2cKKysrIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL0NoYW5n ZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDExLTEwLTEzICBBZGFtIEJhcnRoICA8YWJhcnRoQHdl YmtpdC5vcmc+CisKKyAgICAgICAgV291bGQgYmUgbmljZSBpZiB0aGUgcmV2aWV3LXRvb2wgb2Zm ZXJlZCB0byBzaG93IHRoZSBjb3JyZXNwb25kaW5nIGhlYWRlcgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjM5NjMKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIGlzIGEgZmlyc3QgY3V0IGF0IGFkZGlu ZyBsaW5rcyB0byB0aGUgaGVhZGVyIGZpbGUgaW4gdGhlIHJldmlldworICAgICAgICB0b29sLiAg SWYgZm9sa3MgbGlrZSB0aGlzIGZlYXR1cmUsIHdlIGNhbiBjb3ZlciBtb3JlIGNhc2VzIGluIHRo ZQorICAgICAgICBmdXR1cmUuCisKKyAgICAgICAgKiBjb2RlLXJldmlldy5qczoKKwogMjAxMS0w OS0xNCAgQmVuIFdlbGxzICA8YmVud2VsbHNAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFByZXR0 eVBhdGNoIHNob3VsZCBoYW5kbGUgImRlbHRhIiBwYXRjaCBtZWNoYW5pc20gaW4gZ2l0IGJpbmFy eSBwYXRjaGVzCmRpZmYgLS1naXQgYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZp ZXctdGVzdC5odG1sIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL2NvZGUtcmV2aWV3LXRlc3Qu aHRtbAppbmRleCA1MjBmZDk1NWVlY2Y1NTJiMTMyNGQ0MTYxNjY4YzNkMjU0ZWI5MTY0Li40ODM0 MmM2MWRhOGZhOTVmNzQwM2Y3NWJmMjgyYWE2NTliNTM2YmM3IDEwMDY0NAotLS0gYS9XZWJzaXRl cy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZpZXctdGVzdC5odG1sCisrKyBiL1dlYnNpdGVzL2J1 Z3Mud2Via2l0Lm9yZy9jb2RlLXJldmlldy10ZXN0Lmh0bWwKQEAgLTIsNiArMiw3IEBACiA8ZGl2 PkZJWE1FOiBSdW4gdGhlc2UgYXMgcGFydCBvZiB0aGUgbGF5b3V0IHRlc3Qgc3VpdGU/PC9kaXY+ CiAKIDxzY3JpcHQ+Q09ERV9SRVZJRVdfVU5JVFRFU1QgPSB0cnVlPC9zY3JpcHQ+Cis8c2NyaXB0 IHNyYz0iaHR0cHM6Ly9hamF4Lmdvb2dsZWFwaXMuY29tL2FqYXgvbGlicy9qcXVlcnkvMS40LjIv anF1ZXJ5Lm1pbi5qcyI+PC9zY3JpcHQ+IAogPHNjcmlwdCBzcmM9ImNvZGUtcmV2aWV3LmpzIj48 L3NjcmlwdD4KIDxwcmUgaWQ9Im91dHB1dCI+PC9wcmU+CiA8c2NyaXB0PgpAQCAtNzUsNiArNzYs MTIgQEAgZnVuY3Rpb24gQVNTRVJUX0VRVUFMKGFjdHVhbCwgZXhwZWN0ZWQpIHsKICAgICAgICAg bG9nKCdGQUlMRUQ6XG5nb3Q6XG4nICsgYWN0dWFsICsgJ1xuZXhwZWN0ZWQ6XG4nICsgZXhwZWN0 ZWQgKyAnJyk7CiB9CiAKK3ZhciBsaW5rcyA9IHRyYWNMaW5rcygnZm9vL2Jhci9iYXouY3BwJywg JycpOworQVNTRVJUX0VRVUFMKCQobGlua3MpLmF0dHIoJ2hyZWYnKSwgJ2h0dHA6Ly90cmFjLndl YmtpdC5vcmcvbG9nL3RydW5rL2Zvby9iYXIvYmF6LmgnKTsKKwordmFyIGxpbmtzID0gdHJhY0xp bmtzKCdmb28vYmFyLmNwcC9xdXgubW0nLCAnJyk7CitBU1NFUlRfRVFVQUwoJChsaW5rcykuYXR0 cignaHJlZicpLCAnaHR0cDovL3RyYWMud2Via2l0Lm9yZy9sb2cvdHJ1bmsvZm9vL2Jhci5jcHAv cXV4LmgnKTsKKwogLy8gQmFzaWMgc2V0SXRlbS4KIHZhciBscyA9IG5ldyBNb2NrTG9jYWxTdG9y YWdlKCk7CiBuZXcgTW9ja0RyYWZ0Q29tbWVudFNhdmVyKCcxMjM0JywgbHMpLnNhdmUoKTsKZGlm ZiAtLWdpdCBhL1dlYnNpdGVzL2J1Z3Mud2Via2l0Lm9yZy9jb2RlLXJldmlldy5qcyBiL1dlYnNp dGVzL2J1Z3Mud2Via2l0Lm9yZy9jb2RlLXJldmlldy5qcwppbmRleCA5OGU0NjA2Zjc1YTU2MzFm NzI4OGUwN2NmYjhkYTczN2QwM2ExNWM1Li5iYWMyMjI0ZDNiMGY1NjkwMDM4NzQxMGMyZjgzYmM4 YzRiMDZhZTYzIDEwMDY0NAotLS0gYS9XZWJzaXRlcy9idWdzLndlYmtpdC5vcmcvY29kZS1yZXZp ZXcuanMKKysrIGIvV2Vic2l0ZXMvYnVncy53ZWJraXQub3JnL2NvZGUtcmV2aWV3LmpzCkBAIC0x ODQsNyArMTg0LDcgQEAgdmFyIENPREVfUkVWSUVXX1VOSVRURVNUOwogCiAgIGlmIChDT0RFX1JF VklFV19VTklUVEVTVCkKICAgICB3aW5kb3dbJ0RyYWZ0Q29tbWVudFNhdmVyJ10gPSBEcmFmdENv bW1lbnRTYXZlcjsKLSAgCisKICAgRHJhZnRDb21tZW50U2F2ZXIucHJvdG90eXBlLl9qc29uID0g ZnVuY3Rpb24oKSB7CiAgICAgdmFyIGNvbW1lbnRzID0gJCgnLmNvbW1lbnQnKTsKICAgICB2YXIg Y29tbWVudF9zdG9yZSA9IFtdOwpAQCAtNjIzLDkgKzYyMywyMSBAQCB2YXIgQ09ERV9SRVZJRVdf VU5JVFRFU1Q7CiAgICAgdmFyIHRyYWNfbGlua3MgPSAkKCc8YSB0YXJnZXQ9Il9ibGFuayI+YW5u b3RhdGU8L2E+PGEgdGFyZ2V0PSJfYmxhbmsiPnJldmlzaW9uIGxvZzwvYT4nKTsKICAgICB0cmFj X2xpbmtzWzBdLmhyZWYgPSAnaHR0cDovL3RyYWMud2Via2l0Lm9yZy9icm93c2VyL3RydW5rLycg KyBmaWxlX25hbWUgKyAnP2Fubm90YXRlPWJsYW1lJyArIHVybF9oYXNoOwogICAgIHRyYWNfbGlu a3NbMV0uaHJlZiA9ICdodHRwOi8vdHJhYy53ZWJraXQub3JnL2xvZy90cnVuay8nICsgZmlsZV9u YW1lOworICAgIHZhciBpbXBsZW1lbnRhdGlvbl9zdWZmaXhfbGlzdCA9IFsnLmNwcCcsICcubW0n XTsKKyAgICBmb3IgKHZhciBpID0gMDsgaSA8IGltcGxlbWVudGF0aW9uX3N1ZmZpeF9saXN0Lmxl bmd0aDsgKytpKSB7CisgICAgICB2YXIgc3VmZml4ID0gaW1wbGVtZW50YXRpb25fc3VmZml4X2xp c3RbaV07CisgICAgICBpZiAoZmlsZV9uYW1lLmxhc3RJbmRleE9mKHN1ZmZpeCkgPT0gZmlsZV9u YW1lLmxlbmd0aCAtIHN1ZmZpeC5sZW5ndGgpIHsKKyAgICAgICAgdHJhY19saW5rcy5wcmVwZW5k KCc8YSB0YXJnZXQ9Il9ibGFuayI+aGVhZGVyPC9hPicpOworICAgICAgICB2YXIgc3RlbSA9IGZp bGVfbmFtZS5zdWJzdHIoMCwgZmlsZV9uYW1lLmxlbmd0aCAtIHN1ZmZpeC5sZW5ndGgpOworICAg ICAgICB0cmFjX2xpbmtzWzBdLmhyZWY9ICdodHRwOi8vdHJhYy53ZWJraXQub3JnL2xvZy90cnVu ay8nICsgc3RlbSArICcuaCc7CisgICAgICB9CisgICAgfQogICAgIHJldHVybiB0cmFjX2xpbmtz OwogICB9CiAKKyAgaWYgKENPREVfUkVWSUVXX1VOSVRURVNUKQorICAgIHdpbmRvdy50cmFjTGlu a3MgPSB0cmFjTGlua3M7CisKICAgZnVuY3Rpb24gYWRkRXhwYW5kTGlua3MoZmlsZV9uYW1lKSB7 CiAgICAgaWYgKGZpbGVfbmFtZS5pbmRleE9mKCdDaGFuZ2VMb2cnKSAhPSAtMSkKICAgICAgIHJl dHVybjsKQEAgLTEwNTksNiArMTA3MSw4IEBAIHZhciBDT0RFX1JFVklFV19VTklUVEVTVDsKICAg fQogCiAgICQoZG9jdW1lbnQpLnJlYWR5KGZ1bmN0aW9uKCkgeworICAgIGlmIChDT0RFX1JFVklF V19VTklUVEVTVCkKKyAgICAgIHJldHVybjsKICAgICBjcmF3bERpZmYoKTsKICAgICBmZXRjaEhp c3RvcnkoKTsKICAgICAkKGRvY3VtZW50LmJvZHkpLnByZXBlbmQoJzxkaXYgaWQ9Im1lc3NhZ2Ui PicgKwo=