Bug 52377 – Web Inspector: provide script column offset to frontend

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Podivilov]

Web Inspector: provide script column offset to frontend. This is needed for script beautification and live edit in HTML.

[Pavel Podivilov]

Created attachment 78825 [details]
Patch.

[Yury Semikhatsky]

Comment on attachment 78825 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=78825&action=review

> Source/JavaScriptCore/parser/SourceCode.h:33
> +#include <wtf/text/TextPosition.h>

Mind alphabetical order.

> Source/JavaScriptCore/parser/SourceCode.h:54
> +            , m_columnOffset(0)

Why not pass column as additional parameter?

> Source/JavaScriptCore/parser/SourceCode.h:63
> +            , m_columnOffset(0)

Why not pass column as additional parameter?

> Source/JavaScriptCore/parser/SourceCode.h:88
> +        int lineOffset() const { return m_lineOffset; }

There should be only one of firstLine/lineOffset. Also offset is already used as a linear position in the source string and using the same term for line/column might confuse people.

[Pavel Podivilov]

Created attachment 78928 [details]
Comments addressed.

[Yury Semikhatsky]

Comment on attachment 78928 [details]
Comments addressed.

View in context: https://bugs.webkit.org/attachment.cgi?id=78928&action=review

> Source/WebCore/inspector/front-end/Script.js:31
> +    this.lineOffset = lineOffset;

Consider using the same names as in SourceCode.h for consistency.

> Source/WebCore/inspector/front-end/Script.js:36
> +    this.startingLine = lineOffset + 1;

Please remove this field and use only lineOffset, clients should add 1 when needed.

[Pavel Podivilov]

Committed r75794: <http://trac.webkit.org/changeset/75794>