Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Simple code hints preferences to enable/disable individual/all code hints providers. #8272

Merged
merged 3 commits into from
Jul 19, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions src/editor/CodeHintManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,23 @@ define(function (require, exports, module) {
CodeHintList = require("editor/CodeHintList").CodeHintList,
PreferencesManager = require("preferences/PreferencesManager");

var hintProviders = { "all" : [] },
lastChar = null,
sessionProvider = null,
sessionEditor = null,
hintList = null,
deferredHints = null,
keyDownEditor = null;
var hintProviders = { "all" : [] },
lastChar = null,
sessionProvider = null,
sessionEditor = null,
hintList = null,
deferredHints = null,
keyDownEditor = null,
codeHintsEnabled = true;


PreferencesManager.definePreference("showCodeHints", "boolean", true);
PreferencesManager.definePreference("insertHintOnTab", "boolean", false);


PreferencesManager.on("change", "showCodeHints", function () {
codeHintsEnabled = PreferencesManager.get("showCodeHints");
});

/**
* Comparator to sort providers from high to low priority
*/
Expand Down Expand Up @@ -347,7 +352,15 @@ define(function (require, exports, module) {
* @return {?Array.<{provider: Object, priority: number}>}
*/
function _getProvidersForLanguageId(languageId) {
return hintProviders[languageId] || hintProviders.all;
var providers = hintProviders[languageId] || hintProviders.all;

// Exclude providers that are explicitly disabled in the preferences.
// All code hint providers that do not have their constructor
// names listed in the preferences are enabled by default.
return providers.filter(function (provider) {
var prefKey = "codehint." + provider.provider.constructor.name;
return PreferencesManager.get(prefKey) !== false;
});
}

var _beginSession;
Expand Down Expand Up @@ -451,6 +464,10 @@ define(function (require, exports, module) {
* @param {Editor} editor
*/
_beginSession = function (editor) {
if (!codeHintsEnabled) {
return;
}

// Don't start a session if we have a multiple selection.
if (editor.getSelections().length > 1) {
return;
Expand Down
40 changes: 33 additions & 7 deletions src/extensions/default/JavaScriptCodeHints/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,42 @@ define(function (require, exports, module) {
Session = require("Session"),
Acorn = require("thirdparty/acorn/acorn");

var session = null, // object that encapsulates the current session state
cachedCursor = null, // last cursor of the current hinting session
cachedHints = null, // sorted hints for the current hinting session
cachedType = null, // describes the lookup type and the object context
cachedToken = null, // the token used in the current hinting session
matcher = null, // string matcher for hints
ignoreChange; // can ignore next "change" event if true;
var session = null, // object that encapsulates the current session state
cachedCursor = null, // last cursor of the current hinting session
cachedHints = null, // sorted hints for the current hinting session
cachedType = null, // describes the lookup type and the object context
cachedToken = null, // the token used in the current hinting session
matcher = null, // string matcher for hints
jsHintsEnabled = true, // preference setting to enable/disable the hint session
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code never gets the preferences to initialize jsHintsEnabled. This only gets set correctly when pref is changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we're initializing the default value regardless of whether the prefs are available or not. This is why I use it on the 3rd param to definePreference on line 68. But I can see that you want the other way around. I'm fine to hard-code true for the 3rd param to definePreference. Then if we ever want to flip the default, we have to update these two places at the same time.

ignoreChange; // can ignore next "change" event if true;


// Define the defaultExclusions which are files that are known to cause Tern to run out of control.
PreferencesManager.definePreference("jscodehints.defaultExclusions", "array", []);

// This preference controls when Tern will time out when trying to understand files
PreferencesManager.definePreference("jscodehints.inferenceTimeout", "number", 5000);

// This preference controls whether to create a session and process all JS files or not.
PreferencesManager.definePreference("codehint.JSHints", "boolean", true);

/**
* Check whether any of code hints preferences for JS Code Hints is disabled
* @return {boolean} enabled/disabled
*/
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();
});

Copy link
Contributor

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();
    });

Copy link
Contributor

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.

Copy link
Contributor Author

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.

/**
* Sets the configuration, generally for testing/debugging use.
* Configuration keys are merged into the current configuration.
Expand Down Expand Up @@ -586,6 +608,10 @@ define(function (require, exports, module) {
// always clean up cached scope and hint info
resetCachedHintContext();

if (!jsHintsEnabled) {
return;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should check this after resetCachedHintContext().

if (editor && HintUtils.isSupportedLanguage(LanguageManager.getLanguageForPath(editor.document.file.fullPath).getId())) {
initializeSession(editor, previousEditor);
$(editor)
Expand Down