Bug 106475 – Web Inspector: Correct formatting in inspector-test.js

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

[johnjbarton]

The function initialize_InspectorTest in inspector-test is injected as a string into the WebInspector window. Figuring out what content is in that string is hard because the function spans many lines and its body is not indented per the standard formatting in other files.

Patch to follow

[johnjbarton]

Created attachment 181956 [details]
Patch

[johnjbarton]

diff looks horrible but the change is just formatting.

[Pavel Feldman]

Comment on attachment 181956 [details]
Patch

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

> LayoutTests/http/tests/inspector/inspector-test.js:333
> +    InspectorTest.addConsoleSniffer = function(override, opt_sticky)

As I mentioned, we don't indent namespaces / closures like this. Like this like looks really weird now.

[johnjbarton]

(In reply to comment #3)
> (From update of attachment 181956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181956&action=review
> 
> > LayoutTests/http/tests/inspector/inspector-test.js:333
> > +    InspectorTest.addConsoleSniffer = function(override, opt_sticky)
> 
> As I mentioned, we don't indent namespaces / closures like this. Like this like looks really weird now.

Can you help me understand? What makes this function different from other functions? I don't know what a namespace is in JavaScript and we have many other closures which are indented. (I also think that from a practical point of view this rule is bizarre but at least I'd like to know when to apply it).

[Pavel Feldman]

> Can you help me understand? What makes this function different from other functions? I don't know what a namespace is in JavaScript and we have many other closures which are indented. (I also think that from a practical point of view this rule is bizarre but at least I'd like to know when to apply it).

Whenever the closure is added for the sake of the harness or framework with the sole purpose of serializing the content (such as this case and injected scripts), I think it does not make sense to indent. If JavaScript allowed us serializing and sending over entire files, we would do that instead of coming up with such closures.

[Brian Burg]

Closing as invalid, as this bug pertains to the old inspector UI and/or its tests.
Please file a new bug (https://www.webkit.org/new-inspector-bug) if the bug/feature/issue is still relevant to WebKit trunk.