Bug 63963 – Would be nice if the review-tool offered to show the corresponding header

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

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.

[Ojan Vafai]

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?

[Adam Barth]

In the links that appear on the right (like "blame") you could have a link called "header" that went to trac.

[Adam Barth]

Created attachment 99793 [details]
Untested, probably broken

[Ojan Vafai]

Comment on attachment 99793 [details]
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".

[Adam Barth]

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.

[Adam Barth]

Created attachment 110933 [details]
Patch

[Ojan Vafai]

Comment on attachment 110933 [details]
Patch

Code looks fine. Can you add a test? code-review-test.html

[Adam Barth]

> Can you add a test?

What fun would that be!  :)

[Adam Barth]

Created attachment 110940 [details]
Patch

[WebKit Review Bot]

Comment on attachment 110940 [details]
Patch

Clearing flags on attachment: 110940

Committed r97435: <http://trac.webkit.org/changeset/97435>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.