Fix bug #1109 (Quick Open JS: can't access second function with same name) #1710
Conversation
…name) and equivalent bug for CSS & HTML Quick Open. Simplifies logic as well. Plus two tiny docs fixes.
ghost
assigned 
| } | ||
| var selectorInfo = selectedItem.selectorInfo; | ||
|
|
||
| var from = {line: selectorInfo.selectorStartLine, ch: selectorInfo.selectorStartChar}; |
There was a problem hiding this comment.
The old code checked for "if (selectorInfo)" before dereferencing it. Are you really sure that it will always be defined?
There was a problem hiding this comment.
@redmunds:
I think we can be sure: in search(), selectorInfo is set on every searchResult that's returned. The value it's set to (itemInfo) is guaranteed to be non-null by line 75 where it's dereferenced (although also it should be safe to assume that the input list before filtering doesn't contain null entries). Same is true in the HTML & JS versions of this code.
In the old code that was using the slightly fragile getLocationFrom*() utils to map back from the DOM node to a search result item, it sort of made sense to be worried that something could go wrong and give you null. Now I think we can safely assume it's always non-null.
|
Done with initial review. |
|
Thanks for the 'bonus' code review btw! |
Fix bug #1109 (Quick Open JS: can't access second function with same name)
Fix bug #1109 (Quick Open JS: can't access second function with same name), and equivalent bug for CSS & HTML Quick Open. Simplifies logic at the same time, taking advantage of #1565.
Plus two tiny docs fixes.