[webkit-reviews] review denied: [Bug 81758] Web Inspector: Indenting fully selected line should not indent the line next to it : [Attachment 133214] Add a test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Mar 24 02:06:45 PDT 2012


Pavel Feldman <pfeldman at chromium.org> has denied Nikita Vasilyev
<me at elv1s.ru>'s request for review:
Bug 81758: Web Inspector: Indenting fully selected line should not indent the
line next to it
https://bugs.webkit.org/show_bug.cgi?id=81758

Attachment 133214: Add a test
https://bugs.webkit.org/attachment.cgi?id=133214&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133214&action=review


The change looks good. Could you please add more test scenarios and check for
the post-indent / unindent selection as well?

> LayoutTests/inspector/editor/indenting.html:6
> +function dumpTextModel(model) {

Please remove this.

> LayoutTests/inspector/editor/indenting.html:14
> +	   "*/";

You might want to indent this one char so that it looked like properly
formatted comment and dump the initial state.

> LayoutTests/inspector/editor/indenting.html:21
> +    function dumpTextModel(msg) {

nit: { on the next line.

> LayoutTests/inspector/editor/indenting.html:39
> +This test checks code indentation.

Please provide more verbose description. I would call the test
"indentation.html" as well.

> Source/WebCore/inspector/front-end/TextViewer.js:1109
> +	   if (range.startColumn !== 0)

Placing a comment that we distinguish between full and partial line selection
won't hurt. This could actually be tested as well. I.e. query selection after
the indentation in both cases (full line select and partial line select).


More information about the webkit-reviews mailing list