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

Fix for Issue 1617: Removed all references to CodeHintManager from Editor #5421

Merged
merged 5 commits into from
Oct 12, 2013
Merged

Fix for Issue 1617: Removed all references to CodeHintManager from Editor #5421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9813eb
Remove all references to CodeHintManager
WebsiteDeveloper Oct 4, 2013
e3e9a68
Revert some whitespace changes
WebsiteDeveloper Oct 8, 2013
80c862a
Fix minor nits
WebsiteDeveloper Oct 12, 2013
825fb1e
Merged with master
WebsiteDeveloper Oct 12, 2013
0311641
Fix one minor nit
WebsiteDeveloper Oct 12, 2013
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
138 changes: 79 additions & 59 deletions src/editor/CodeHintManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
/*
* Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved.
*
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*
*/

/*
Expand Down Expand Up @@ -91,36 +91,36 @@
* to show hints. Because implicit hints can be triggered by every
* character insertion, hasHints may be called frequently; consequently,
* the provider should endeavor to return a value as quickly as possible.
*
*
* Because calls to hasHints imply that a hinting session is about to
* begin, a provider may wish to clean up cached data from previous
* sessions in this method. Similarly, if the provider returns true, it
* may wish to prepare to cache data suitable for the current session. In
* particular, it should keep a reference to the editor object so that it
* can access the editor in future calls to getHints and insertHints.
*
* param {Editor} editor
*
* param {Editor} editor
* A non-null editor object for the active window.
*
* param {string} implicitChar
* param {string} implicitChar
* Either null, if the hinting request was explicit, or a single character
* that represents the last insertion and that indicates an implicit
* hinting request.
*
* return {boolean}
* return {boolean}
* Determines whether the current provider is able to provide hints for
* the given editor context and, in case implicitChar is non- null,
* whether it is appropriate to do so.
*
*
*
*
* # CodeHintProvider.getHints(implicitChar)
*
*
* The method by which a provider provides hints for the editor context
* associated with the current session. The getHints method is called only
* if the provider asserted its willingness to provide hints in an earlier
* call to hasHints. The provider may return null or false, which indicates
* call to hasHints. The provider may return null or false, which indicates
* that the manager should end the current hinting session and close the hint
* list window; or true, which indicates that the manager should end the
* list window; or true, which indicates that the manager should end the
* current hinting session but immediately attempt to begin a new hinting
* session by querying registered providers. Otherwise, the provider should
* return a response object that contains the following properties:
Expand Down Expand Up @@ -156,7 +156,7 @@
* deferred object from the current call has resolved or been rejected. If
* the provider rejects the deferred, the manager will end the hinting
* session.
*
*
* The getHints method may be called by the manager repeatedly during a
* hinting session. Providers may wish to cache information for efficiency
* that may be useful throughout these sessions. The same editor context
Expand All @@ -177,25 +177,25 @@
* match: string,
* selectInitial: boolean,
* handleWideResults: boolean}}
*
*
* Null if the provider wishes to end the hinting session. Otherwise, a
* response object, possibly deferred, that provides 1. a sorted array
* hints that consists either of strings or jQuery objects; 2. a string
* match, possibly null, that is used by the manager to emphasize
* matching substrings when rendering the hint list; and 3. a boolean that
* indicates whether the first result, if one exists, should be selected
* by default in the hint list window. If match is non-null, then the
* hints should be strings.
*
* If the match is null, the manager will not
* attempt to emphasize any parts of the hints when rendering the hint
* list; instead the provider may return strings or jQuery objects for
* hints should be strings.
*
* If the match is null, the manager will not
* attempt to emphasize any parts of the hints when rendering the hint
* list; instead the provider may return strings or jQuery objects for
* which emphasis is self-contained. For example, the strings may contain
* substrings that wrapped in bold tags. In this way, the provider can
* substrings that wrapped in bold tags. In this way, the provider can
* choose to let the manager handle emphasis for the simple and common case
* of prefix matching, or can provide its own emphasis if it wishes to use
* of prefix matching, or can provide its own emphasis if it wishes to use
* a more sophisticated matching algorithm.
*
*
*
* # CodeHintProvider.insertHint(hint)
*
Expand All @@ -209,18 +209,18 @@
* explicit hinting request, which may result in a new hinting session
* being opened with some provider, but not necessarily the current one.
*
* param {string} hint
* param {string} hint
* The hint to be inserted into the editor context for the current session.
*
* return {boolean}
*
* return {boolean}
* Indicates whether the manager should follow hint insertion with an
* explicit hint request.
*
*
* # CodeHintProvider.insertHintOnTab
*
*
* type {?boolean} insertHintOnTab
* Indicates whether the CodeHintManager should request that the provider of
* Indicates whether the CodeHintManager should request that the provider of
* the current session insert the currently selected hint on tab key events,
* or if instead a tab character should be inserted into the editor. If omitted,
* the fallback behavior is determined by the CodeHintManager. The default
Expand Down Expand Up @@ -275,12 +275,12 @@ define(function (require, exports, module) {
return b.priority - a.priority;
}

/**
/**
* The method by which a CodeHintProvider registers its willingness to
* providing hints for editors in a given language.
*
* @param {!CodeHintProvider} provider
* The hint provider to be registered, described below.
* The hint provider to be registered, described below.
*
* @param {!Array.<string>} languageIds
* The set of language ids for which the provider is capable of
Expand Down Expand Up @@ -353,7 +353,7 @@ define(function (require, exports, module) {
});
}

/**
/**
* Return the array of hint providers for the given language id.
* This gets called (potentially) on every keypress. So, it should be fast.
*
Expand Down Expand Up @@ -384,14 +384,14 @@ define(function (require, exports, module) {
}
}

/**
/**
* Is there a hinting session active for a given editor?
*
*
* NOTE: the sessionEditor, sessionProvider and hintList objects are
* only guaranteed to be initialized during an active session.
*
* only guaranteed to be initialized during an active session.
*
* @param {Editor} editor
* @return boolean
* @return boolean
*/
function _inSession(editor) {
if (sessionEditor) {
Expand Down Expand Up @@ -433,7 +433,7 @@ define(function (require, exports, module) {
_beginSession(previousEditor);
} else if (response.hasOwnProperty("hints")) { // a synchronous response
if (hintList.isOpen()) {
// the session is open
// the session is open
hintList.update(response);
} else {
hintList.open(response);
Expand All @@ -442,7 +442,7 @@ define(function (require, exports, module) {
deferredHints = response;
response.done(function (hints) {
if (hintList.isOpen()) {
// the session is open
// the session is open
hintList.update(hints);
} else {
hintList.open(hints);
Expand All @@ -453,7 +453,7 @@ define(function (require, exports, module) {
}

/**
* Try to begin a new hinting session.
* Try to begin a new hinting session.
* @param {Editor} editor
*/
_beginSession = function (editor) {
Expand Down Expand Up @@ -497,7 +497,7 @@ define(function (require, exports, module) {
};

/**
* Explicitly start a new session. If we have an existing session,
* Explicitly start a new session. If we have an existing session,
* then close the current one and restart a new one.
* @param {Editor} editor
*/
Expand All @@ -517,18 +517,19 @@ define(function (require, exports, module) {
}

/**
* Handles keys related to displaying, searching, and navigating the hint list.
* Handles keys related to displaying, searching, and navigating the hint list.
* This gets called before handleChange.
*
* TODO: Ideally, we'd get a more semantic event from the editor that told us
* what changed so that we could do all of this logic without looking at
* key events. Then, the purposes of handleKeyEvent and handleChange could be
* combined. Doing this well requires changing CodeMirror.
*
* @param {Event} jqEvent
* @param {Editor} editor
* @param {KeyboardEvent} event
*/
function handleKeyEvent(editor, event) {
function _handleKeyEvent(jqEvent, editor, event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to update the JSDoc above by adding the first param jqEvent. Also, please change the function to _handleKeyEvents (plural) so that it has the same name as the one in Editor module.

keyDownEditor = editor;
if (event.type === "keydown") {
if (!(event.ctrlKey || event.altKey || event.metaKey) &&
Expand Down Expand Up @@ -560,19 +561,23 @@ define(function (require, exports, module) {
}

/**
* Start a new implicit hinting session, or update the existing hint list.
* Start a new implicit hinting session, or update the existing hint list.
* Called by the editor after handleKeyEvent, which is responsible for setting
* the lastChar.
*
* @param {Event} event
* @param {Editor} editor
* @param {{from: Pos, to: Pos, text: Array, origin: string}} changeList
*/
function handleChange(editor, changeList) {
function _handleChange(event, editor, changeList) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add all params to JSDoc.

if (lastChar && editor === keyDownEditor) {
keyDownEditor = null;
if (_inSession(editor)) {
var charToRetest = lastChar;
_updateHintList();

// _updateHintList() may end a hinting session and clear lastChar, but a
// different provider may want to start a new session with the same character.
// _updateHintList() may end a hinting session and clear lastChar, but a
// different provider may want to start a new session with the same character.
// So check whether current provider terminates the current hinting
// session. If so, then restore lastChar and restart a new session.
if (!_inSession(editor)) {
Expand Down Expand Up @@ -618,9 +623,26 @@ define(function (require, exports, module) {
return hintList;
}

function activeEditorChangeHandler(event, current, previous) {
if (current) {
$(current).on("editorChange", _handleChange);
$(current).on("keyEvent", _handleKeyEvent);
}

if (previous) {
//Removing all old Handlers
$(previous).off("editorChange", _handleChange);
$(previous).off("keyEvent", _handleKeyEvent);
}
}

activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null);

$(EditorManager).on("activeEditorChange", activeEditorChangeHandler);

// Dismiss code hints before executing any command since the command
// may make the current hinting session irrevalent after execution.
// For example, when the user hits Ctrl+K to open Quick Doc, it is
// may make the current hinting session irrevalent after execution.
// For example, when the user hits Ctrl+K to open Quick Doc, it is
// pointless to keep the hint list since the user wants to view the Quick Doc.
$(CommandManager).on("beforeExecuteCommand", _endSession);

Expand All @@ -631,8 +653,6 @@ define(function (require, exports, module) {

// Define public API
exports.isOpen = isOpen;
exports.handleKeyEvent = handleKeyEvent;
exports.handleChange = handleChange;
exports.registerHintProvider = registerHintProvider;
exports.hasValidExclusion = hasValidExclusion;
exports.setInsertHintOnTab = setInsertHintOnTab;
Expand Down
17 changes: 9 additions & 8 deletions src/editor/Editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@
define(function (require, exports, module) {
"use strict";

var CodeHintManager = require("editor/CodeHintManager"),
Commands = require("command/Commands"),
var Commands = require("command/Commands"),
CommandManager = require("command/CommandManager"),
Menus = require("command/Menus"),
PerfUtils = require("utils/PerfUtils"),
Expand Down Expand Up @@ -269,9 +268,6 @@ define(function (require, exports, module) {

function _handleKeyEvents(jqEvent, editor, event) {
_checkElectricChars(jqEvent, editor, event);

// Pass the key event to the code hint manager. It may call preventDefault() on the event.
CodeHintManager.handleKeyEvent(editor, event);
}

/**
Expand Down Expand Up @@ -536,7 +532,7 @@ define(function (require, exports, module) {
return;
}
}

// Apply text changes to CodeMirror editor
var cm = this._codeMirror;
cm.operation(function () {
Expand Down Expand Up @@ -597,13 +593,18 @@ define(function (require, exports, module) {
// note: this change might have been a real edit made by the user, OR this might have
// been a change synced from another editor

CodeHintManager.handleChange(this, changeList);
// The "editorChange" event is mostly for the use of the CodeHintManager.
// It differs from the normal "change" event, that it's actually publicly usable,
// whereas the "change" event should be listened to on the document. Also the
// Editor dispatches a change event before this event is dispatched, because
// CodeHintManager needs to hook in here when other things are already done.
$(this).triggerHandler("editorChange", [this, changeList]);
};

/**
* Responds to changes in the Document's text, syncing the changes into our CodeMirror instance.
* There are several cases where we want to ignore a Document change:
* - if we're the master editor, Document changes should be ignored becuase we already have the right
* - if we're the master editor, Document changes should be ignored because we already have the right
* text (either the change originated with us, or it has already been set into us by Document)
* - if we're a secondary editor, Document changes should be ignored if they were caused by us sending
* the document an editor change that originated with us
Expand Down
8 changes: 4 additions & 4 deletions src/styles/brackets.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,10 +913,10 @@ a, img {
}
}

.navigator {
display: none;
button {
margin: 2px 1px 3px;
.navigator {
display: none;
button {
margin: 2px 1px 3px;
}
}
}
Expand Down
Loading