[webkit-reviews] review denied: [Bug 108697] Web Inspector: highlight matching braces in DTE. : [Attachment 186783] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 6 04:07:12 PST 2013


Pavel Feldman <pfeldman at chromium.org> has denied Andrey Lushnikov
<lushnikov at chromium.org>'s request for review:
Bug 108697: Web Inspector: highlight matching braces in DTE.
https://bugs.webkit.org/show_bug.cgi?id=108697

Attachment 186783: Patch
https://bugs.webkit.org/attachment.cgi?id=186783&action=review

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


Overall looks good. Please migrate the {} auto-indent code to the new matcher
to test if the API is universal enough.

> Source/WebCore/inspector/front-end/TextEditorHighlighter.js:199
> +			   else

{} around multiline block please.

> Source/WebCore/inspector/front-end/TextEditorModel.js:676
> +	   if (lineNumber >= this._textModel.linesCount || lineNumber < 0)

When does this happen?

> Source/WebCore/inspector/front-end/TextEditorModel.js:702
> +    findLeftPretendent: function(lineNumber, column, maxBraceIteration)

Pretendent -> Candidate

> Source/WebCore/inspector/front-end/TextEditorModel.js:796
> +

You don't need to make every other line a whitespace.


More information about the webkit-reviews mailing list