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

Only query hint providers that have offered to show hints #2261

Closed
wants to merge 3 commits into from
Closed

Only query hint providers that have offered to show hints #2261

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a62ea1
Only query hint providers that have offered to show hints
iwehrman Dec 1, 2012
16da1ef
Don't explicitly close the hint list when shouldShowHintsOnKey is false
iwehrman Dec 3, 2012
9110664
initialize queryAllProviders to true in case hinting isn't preceded b…
iwehrman Dec 4, 2012
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
39 changes: 25 additions & 14 deletions src/editor/CodeHintManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ define(function (require, exports, module) {


var hintProviders = [],
enabledHintProviders = [],
queryAllProviders,
hintList,
shouldShowHintsOnChange = false,
keyDownEditor;


Expand Down Expand Up @@ -257,13 +258,19 @@ define(function (require, exports, module) {
* @param {Editor} editor
*/
CodeHintList.prototype.open = function (editor) {
var self = this;
var self = this,
providers = enabledHintProviders;

this.editor = editor;

Menus.closeAll();

if (queryAllProviders) {
providers = hintProviders;
}

this.currentProvider = null;
$.each(hintProviders, function (index, item) {
$.each(providers, function (index, item) {
var query = item.getQueryInfo(self.editor, self.editor.getCursorPos());
if (query.queryStr !== null) {
self.query = query;
Expand All @@ -284,7 +291,7 @@ define(function (require, exports, module) {
var hintPos = this.calcHintListLocation();

this.$hintMenu.addClass("open")
.css({"left": hintPos.left, "top": hintPos.top});
.css({"left": hintPos.left, "top": hintPos.top});
this.opened = true;

PopUpManager.addPopUp(this.$hintMenu,
Expand All @@ -311,7 +318,6 @@ define(function (require, exports, module) {
this.$hintMenu.remove();
if (hintList === this) {
hintList = null;
shouldShowHintsOnChange = false;
keyDownEditor = null;
}
};
Expand Down Expand Up @@ -386,23 +392,25 @@ define(function (require, exports, module) {
* @param {KeyboardEvent} event
*/
function handleKeyEvent(editor, event) {
var provider = null;

// Check for Control+Space
if (event.type === "keydown" && event.keyCode === 32 && event.ctrlKey) {
// shouldShowHintsOnKey will not be called in this case,
// so the entire list of providers will be queried
queryAllProviders = true;
showHint(editor);
event.preventDefault();
} else if (event.type === "keypress") {
// Check if any provider wants to show hints on this key.
// otherwise, shouldShowHintsOnKey determines which providers
// will be queried (from handleChange) for hints
queryAllProviders = false;
enabledHintProviders = [];
$.each(hintProviders, function (index, item) {
if (item.shouldShowHintsOnKey(String.fromCharCode(event.charCode))) {
provider = item;
return false;
enabledHintProviders.push(item);
}
});

shouldShowHintsOnChange = !!provider;
if (shouldShowHintsOnChange) {
if (enabledHintProviders.length > 0) {
keyDownEditor = editor;
}
}
Expand All @@ -417,10 +425,13 @@ define(function (require, exports, module) {
*
*/
function handleChange(editor) {
if (shouldShowHintsOnChange && keyDownEditor === editor) {
shouldShowHintsOnChange = false;
if (enabledHintProviders.length > 0 && keyDownEditor === editor) {
keyDownEditor = null;
showHint(editor);
} else {
if (hintList) {
hintList.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you would like to close the hintlist here? If you do that, then as soon as you type the very first letter the hint list will get closed. Try with <h in a html page and you will see what I mean.

Copy link

Choose a reason for hiding this comment

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

I can only guess why, but in short t'is to emphasize shouldShowHintsOnKey. I just tried this code with both html and css hinting. With this implementation the hints disappear whenever you press a key, that is not handled in shouldShowHintsOnKey (in your example the '<' will trigger+show html hints, but the 'h' will return false in SSHOK) - for css hinting this kind-of works, because I (naive thinking) added the whole alphabet a-z in SSHOK to return true, thus the hints stay visible.

To get this working one must change shouldShowHintsOnKey to include all keys (e.g. a-z) when to show hints in every provider - though I'm afraid this wouldn't solve the problem since every provider would return true and again every provider calls getQueryInfo
(as far as I understand this change.)
Regards, André

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the original intent was to use shouldShowHintsOnKey to determine whether to open or close the hint list. But indeed this breaks the HTML hinting and isn't necessary for the JS hinting that I'm working on separately. I will amend the pull request.

}
}
}

Expand Down