Created attachment 11317 [details]
This patch adds the function popup functionality in Drosera. Once we have fixed the parsing functions to get keywords properly, this will work a bit better and potentially faster as many "if" statements can be removed . Also, we'll need to handle special cases such as var foo = function() or foo: function() so that the popup is a bit more useful in those cases. Right now, the popup only gives "function()" in those instances.
Created attachment 11318 [details]
Screenshot with file popup active
Created attachment 11319 [details]
Screenshot with file popup closed and selected function highlighted
Comment on attachment 11317 [details]
This is looking good. There are a few things that need addressed.
Many elese statments do not meet our style guidlines.
Place the else on the same line as the end brace.
+ } else
+ result += "<span class=\"keyword\">" + "<a name=\"function-"
These first two strings can be combined into one, eliminating a concatination.
+ var functionNames = new Array();
Why have a local functionNames when you just use file. functionNames?
+ var counter = 8;
Rename that to functionKeywordOffset or something meaningful. The current name is confusing.
Please use "<No selected symbol>", like Xcode. Note the missing spaces and the capital No.
removeChildrenFromElement should be a prototype function on Element and just called removeChildren. There are a couple of functions we add to element for adding and removing class names.
I am not big fan of the red highlight around the function name, lets just remove that. Xcode dosen't do that.
Xcode selects the function name when you pick one. We should do the same and not the red highlight.
Here is how in WebKit:
var selection = window.getSelection();
var element = document.getElementById("test");
selection.setBaseAndExtent(element, 0, element, 1);
Created attachment 11367 [details]
- corrected else statements
- removed unnecessary variable
- removed unnecessary concatenation
- renamed var counter to functionKeywordOffset
- prototyped Element.removeChildren
- renamed placeholder text to <No selected symbol>
- replaced function name highlight with selection
Comment on attachment 11367 [details]
Great job. Landed in r17583.
Closing since Drosera has been replaced by the new Web Inspector debugger. Moving to the New Bugs component so the Drosera component can be closed and removed.