Looks interesting. I have not yet looked too deeply at all of the code, but are you trying to remove "YAHOO" competition ;) , you definitely gets some kudos for being able to read that spaghetti :D.
You bring up a good point about how this could be made extensible, which in the current format is simply not possible. Perhaps being able to register a custom handler for specific JavaScript cile events, like the "handleFunctionWithArguments" event could be a good start.
the problem I faced is to detect the line number for the dojo.declare/dojo.extend. I can't seem to figure it out, so in the patch I just use the the starting line number of the last argument (which is an object) to the function, which is not always correct. The following example will work correctly, as the starting line number of the {} is the same as the dojo.declare line
dojo.declare("A",null,{
});
while it is off by one line in the following sample:
dojo.declare("A",null,
{
});
not such a big deal, but I'd like to know whether it is possible to obtain the line number of dojo.declare in the handleFunctionWithArguments function
The line number is a problem I've also noticed, but has not been a high priority to fix. It is rather an architectural problem, lang_javascript.py only stores one variable for the current line, whereas the styles/text data is an array of the last x positions, and since could actually come from multiple lines, the line number likely should also be stored the same way...
That way we can review to make sure the patch is suitable and then apply it (obviously it would need some tweaks, such as working with the existing YAHOO handling :).
That is correct, these two issues have not yet been fixed. I am sorry, but with the 4.3.0 release rush we did not get the time to review the patches :(
I'll definitely be reviewing them now that 4.3.0 is out and this should get slotted for the 4.3.1 update.
I've added the test cases for dojo.declare and dojo.extend. It works rather well, though dojo.declare is failing on the following test though:
dojo.declare("Person_bug75069", null, {
initializer: function(name, age, currentResidence){ this.name=name; } }); var matt_bug75069 = new Person_bug75069(<1>'Matt', 25, 'New Mexico');
dojo.declare("Person_bug75069", null, {
initializer: function(name, age, currentResidence) {
this.name=name;
}
});
var matt_bug75069 = new Person_bug75069(<1>'Matt', 25, 'New Mexico');
When the cursor position is at "<1>" the calltip is expected to be:
"Person_bug75069(name, age, currentResidence)"
I'm guessing it's because the "initializer" constructor method is not getting correctly renamed to match the class name, and you'll also need to add the "__ctor__" value to the attributes for this function.
in dojo trunk (0.9+), initializer is deprecated and constructor should be used.
the patch I attached handles that as followings: if it finds a constructor function for a dojo.declare-d class, it remove it from function and change the function name to the class name, finally add the function back to the class. (search for comment "#change function constructor name..." in the patch)
that's the only way I can find for codeintel to use the constructor function as the "initializer" constructor method. I don't know whether there is a better way to achieve this
Yes, we want to get the JS argument problems fixed up. This patch will not make it into the 4.3.2 release though, it is still marked as a todo item. It will likely end up in the Komodo 4.4 release.
Looks interesting. I have not yet looked too deeply at all of the code, but are you trying to remove "YAHOO" competition ;) , you definitely gets some kudos for being able to read that spaghetti :D.
You bring up a good point about how this could be made extensible, which in the current format is simply not possible. Perhaps being able to register a custom handler for specific JavaScript cile events, like the "handleFunctionWithArguments" event could be a good start.
Cheers,
Todd
handleFunctionWithArguments sounds good to me too
the problem I faced is to detect the line number for the dojo.declare/dojo.extend. I can't seem to figure it out, so in the patch I just use the the starting line number of the last argument (which is an object) to the function, which is not always correct. The following example will work correctly, as the starting line number of the {} is the same as the dojo.declare line
dojo.declare("A",null,{ });while it is off by one line in the following sample:
dojo.declare("A",null, { });not such a big deal, but I'd like to know whether it is possible to obtain the line number of dojo.declare in the handleFunctionWithArguments function
The line number is a problem I've also noticed, but has not been a high priority to fix. It is rather an architectural problem, lang_javascript.py only stores one variable for the current line, whereas the styles/text data is an array of the last x positions, and since could actually come from multiple lines, the line number likely should also be stored the same way...
Oh yeah, I forgot to mention, if you'd like to see your patch make it into the main OpenKomodo tree, the best way is to submit a bug and patch here:
http://bugs.activestate.com/enter_bug.cgi?product=openkomodo
That way we can review to make sure the patch is suitable and then apply it (obviously it would need some tweaks, such as working with the existing YAHOO handling :).
Cheers,
Todd
submitted a cleaner patch as http://bugs.activestate.com/show_bug.cgi?id=75069
another bug I found is reported here: http://bugs.activestate.com/show_bug.cgi?id=75068 (also with a patch)
I don't use YAHOO, that's why I removed it from the file :)
komodo 4.3 is released, but it seems these 2 issues are not fixed in this release, right?
That is correct, these two issues have not yet been fixed. I am sorry, but with the 4.3.0 release rush we did not get the time to review the patches :(
I'll definitely be reviewing them now that 4.3.0 is out and this should get slotted for the 4.3.1 update.
Cheers,
Todd
seems this is not fixed in 4.3.1 either...
I've got to stop myself committing to things :D
I've added the test cases for dojo.declare and dojo.extend. It works rather well, though dojo.declare is failing on the following test though:
initializer: function(name, age, currentResidence) {
this.name=name;
}
});
var matt_bug75069 = new Person_bug75069(<1>'Matt', 25, 'New Mexico');
dojo.declare("Person_bug75069", null, { initializer: function(name, age, currentResidence) { this.name=name; } }); var matt_bug75069 = new Person_bug75069(<1>'Matt', 25, 'New Mexico');When the cursor position is at "<1>" the calltip is expected to be:
"Person_bug75069(name, age, currentResidence)"
I'm guessing it's because the "initializer" constructor method is not getting correctly renamed to match the class name, and you'll also need to add the "__ctor__" value to the attributes for this function.
Cheers,
Todd
in dojo trunk (0.9+), initializer is deprecated and constructor should be used.
the patch I attached handles that as followings: if it finds a constructor function for a dojo.declare-d class, it remove it from function and change the function name to the class name, finally add the function back to the class. (search for comment "#change function constructor name..." in the patch)
that's the only way I can find for codeintel to use the constructor function as the "initializer" constructor method. I don't know whether there is a better way to achieve this
After updating the testcase to use the "constructor" method, the tests all pass, nice work! I like it when things just work :D
Cheers,
Todd
please see the up-to-date doc for dojo here: (rather than the old jot wiki)
http://redesign.dojotoolkit.org/jsdoc/dojo/HEAD/dojo.declare
Todd committed the patch, and it will appear in 4.3.2:
http://svn.openkomodo.com/openkomodo/revision?rev=1236
Thanks again!
What about the other bug please?
while it may seem trivial, it is quite misleading while using komodo with dojo
Bug reference:
http://bugs.activestate.com/show_bug.cgi?id=75068
Yes, we want to get the JS argument problems fixed up. This patch will not make it into the 4.3.2 release though, it is still marked as a todo item. It will likely end up in the Komodo 4.4 release.
Cheers,
Todd
just uploaded a new patch which covers all the issues in the bug 75068, please review it