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
RaymondLim 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 2 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
83 changes: 47 additions & 36 deletions src/editor/CodeHintManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* 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
Expand All @@ -14,9 +14,9 @@
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* 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 @@ -98,8 +98,8 @@
* 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
Expand All @@ -111,8 +111,8 @@
* 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
Expand Down Expand Up @@ -185,11 +185,11 @@
* 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
* choose to let the manager handle emphasis for the simple and common case
Expand All @@ -211,16 +211,16 @@
*
* 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,7 +275,7 @@ 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.
*
Expand Down Expand Up @@ -384,12 +384,12 @@ 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
*/
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,7 +517,7 @@ 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
Expand All @@ -528,7 +528,7 @@ define(function (require, exports, module) {
* @param {Editor} editor
* @param {KeyboardEvent} event
*/
function handleKeyEvent(editor, event) {
function _handleKeyEvent(jqEvent, editor, event) {
Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Oct 12, 2013

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 @@ -564,15 +564,15 @@ define(function (require, exports, module) {
* Called by the editor after handleKeyEvent, which is responsible for setting
* the lastChar.
*/
function handleChange(editor, changeList) {
function _handleChange(event, editor, changeList) {
Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Oct 12, 2013

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 +618,22 @@ define(function (require, exports, module) {
return hintList;
}

function activeEditorChangeHandler(event, current, previous) {
$(current).on("editorChange", _handleChange);
Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Oct 12, 2013

Choose a reason for hiding this comment

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

You need to check null on current before executing these two lines.

$(current).on("keyEvent", _handleKeyEvent);

//Removing all old Handlers
$(previous).off("editorChange", _handleChange);
Copy link
Copy Markdown
Contributor

@RaymondLim RaymondLim Oct 12, 2013

Choose a reason for hiding this comment

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

Same as above. Please wrap these two lines with if (previous) {}.

$(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 +644,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
63 changes: 32 additions & 31 deletions src/editor/Editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
/*
* 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 All @@ -38,7 +38,7 @@
*
* For now, direct access to the underlying CodeMirror object is still possible via _codeMirror --
* but this is considered deprecated and may go away.
*
*
* The Editor object dispatches the following events:
* - keyEvent -- When any key event happens in the editor (whether it changes the text or not).
* Event handlers are passed ({Editor}, {KeyboardEvent}). The 2nd arg is the raw DOM event.
Expand All @@ -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 @@ -120,13 +119,13 @@ define(function (require, exports, module) {
*/
function _handleTabKey(instance) {
// Tab key handling is done as follows:
// 1. If the selection is before any text and the indentation is to the left of
// the proper indentation then indent it to the proper place. Otherwise,
// add another tab. In either case, move the insertion point to the
// beginning of the text.
// 1. If the selection is before any text and the indentation is to the left of
// the proper indentation then indent it to the proper place. Otherwise,
// add another tab. In either case, move the insertion point to the
// beginning of the text.
// 2. If the selection is multi-line, indent all the lines.
// 3. If the selection is after the first non-space character, and is an
// insertion point, insert a tab character or the appropriate number
// 3. If the selection is after the first non-space character, and is an
// insertion point, insert a tab character or the appropriate number
// of spaces to pad to the nearest tab boundary.
var from = instance.getCursor(true),
to = instance.getCursor(false),
Expand Down Expand Up @@ -271,9 +270,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 All @@ -295,7 +291,7 @@ define(function (require, exports, module) {
var _instances = [];


/**
/**
* @constructor
*
* Creates a new CodeMirror editor instance bound to the given Document. The Document need not have
Expand All @@ -305,7 +301,7 @@ define(function (require, exports, module) {
*
* ALWAYS call destroy() when you are done with an Editor - otherwise it will leak a Document ref.
*
* @param {!Document} document
* @param {!Document} document
* @param {!boolean} makeMasterEditor If true, this Editor will set itself as the (secret) "master"
* Editor for the Document. If false, this Editor will attach to the Document as a "slave"/
* secondary editor.
Expand Down Expand Up @@ -492,7 +488,7 @@ define(function (require, exports, module) {
};


/**
/**
* Selects all text and maintains the current scroll position.
*/
Editor.prototype.selectAllNoScroll = function () {
Expand Down Expand Up @@ -599,7 +595,12 @@ 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 listend 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]);
};

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

/**
* Sets the height of an inline widget in this editor.
* Sets the height of an inline widget in this editor.
* @param {!InlineWidget} inlineWidget The widget whose height should be set.
* @param {!number} height The height of the widget.
* @param {boolean} ensureVisible Whether to scroll the entire widget into view.
Expand Down Expand Up @@ -1304,7 +1305,7 @@ define(function (require, exports, module) {
/**
* @private
* Get the starting line number for an inline widget.
* @param {!InlineWidget} inlineWidget
* @param {!InlineWidget} inlineWidget
* @return {number} The line number of the widget or -1 if not found.
*/
Editor.prototype._getInlineWidgetLineNumber = function (inlineWidget) {
Expand Down Expand Up @@ -1341,7 +1342,7 @@ define(function (require, exports, module) {
*/
Editor.prototype.refresh = function (handleResize) {
// If focus is currently in a child of the CodeMirror editor (e.g. in an inline widget), but not in
// the CodeMirror input field itself, remember the focused item so we can restore focus after the
// the CodeMirror input field itself, remember the focused item so we can restore focus after the
// refresh (which might cause the widget to be removed from the display list temporarily).
var focusedItem = window.document.activeElement,
restoreFocus = $.contains(this._codeMirror.getScrollerElement(), focusedItem);
Expand Down Expand Up @@ -1407,7 +1408,7 @@ define(function (require, exports, module) {
* A-B-A would return A as the mode, not null).
*
* @return {?(Object|string)} Name of syntax-highlighting mode, or object containing a "name" property
* naming the mode along with configuration options required by the mode.
* naming the mode along with configuration options required by the mode.
* See {@link LanguageManager#getLanguageForPath()} and {@link Language#getMode()}.
*/
Editor.prototype.getModeForSelection = function () {
Expand Down
Loading
⚡