[webkit-reviews] review denied: [Bug 71625] Web Inspector: Preserve an indentation level when inserting a new line : [Attachment 115789] Add normalize method

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Nikita Vasilyev
<me at elv1s.ru>'s request for review:
Bug 71625: Web Inspector: Preserve an indentation level when inserting a new
line
https://bugs.webkit.org/show_bug.cgi?id=71625

Attachment 115789: Add normalize method
https://bugs.webkit.org/attachment.cgi?id=115789&action=review

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


Looks good with a request for clarification on {} and a nit. Also, it would be
nice to unindent upon closing bracket in case we produced the auto-indent
(could be done in a separate change).

> Source/WebCore/inspector/front-end/TextEditorModel.js:53
> +    collapseToEnd: function()

I would keep the text range immutable and return the new object from these
methods.

> Source/WebCore/inspector/front-end/TextEditorModel.js:116
> +    get lineBreak() {

{ should be on the next line.

> Source/WebCore/inspector/front-end/TextViewer.js:1060
> +	   range.normalize();

I.e. that would be var range = selection.normalize(); here.

> Source/WebCore/inspector/front-end/TextViewer.js:-1059
> -	       range = new WebInspector.TextRange(range.endLine,
range.endColumn, range.startLine, range.startColumn);

As it actually was.

> Source/WebCore/inspector/front-end/TextViewer.js:1168
> +	       newRange = this._setText(range, lineBreak + indent + lineBreak +
currentIndent);

What's happening here? Could you comment on this case?

> Source/WebCore/inspector/front-end/TextViewer.js:1471
> +	   return new WebInspector.TextRange(start.line, start.column,
end.line, end.column);

So the selection is no longer normalized. It seems like restoreSelection still
handles it ok.