-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Iwehrman/rework code hint manager #2387
Iwehrman/rework code hint manager #2387
Conversation
… Glenn. This change also makes the problematic shouldShowHintsOnKey api no longer needed. It also resolves the conflicts between two or more providers by allowing each provider to register its specificity.
…rking on requires to be called for every keystokes.
…he default initial selection. This fixes issue adobe#2286
// so that we avoid registering a provider twice | ||
var providerName; | ||
for (providerName in hintProviders) { | ||
if (hintProviders.hasOwnProperty(providerName)) { |
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.
The hintProviders object members (indexes?) are modes, so I think this code would be more clear if "providerName" was changed to "modeName".
} else if (this.selectedIndex !== -1 && | ||
(keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_TAB)) { | ||
// Trigger a click handler to commmit the selected item | ||
$(this.$hintMenu.find("li")[this.selectedIndex]).triggerHandler("click"); |
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.
First, you need to check to see if anything is selected. If nothing is selected, then return. Otherwise, event.preventDefault() will be called (below) and the Return or Tab will not get inserted into the page.
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.
With the updated rotation logic, if nothing is selected then up/down navigation will always cause something to be selected. If the user presses tab/enter and nothing is selected, the default handler won't be prevented because the final else-if branch won't be entered and instead the final if-branch will return before the prevention.
Done with initial code review. |
if (!_inSession(editor) && lastChar) { | ||
_beginSession(editor); | ||
} else if (_inSession(editor)) { | ||
_updateHintList(); | ||
} |
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 can be simplified:
if (_inSession(editor)) {
_updateHintList();
} else {
if (lastChar) {
_beginSession(editor);
}
}
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.
Yep! Fixed.
Looks great. Merging. Thanks, Ian! |
Iwehrman/rework code hint manager
Implements the revised code hinting API as described at https://github.com/adobe/brackets/wiki/New-Code-Hinting-API-Proposal. Along with the previous commits, this directly addresses #1624 and #2258. The API should be flexible enough to allow a solution to #2286 as well.
cc @joelrbrandt @RaymondLim