-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Simple code hints preferences to enable/disable individual/all code hints providers. #8272
Conversation
…l code hints providers.
CSShint:false, it's doesn't work... |
@hkongm Are you sure you're trying them with my changes and adding them into the correct brackets.json file? Note that your try of |
{ every option does work, but the codehints ... my plugins list: |
@hkongm I'm still confused with what you're saying. Just having |
@hkongm I replaced my brackets.json file with yours and I'm still able to disable all code hints on my Win 7. BTW, which OS platform are you using? And also, how do you put my changes into your local build? Are you replacing your local version of src/editor/CodeHintManager.js with the changes in this pull request? Or are you switching to rlim/code-hints-pref branch in your local Brackets repo? |
osx 10.9.4 I will abort this issue, after all this is not a big deal. thanks again, @RaymondLim |
@hkongm Since you're running sprint 41 build on Mac, you don't have my changes to test. In order to test with my changes, you can read this How to Hack on Brackets to set up your local brackets repo and then switch to my branch (which is not yet landed in master). Or you have to wait for the next release after this pull request is merged. |
Triage Complete. |
@@ -501,6 +518,9 @@ define(function (require, exports, module) { | |||
* @param {Editor} editor | |||
*/ | |||
function _startNewSession(editor) { | |||
if (!codeHintsEnabled) { | |||
return; | |||
} |
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.
I don't think this check is necessary since it calls _beginSession()
.
After turning off JS Code Hints, I notice that the JavaScriptCodeHints extension still processes files. You should try to prevent files from being processed so this change also helps performance. |
Done with initial review. |
@redmunds All of your suggestions are taking care of. Ready for re-review. |
jsHintsEnabled = PreferencesManager.get("codehint.JSHints"); | ||
} | ||
}); | ||
|
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.
This logic is not quite right. jsHintsEnabled
needs to be recomputed whenever either of the prefs change, right? Also, if pref is not set (i.e. undefined), then default is true (not false). Something like this:
function areHintsEnabled() {
return (PreferencesManager.get("codehint.JSHints") !== false) && (PreferencesManager.get("showCodeHints") !== false);
}
PreferencesManager.on("change", "codehint.JSHints", function () {
jsHintsEnabled = areHintsEnabled();
});
PreferencesManager.on("change", "showCodeHints", function () {
jsHintsEnabled = areHintsEnabled();
});
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.
if pref is not set (i.e. undefined), then default is true (not false).
Both of these prefs have default values, so this statement is not correct, but there is no UI so user needs to manually set values, so it's still a good idea to compare that returned value is !== false
.
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.
@redmunds Thanks for the code. You're right that I forgot to consider the cases where either of the prefs is not set.
Done with review. |
@redmunds Changes are pushed. Ready for re-review. |
Looks good. Merging. |
Simple code hints preferences to enable/disable individual/all code hints providers.
This fixes #8173. You can use one or more of the following preferences to turn on/off individual hints provider.
By default,
showCodeHints
is true and you don't need any of the following preferences to use the existing code hints providers. When you want to disable any of them, just add"codehint.<HINT_PROVIDER_CONSTRUCTOR_NAME>": false
in the brackets.json file.