Fix for Issue 1617: Removed all references to CodeHintManager from Editor #5421
Conversation
|
Credits to @jbalsas for finding the fix i incorporated into this pull |
|
@WebsiteDeveloper |
|
@RaymondLim i reverted some of the whitespace changes but decided to stop because i would have to revert them each manually. |
|
Just a heads up, you can view the file changes and ignore any whitespace changes by adding ?w=1 to the end of the url, e.g. https://github.com/adobe/brackets/pull/5421/files?w=1 |
|
Good tip, @TuckerWhitehouse I have verified that #5412 is not re-introduced by this change. |
|
Thanks for checking. @RaymondLim time to review? |
ghost
assigned 
| @@ -528,7 +528,7 @@ define(function (require, exports, module) { | |||
| * @param {Editor} editor | |||
| * @param {KeyboardEvent} event | |||
| */ | |||
| function handleKeyEvent(editor, event) { | |||
| function _handleKeyEvent(jqEvent, editor, event) { | |||
There was a problem hiding this comment.
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.
|
@WebsiteDeveloper Done with initial review. I think you need to merge with master to resolve the conflict. |
|
@RaymondLim merged with master and fixed nits |
| * | ||
| * @param {Event} event | ||
| * @param {Editor} editor | ||
| * @param {{from: Pos, to: Pos, text: Array, origin: String}} changeList |
There was a problem hiding this comment.
One more minor nit: String -> string.
|
@RaymondLim fixed. |
|
Looks good. Merging. |
Fix for Issue 1617: Removed all references to CodeHintManager from Editor
@dangoor to verify that #5412 wasn't introduced.
@RaymondLim since you were the original reviewer