[Webkit-unassigned] [Bug 86367] Web Inspector: Use CSS columns feature for HelpScreen contents.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 14 22:49:03 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=86367





--- Comment #11 from eustas.bug at gmail.com  2012-05-14 22:48:07 PST ---
(From update of attachment 141714)
View in context: https://bugs.webkit.org/attachment.cgi?id=141714&action=review

>> Source/WebCore/inspector/front-end/SettingsScreen.js:141
>> +    _appendSection: function(name, container)
> 
> Do we ever pass different containers to append section? Looks like making container a member will simplify things.

Adding members for the sake of simplicity is not a good practice.

Adding temporary members used only in constructor lifetime is a bad practice.

IMHO nested function will match better.

>> Source/WebCore/inspector/front-end/ShortcutsScreen.js:81
>> +        for (var i = 0 ; i < orderedSections.length; ++i) {
> 
> no braces around single-line blocks

Fixed. Thanks.

>> Source/WebCore/inspector/front-end/ShortcutsScreen.js:131
>> +        var i, keyCell, line;
> 
> Why extract all the variable declarations?

To avoid accidental duplicate declaration.

Variable declaration at any place inside scope is semantically equivalent to declaration at the beginning of context.
So explicit declaration at the beginning makes code much more clear.

Quote: "...the isolated scope... is created only by execution contexts with 'function' code type... in contrast with C/C++... the block of for loop... does not create a local context".
Reference: http://ecma262-5.com/ELS5_Section_10.htm#Section_10.3

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list