From bb3cea9ae9b6f2211b59976c591da3ab886bad9a Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Sun, 28 Feb 2016 21:22:16 -0500 Subject: [PATCH 01/14] Changes made suggested by PR --- src/editor/CodeHintList.js | 15 ++++--- src/editor/CodeHintManager.js | 79 ++++++++++------------------------- 2 files changed, 32 insertions(+), 62 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index 9c99e8cb67f..bde2ae3748f 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -308,15 +308,18 @@ define(function (require, exports, module) { /** * Check whether Event is one of the keys that we handle or not. * - * @param {KeyBoardEvent} keyEvent + * @param {KeyBoardEvent|keyBoardEvent.keyCode} keyEvent */ - CodeHintList.prototype.isHandlingEvent = function (event) { - var keyCode = event.keyCode; + CodeHintList.prototype.isHandlingKeyCode = function (keyCodeOrEvent) { + var keyCode = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.keyCode : keyCodeOrEvent; + var ctrlKey = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.ctrlKey : false; + + return (keyCode === KeyEvent.DOM_VK_UP || keyCode === KeyEvent.DOM_VK_DOWN || keyCode === KeyEvent.DOM_VK_PAGE_UP || keyCode === KeyEvent.DOM_VK_PAGE_DOWN || keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_CONTROL || - (event.ctrlKey === true && keyCode === KeyEvent.DOM_VK_SPACE) || + (ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE) || (keyCode === KeyEvent.DOM_VK_TAB && this.insertHintOnTab)); }; @@ -387,7 +390,7 @@ define(function (require, exports, module) { } // (page) up, (page) down, enter and tab key are handled by the list - if (event.type === "keydown" && this.isHandlingEvent(event)) { + if (event.type === "keydown" && this.isHandlingKeyCode(event)) { keyCode = event.keyCode; if (event.shiftKey && @@ -400,7 +403,7 @@ define(function (require, exports, module) { // Let the event bubble. return false; } else if (keyCode === KeyEvent.DOM_VK_UP || - (event.ctrlKey === true && keyCode === KeyEvent.DOM_VK_SPACE)) { + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, -1); } else if (keyCode === KeyEvent.DOM_VK_DOWN) { _rotateSelection.call(this, 1); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 2133ec844bf..0b56570d605 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -412,55 +412,16 @@ define(function (require, exports, module) { } return false; } - function _callMoveUp(event) { - if (deferredHints) { - deferredHints.reject(); - deferredHints = null; - } - - var response = sessionProvider.getHints(lastChar); - lastChar = null; - - if (!response) { - // the provider wishes to close the session - _endSession(); - } else { - // if the response is true, end the session and begin another - if (response === true) { - var previousEditor = sessionEditor; - //_endSession(); - //_beginSession(previousEditor); - } else if (response.hasOwnProperty("hints")) { // a synchronous response - if (hintList.isOpen()) { - // the session is open - hintList.callMoveUp(event); - } - } else { // response is a deferred - deferredHints = response; - response.done(function (hints) { - // Guard against timing issues where the session ends before the - // response gets a chance to execute the callback. If the session - // ends first while still waiting on the response, then hintList - // will get cleared up. - if (!hintList) { - return; - } - - if (hintList.isOpen()) { - // the session is open - hintList.callMoveUp(event); - } - }); - } - } - } /** * From an active hinting session, get hints from the current provider and * render the hint list window. * * Assumes that it is called when a session is active (i.e. sessionProvider is not null). */ - function _updateHintList() { + function _updateHintList(callMoveUpEvent) { + + var callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; + if (deferredHints) { deferredHints.reject(); deferredHints = null; @@ -476,12 +437,19 @@ define(function (require, exports, module) { // if the response is true, end the session and begin another if (response === true) { var previousEditor = sessionEditor; - _endSession(); - _beginSession(previousEditor); + + if (!callMoveUpEvent) { + _endSession(); + _beginSession(previousEditor); + } } else if (response.hasOwnProperty("hints")) { // a synchronous response if (hintList.isOpen()) { // the session is open - hintList.update(response); + if (callMoveUpEvent) { + hintList.callMoveUp(callMoveUpEvent); + } else { + hintList.update(response); + } } else { hintList.open(response); } @@ -498,7 +466,11 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - hintList.update(hints); + if (callMoveUpEvent) { + hintList.callMoveUp(callMoveUpEvent); + } else { + hintList.update(response); + } } else { hintList.open(hints); } @@ -579,10 +551,10 @@ define(function (require, exports, module) { if (_inSession(editor)) { _endSession(); } - // Begin a new explicit session - _beginSession(editor); + // Begin a new explicit session codeHintOpened = true; + _beginSession(editor); } } @@ -631,8 +603,8 @@ define(function (require, exports, module) { // We do this in "keyup" because we want the cursor position to be updated before // we redraw the list. _updateHintList(); - } else if (event.ctrlKey === true && event.keyCode === KeyEvent.DOM_VK_SPACE) { - _callMoveUp(event); + } else if (event.ctrlKey && event.keyCode === KeyEvent.DOM_VK_SPACE) { + _updateHintList(event); } } } @@ -747,11 +719,6 @@ define(function (require, exports, module) { 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 - // pointless to keep the hint list since the user wants to view the Quick Doc. - CommandManager.register(Strings.CMD_SHOW_CODE_HINTS, Commands.SHOW_CODE_HINTS, _startNewSession); exports._getCodeHintList = _getCodeHintList; From fa5d498a866e68182e68783062c792e31c2e03e9 Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Tue, 1 Mar 2016 23:44:34 -0500 Subject: [PATCH 02/14] Found a cleaner way to do the alreadyOpen Bool. Still having trouble with the javascript '.' character hints --- src/editor/CodeHintList.js | 6 +++--- src/editor/CodeHintManager.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index bde2ae3748f..2e4831fe27e 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -402,10 +402,10 @@ define(function (require, exports, module) { // Let the event bubble. return false; - } else if (keyCode === KeyEvent.DOM_VK_UP || - (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { + } else if (keyCode === KeyEvent.DOM_VK_UP) { _rotateSelection.call(this, -1); - } else if (keyCode === KeyEvent.DOM_VK_DOWN) { + } else if (keyCode === KeyEvent.DOM_VK_DOWN || + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, 1); } else if (keyCode === KeyEvent.DOM_VK_PAGE_UP) { _rotateSelection.call(this, -_itemsPerPage()); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 0b56570d605..74b8e1e05ef 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -484,6 +484,7 @@ define(function (require, exports, module) { * @param {Editor} editor */ _beginSession = function (editor) { + if (!codeHintsEnabled) { return; } @@ -538,14 +539,14 @@ define(function (require, exports, module) { * @param {Editor} editor */ function _startNewSession(editor) { - if (codeHintOpened) { + + if (isOpen()) { return; } if (!editor) { editor = EditorManager.getFocusedEditor(); } - if (editor) { lastChar = null; if (_inSession(editor)) { @@ -553,7 +554,6 @@ define(function (require, exports, module) { } // Begin a new explicit session - codeHintOpened = true; _beginSession(editor); } } From 5c1a8229e5528f89bb1a483061f03d0ff3f3003c Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 19:46:28 +0300 Subject: [PATCH 03/14] Remove unneeded var --- src/editor/CodeHintManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 74b8e1e05ef..8ebc2291da7 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -420,7 +420,7 @@ define(function (require, exports, module) { */ function _updateHintList(callMoveUpEvent) { - var callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; + callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; if (deferredHints) { deferredHints.reject(); From a7d3671318719e0e55c0fadd03effb8c9d22d5f4 Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 20:14:38 +0300 Subject: [PATCH 04/14] Fix issues with CodeHints starting with dots --- src/editor/CodeHintManager.js | 71 ++++++++++++++++------------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 8ebc2291da7..8d75c36aa3f 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -427,6 +427,10 @@ define(function (require, exports, module) { deferredHints = null; } + if (callMoveUpEvent) { + return hintList.callMoveUp(callMoveUpEvent); + } + var response = sessionProvider.getHints(lastChar); lastChar = null; @@ -438,18 +442,12 @@ define(function (require, exports, module) { if (response === true) { var previousEditor = sessionEditor; - if (!callMoveUpEvent) { - _endSession(); - _beginSession(previousEditor); - } + _endSession(); + _beginSession(previousEditor); } else if (response.hasOwnProperty("hints")) { // a synchronous response if (hintList.isOpen()) { // the session is open - if (callMoveUpEvent) { - hintList.callMoveUp(callMoveUpEvent); - } else { - hintList.update(response); - } + hintList.update(response); } else { hintList.open(response); } @@ -466,11 +464,7 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - if (callMoveUpEvent) { - hintList.callMoveUp(callMoveUpEvent); - } else { - hintList.update(response); - } + hintList.update(response); } else { hintList.open(hints); } @@ -533,31 +527,6 @@ define(function (require, exports, module) { } }; - /** - * Explicitly start a new session. If we have an existing session, - * then close the current one and restart a new one. - * @param {Editor} editor - */ - function _startNewSession(editor) { - - if (isOpen()) { - return; - } - - if (!editor) { - editor = EditorManager.getFocusedEditor(); - } - if (editor) { - lastChar = null; - if (_inSession(editor)) { - _endSession(); - } - - // Begin a new explicit session - _beginSession(editor); - } - } - /** * Handles keys related to displaying, searching, and navigating the hint list. * This gets called before handleChange. @@ -689,6 +658,30 @@ define(function (require, exports, module) { return (hintList && hintList.isOpen()); } + /** + * Explicitly start a new session. If we have an existing session, + * then close the current one and restart a new one. + * @param {Editor} editor + */ + function _startNewSession(editor) { + if (isOpen()) { + return; + } + + if (!editor) { + editor = EditorManager.getFocusedEditor(); + } + if (editor) { + lastChar = null; + if (_inSession(editor)) { + _endSession(); + } + + // Begin a new explicit session + _beginSession(editor); + } + } + /** * Expose CodeHintList for unit testing */ From 5e8edbacc1764e0877261820ae4d886ba6b3877b Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 20:16:50 +0300 Subject: [PATCH 05/14] Fix wrong parameter --- src/editor/CodeHintManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 8d75c36aa3f..e31e4b64fc3 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -464,7 +464,7 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - hintList.update(response); + hintList.update(hints); } else { hintList.open(hints); } From 40ddf0ac1bba6b7393535d22dff947a024864719 Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Sun, 28 Feb 2016 21:22:16 -0500 Subject: [PATCH 06/14] Changes made suggested by PR --- src/editor/CodeHintList.js | 15 ++++--- src/editor/CodeHintManager.js | 79 ++++++++++------------------------- 2 files changed, 32 insertions(+), 62 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index 4705bcdf120..b8216e706e3 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -309,15 +309,18 @@ define(function (require, exports, module) { /** * Check whether Event is one of the keys that we handle or not. * - * @param {KeyBoardEvent} keyEvent + * @param {KeyBoardEvent|keyBoardEvent.keyCode} keyEvent */ - CodeHintList.prototype.isHandlingEvent = function (event) { - var keyCode = event.keyCode; + CodeHintList.prototype.isHandlingKeyCode = function (keyCodeOrEvent) { + var keyCode = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.keyCode : keyCodeOrEvent; + var ctrlKey = typeof keyCodeOrEvent === "object" ? keyCodeOrEvent.ctrlKey : false; + + return (keyCode === KeyEvent.DOM_VK_UP || keyCode === KeyEvent.DOM_VK_DOWN || keyCode === KeyEvent.DOM_VK_PAGE_UP || keyCode === KeyEvent.DOM_VK_PAGE_DOWN || keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_CONTROL || - (event.ctrlKey === true && keyCode === KeyEvent.DOM_VK_SPACE) || + (ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE) || (keyCode === KeyEvent.DOM_VK_TAB && this.insertHintOnTab)); }; @@ -388,7 +391,7 @@ define(function (require, exports, module) { } // (page) up, (page) down, enter and tab key are handled by the list - if (event.type === "keydown" && this.isHandlingEvent(event)) { + if (event.type === "keydown" && this.isHandlingKeyCode(event)) { keyCode = event.keyCode; if (event.shiftKey && @@ -401,7 +404,7 @@ define(function (require, exports, module) { // Let the event bubble. return false; } else if (keyCode === KeyEvent.DOM_VK_UP || - (event.ctrlKey === true && keyCode === KeyEvent.DOM_VK_SPACE)) { + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, -1); } else if (keyCode === KeyEvent.DOM_VK_DOWN) { _rotateSelection.call(this, 1); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 2133ec844bf..0b56570d605 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -412,55 +412,16 @@ define(function (require, exports, module) { } return false; } - function _callMoveUp(event) { - if (deferredHints) { - deferredHints.reject(); - deferredHints = null; - } - - var response = sessionProvider.getHints(lastChar); - lastChar = null; - - if (!response) { - // the provider wishes to close the session - _endSession(); - } else { - // if the response is true, end the session and begin another - if (response === true) { - var previousEditor = sessionEditor; - //_endSession(); - //_beginSession(previousEditor); - } else if (response.hasOwnProperty("hints")) { // a synchronous response - if (hintList.isOpen()) { - // the session is open - hintList.callMoveUp(event); - } - } else { // response is a deferred - deferredHints = response; - response.done(function (hints) { - // Guard against timing issues where the session ends before the - // response gets a chance to execute the callback. If the session - // ends first while still waiting on the response, then hintList - // will get cleared up. - if (!hintList) { - return; - } - - if (hintList.isOpen()) { - // the session is open - hintList.callMoveUp(event); - } - }); - } - } - } /** * From an active hinting session, get hints from the current provider and * render the hint list window. * * Assumes that it is called when a session is active (i.e. sessionProvider is not null). */ - function _updateHintList() { + function _updateHintList(callMoveUpEvent) { + + var callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; + if (deferredHints) { deferredHints.reject(); deferredHints = null; @@ -476,12 +437,19 @@ define(function (require, exports, module) { // if the response is true, end the session and begin another if (response === true) { var previousEditor = sessionEditor; - _endSession(); - _beginSession(previousEditor); + + if (!callMoveUpEvent) { + _endSession(); + _beginSession(previousEditor); + } } else if (response.hasOwnProperty("hints")) { // a synchronous response if (hintList.isOpen()) { // the session is open - hintList.update(response); + if (callMoveUpEvent) { + hintList.callMoveUp(callMoveUpEvent); + } else { + hintList.update(response); + } } else { hintList.open(response); } @@ -498,7 +466,11 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - hintList.update(hints); + if (callMoveUpEvent) { + hintList.callMoveUp(callMoveUpEvent); + } else { + hintList.update(response); + } } else { hintList.open(hints); } @@ -579,10 +551,10 @@ define(function (require, exports, module) { if (_inSession(editor)) { _endSession(); } - // Begin a new explicit session - _beginSession(editor); + // Begin a new explicit session codeHintOpened = true; + _beginSession(editor); } } @@ -631,8 +603,8 @@ define(function (require, exports, module) { // We do this in "keyup" because we want the cursor position to be updated before // we redraw the list. _updateHintList(); - } else if (event.ctrlKey === true && event.keyCode === KeyEvent.DOM_VK_SPACE) { - _callMoveUp(event); + } else if (event.ctrlKey && event.keyCode === KeyEvent.DOM_VK_SPACE) { + _updateHintList(event); } } } @@ -747,11 +719,6 @@ define(function (require, exports, module) { 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 - // pointless to keep the hint list since the user wants to view the Quick Doc. - CommandManager.register(Strings.CMD_SHOW_CODE_HINTS, Commands.SHOW_CODE_HINTS, _startNewSession); exports._getCodeHintList = _getCodeHintList; From dd8f7b965f8643c99c489da65c98bcd4454f7fbc Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Tue, 1 Mar 2016 23:44:34 -0500 Subject: [PATCH 07/14] Found a cleaner way to do the alreadyOpen Bool. Still having trouble with the javascript '.' character hints --- src/editor/CodeHintList.js | 6 +++--- src/editor/CodeHintManager.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index b8216e706e3..24b8e9203b9 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -403,10 +403,10 @@ define(function (require, exports, module) { // Let the event bubble. return false; - } else if (keyCode === KeyEvent.DOM_VK_UP || - (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { + } else if (keyCode === KeyEvent.DOM_VK_UP) { _rotateSelection.call(this, -1); - } else if (keyCode === KeyEvent.DOM_VK_DOWN) { + } else if (keyCode === KeyEvent.DOM_VK_DOWN || + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, 1); } else if (keyCode === KeyEvent.DOM_VK_PAGE_UP) { _rotateSelection.call(this, -_itemsPerPage()); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 0b56570d605..74b8e1e05ef 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -484,6 +484,7 @@ define(function (require, exports, module) { * @param {Editor} editor */ _beginSession = function (editor) { + if (!codeHintsEnabled) { return; } @@ -538,14 +539,14 @@ define(function (require, exports, module) { * @param {Editor} editor */ function _startNewSession(editor) { - if (codeHintOpened) { + + if (isOpen()) { return; } if (!editor) { editor = EditorManager.getFocusedEditor(); } - if (editor) { lastChar = null; if (_inSession(editor)) { @@ -553,7 +554,6 @@ define(function (require, exports, module) { } // Begin a new explicit session - codeHintOpened = true; _beginSession(editor); } } From a890ee060d96d765effbc8273c916bcdce2ae39a Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 19:46:28 +0300 Subject: [PATCH 08/14] Remove unneeded var --- src/editor/CodeHintManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 74b8e1e05ef..8ebc2291da7 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -420,7 +420,7 @@ define(function (require, exports, module) { */ function _updateHintList(callMoveUpEvent) { - var callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; + callMoveUpEvent = typeof callMoveUpEvent === "undefined" ? false : callMoveUpEvent; if (deferredHints) { deferredHints.reject(); From 9c59b88ee75d3d43aa913d8986a724875fe97198 Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 20:14:38 +0300 Subject: [PATCH 09/14] Fix issues with CodeHints starting with dots --- src/editor/CodeHintManager.js | 71 ++++++++++++++++------------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 8ebc2291da7..8d75c36aa3f 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -427,6 +427,10 @@ define(function (require, exports, module) { deferredHints = null; } + if (callMoveUpEvent) { + return hintList.callMoveUp(callMoveUpEvent); + } + var response = sessionProvider.getHints(lastChar); lastChar = null; @@ -438,18 +442,12 @@ define(function (require, exports, module) { if (response === true) { var previousEditor = sessionEditor; - if (!callMoveUpEvent) { - _endSession(); - _beginSession(previousEditor); - } + _endSession(); + _beginSession(previousEditor); } else if (response.hasOwnProperty("hints")) { // a synchronous response if (hintList.isOpen()) { // the session is open - if (callMoveUpEvent) { - hintList.callMoveUp(callMoveUpEvent); - } else { - hintList.update(response); - } + hintList.update(response); } else { hintList.open(response); } @@ -466,11 +464,7 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - if (callMoveUpEvent) { - hintList.callMoveUp(callMoveUpEvent); - } else { - hintList.update(response); - } + hintList.update(response); } else { hintList.open(hints); } @@ -533,31 +527,6 @@ define(function (require, exports, module) { } }; - /** - * Explicitly start a new session. If we have an existing session, - * then close the current one and restart a new one. - * @param {Editor} editor - */ - function _startNewSession(editor) { - - if (isOpen()) { - return; - } - - if (!editor) { - editor = EditorManager.getFocusedEditor(); - } - if (editor) { - lastChar = null; - if (_inSession(editor)) { - _endSession(); - } - - // Begin a new explicit session - _beginSession(editor); - } - } - /** * Handles keys related to displaying, searching, and navigating the hint list. * This gets called before handleChange. @@ -689,6 +658,30 @@ define(function (require, exports, module) { return (hintList && hintList.isOpen()); } + /** + * Explicitly start a new session. If we have an existing session, + * then close the current one and restart a new one. + * @param {Editor} editor + */ + function _startNewSession(editor) { + if (isOpen()) { + return; + } + + if (!editor) { + editor = EditorManager.getFocusedEditor(); + } + if (editor) { + lastChar = null; + if (_inSession(editor)) { + _endSession(); + } + + // Begin a new explicit session + _beginSession(editor); + } + } + /** * Expose CodeHintList for unit testing */ From 8e9e119ef7e74020c2c79a6c8893f6b7ba5e3869 Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 20:16:50 +0300 Subject: [PATCH 10/14] Fix wrong parameter --- src/editor/CodeHintManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 8d75c36aa3f..e31e4b64fc3 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -464,7 +464,7 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - hintList.update(response); + hintList.update(hints); } else { hintList.open(hints); } From f4578093987ead413babcf5706eeacf9ad3b3836 Mon Sep 17 00:00:00 2001 From: Pete Nykanen Date: Fri, 1 Apr 2016 20:16:50 +0300 Subject: [PATCH 11/14] Fix wrong parameter --- src/editor/CodeHintManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index 8d75c36aa3f..e31e4b64fc3 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -464,7 +464,7 @@ define(function (require, exports, module) { if (hintList.isOpen()) { // the session is open - hintList.update(response); + hintList.update(hints); } else { hintList.open(hints); } From ab73306bda01fbf46eecff08906035b7e25b77db Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Mon, 27 Jun 2016 23:52:15 -0400 Subject: [PATCH 12/14] All tests pass. --- src/editor/CodeHintList.js | 8 +++++--- src/editor/CodeHintManager.js | 12 +++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index 24b8e9203b9..824c091a180 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -320,6 +320,7 @@ define(function (require, exports, module) { keyCode === KeyEvent.DOM_VK_PAGE_UP || keyCode === KeyEvent.DOM_VK_PAGE_DOWN || keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_CONTROL || + keyCode === KeyEvent.DOM_VK_ESCAPE || (ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE) || (keyCode === KeyEvent.DOM_VK_TAB && this.insertHintOnTab)); }; @@ -394,11 +395,12 @@ define(function (require, exports, module) { if (event.type === "keydown" && this.isHandlingKeyCode(event)) { keyCode = event.keyCode; - if (event.shiftKey && + if (event.keyCode === KeyEvent.DOM_VK_ESCAPE || + (event.shiftKey && (event.keyCode === KeyEvent.DOM_VK_UP || event.keyCode === KeyEvent.DOM_VK_DOWN || event.keyCode === KeyEvent.DOM_VK_PAGE_UP || - event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN)) { + event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN))) { this.handleClose(); // Let the event bubble. @@ -406,7 +408,7 @@ define(function (require, exports, module) { } else if (keyCode === KeyEvent.DOM_VK_UP) { _rotateSelection.call(this, -1); } else if (keyCode === KeyEvent.DOM_VK_DOWN || - (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, 1); } else if (keyCode === KeyEvent.DOM_VK_PAGE_UP) { _rotateSelection.call(this, -_itemsPerPage()); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index e31e4b64fc3..72edda134e3 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -563,7 +563,8 @@ define(function (require, exports, module) { function _handleKeyupEvent(jqEvent, editor, event) { keyDownEditor = editor; if (_inSession(editor)) { - if (event.keyCode === KeyEvent.DOM_VK_HOME || event.keyCode === KeyEvent.DOM_VK_END) { + if (event.keyCode === KeyEvent.DOM_VK_HOME || + event.keyCode === KeyEvent.DOM_VK_END) { _endSession(); } else if (event.keyCode === KeyEvent.DOM_VK_LEFT || event.keyCode === KeyEvent.DOM_VK_RIGHT || @@ -711,6 +712,15 @@ define(function (require, exports, module) { activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null); EditorManager.on("activeEditorChange", activeEditorChangeHandler); + // Dismiss code hints before executing any command other than showing code hints 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 + // pointless to keep the hint list since the user wants to view the Quick Doc + CommandManager.on("beforeExecuteCommand", function (event, commandId) { + if (commandId !== Commands.SHOW_CODE_HINTS) { + _endSession(); + } + }); CommandManager.register(Strings.CMD_SHOW_CODE_HINTS, Commands.SHOW_CODE_HINTS, _startNewSession); From 8ec63df5c2dd456d6cb6988565520e20daf89bec Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Mon, 27 Jun 2016 23:52:15 -0400 Subject: [PATCH 13/14] All tests pass. --- src/editor/CodeHintList.js | 8 +++++--- src/editor/CodeHintManager.js | 12 +++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/editor/CodeHintList.js b/src/editor/CodeHintList.js index 24b8e9203b9..824c091a180 100644 --- a/src/editor/CodeHintList.js +++ b/src/editor/CodeHintList.js @@ -320,6 +320,7 @@ define(function (require, exports, module) { keyCode === KeyEvent.DOM_VK_PAGE_UP || keyCode === KeyEvent.DOM_VK_PAGE_DOWN || keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_CONTROL || + keyCode === KeyEvent.DOM_VK_ESCAPE || (ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE) || (keyCode === KeyEvent.DOM_VK_TAB && this.insertHintOnTab)); }; @@ -394,11 +395,12 @@ define(function (require, exports, module) { if (event.type === "keydown" && this.isHandlingKeyCode(event)) { keyCode = event.keyCode; - if (event.shiftKey && + if (event.keyCode === KeyEvent.DOM_VK_ESCAPE || + (event.shiftKey && (event.keyCode === KeyEvent.DOM_VK_UP || event.keyCode === KeyEvent.DOM_VK_DOWN || event.keyCode === KeyEvent.DOM_VK_PAGE_UP || - event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN)) { + event.keyCode === KeyEvent.DOM_VK_PAGE_DOWN))) { this.handleClose(); // Let the event bubble. @@ -406,7 +408,7 @@ define(function (require, exports, module) { } else if (keyCode === KeyEvent.DOM_VK_UP) { _rotateSelection.call(this, -1); } else if (keyCode === KeyEvent.DOM_VK_DOWN || - (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { + (event.ctrlKey && keyCode === KeyEvent.DOM_VK_SPACE)) { _rotateSelection.call(this, 1); } else if (keyCode === KeyEvent.DOM_VK_PAGE_UP) { _rotateSelection.call(this, -_itemsPerPage()); diff --git a/src/editor/CodeHintManager.js b/src/editor/CodeHintManager.js index e31e4b64fc3..72edda134e3 100644 --- a/src/editor/CodeHintManager.js +++ b/src/editor/CodeHintManager.js @@ -563,7 +563,8 @@ define(function (require, exports, module) { function _handleKeyupEvent(jqEvent, editor, event) { keyDownEditor = editor; if (_inSession(editor)) { - if (event.keyCode === KeyEvent.DOM_VK_HOME || event.keyCode === KeyEvent.DOM_VK_END) { + if (event.keyCode === KeyEvent.DOM_VK_HOME || + event.keyCode === KeyEvent.DOM_VK_END) { _endSession(); } else if (event.keyCode === KeyEvent.DOM_VK_LEFT || event.keyCode === KeyEvent.DOM_VK_RIGHT || @@ -711,6 +712,15 @@ define(function (require, exports, module) { activeEditorChangeHandler(null, EditorManager.getActiveEditor(), null); EditorManager.on("activeEditorChange", activeEditorChangeHandler); + // Dismiss code hints before executing any command other than showing code hints 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 + // pointless to keep the hint list since the user wants to view the Quick Doc + CommandManager.on("beforeExecuteCommand", function (event, commandId) { + if (commandId !== Commands.SHOW_CODE_HINTS) { + _endSession(); + } + }); CommandManager.register(Strings.CMD_SHOW_CODE_HINTS, Commands.SHOW_CODE_HINTS, _startNewSession); From 752770f7289ba81219e3b9543ef816e307e9d45b Mon Sep 17 00:00:00 2001 From: Brandon Max Date: Mon, 27 Jun 2016 23:55:07 -0400 Subject: [PATCH 14/14] Added new tests to support this fix. --- test/spec/CodeHint-test.js | 82 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/spec/CodeHint-test.js b/test/spec/CodeHint-test.js index 0d1c4f24ae1..b78297a36d3 100644 --- a/test/spec/CodeHint-test.js +++ b/test/spec/CodeHint-test.js @@ -279,6 +279,88 @@ define(function (require, exports, module) { }); }); + it("should go to next hint with ctrl+space", function () { + var editor, + pos = {line: 3, ch: 1}, + hintBefore, + hintAfter; + + // minimal markup with an open '<' before IP + // Note: line for pos is 0-based and editor lines numbers are 1-based + initCodeHintTest("test1.html", pos); + + // simulate ctrl+space key to make sure it goes to next hint + runs(function () { + var e = $.Event("keydown"); + e.keyCode = KeyEvent.DOM_VK_SPACE; + e.ctrlKey = true; + + editor = EditorManager.getCurrentFullEditor(); + expect(editor).toBeTruthy(); + + invokeCodeHints(); + var codeHintList = expectSomeHints(); + hintBefore = codeHintList.selectedIndex; + + // make sure hint list starts at 0 + expect(hintBefore).toEqual(0); + + // simulate ctrl+space keyhook + CodeHintManager._getCodeHintList()._keydownHook(e); + hintAfter = codeHintList.selectedIndex; + + // selectedIndex should be one more after doing ctrl+space key event. + expect(hintBefore).toEqual(hintAfter-1); + + editor = null; + }); + }); + + it("should loop to first hint when ctrl+space at last hint", function () { + var editor, + pos = {line: 3, ch: 1}, + hintBefore, + hintAfter; + + // minimal markup with an open '<' before IP + // Note: line for pos is 0-based and editor lines numbers are 1-based + initCodeHintTest("test1.html", pos); + + // simulate ctrl+space key to make sure it goes to next hint + runs(function () { + var e = $.Event("keydown"); + e.keyCode = KeyEvent.DOM_VK_UP; + + editor = EditorManager.getCurrentFullEditor(); + expect(editor).toBeTruthy(); + + invokeCodeHints(); + + // simulate up keyhook to send it to last hint + CodeHintManager._getCodeHintList()._keydownHook(e); + + var codeHintList = expectSomeHints(); + hintBefore = codeHintList.selectedIndex; + var numberOfHints = codeHintList.$hintMenu.find("li").length-1; + + // should be at last hint + expect(hintBefore).toEqual(numberOfHints); + + // call ctrl+space to loop it to first hint + e.keyCode = KeyEvent.DOM_VK_SPACE; + e.ctrlKey = true; + + // simulate ctrl+space keyhook to send it to first hint + CodeHintManager._getCodeHintList()._keydownHook(e); + hintAfter = codeHintList.selectedIndex; + + // should now be at hint 0 + expect(hintAfter).toEqual(0); + + editor = null; + }); + }); + it("should not show code hints if there is a multiple selection", function () { // minimal markup with an open '<' before IP // Note: line for pos is 0-based and editor lines numbers are 1-based