-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unified code style #63
Conversation
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
This reverts commit 7221964. Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
- Don’t use strict version of JS for now - Switch back to get_settings for getting settings - Check in filtertypes.js, whether we are running a unit test or not in order to not break AdBlock while not being in an unit test Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
} | ||
return subscribed_filter_names; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless semicolon. Braces indicating end of multi-line statement here, so semicolon indicating end of single-line statement not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kpeckett There really should be a semicolon :) After each declaration of variable needs to be a semicolon, an exception occurs at the end of a named function e.g. :
function foo(bar) { ... }
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@@ -1355,22 +1399,22 @@ function isSafariContentBlockingAvailable() { | |||
return (SAFARI && | |||
safari && | |||
safari.extension && | |||
(typeof safari.extension.setContentBlocker === 'function')); | |||
(typeof safari.extension.setContentBlocker === "function")); | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function header format - are we using anon functions attached to variables or named functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kpeckett We should use named functions, mainly because of "hoisting" and performance reasons.
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@@ -559,7 +559,7 @@ var count_cache = (function(count_map) { | |||
var cache = count_map; | |||
|
|||
// Update custom filter count stored in localStorage | |||
var _updateCustomFilterCount = function() { | |||
function _updateCustomFilterCount() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this right? It looks like a local function becomes global with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kpeckett Yes, it's right as part of the switching from function expressions to function declarations. The scope of the function hasn't been changed.
LGTM. Merging. |
Signed-off-by: Tomáš Taro tomas@getadblock.com
This PR also contains: