New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imenu problems #27

Closed
mbuczko opened this Issue Dec 14, 2011 · 30 comments

Comments

Projects
None yet
3 participants
@mbuczko

mbuczko commented Dec 14, 2011

hi,

looks like there is something wrong with how imenu shows available functions.
following example works perfectly:

var foo = function(options) {
};

and imenu shows "foo" on the list. but let's modify it a bit:

var foo = function(options) {
    var bar = function() {
    }
};

and we get "" and "bar" on the list. There is a second problem as well:

(function($) {
    var foo = function(options) {
        var bar = function() {
        };
    };
})(jQuery);

imenu shows only "foo".

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 14, 2011

Collaborator

The bit with <definition-1> is kind of expected behavior. What do you expect to see there?

bar not being shown in the index in the last example is caused by the limitation in the amount of function nesting the index function currently handles. Do you usually have this (or higher) level of nesting?

Collaborator

dgutov commented Dec 14, 2011

The bit with <definition-1> is kind of expected behavior. What do you expect to see there?

bar not being shown in the index in the last example is caused by the limitation in the amount of function nesting the index function currently handles. Do you usually have this (or higher) level of nesting?

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars Dec 14, 2011

Seems to me like the nesting is a bit too shallow. At least, extending an object inside an immediately invoked function expression is enough for it to miss them. Like this:

(function () {

    var child = $.extend(parent, {
        func: function () {}
    });

}());

Here func isn't in the index. Move it out of the IIFE or the extend and it shows up.

This is a very common pattern in my code at least.

magnars commented Dec 14, 2011

Seems to me like the nesting is a bit too shallow. At least, extending an object inside an immediately invoked function expression is enough for it to miss them. Like this:

(function () {

    var child = $.extend(parent, {
        func: function () {}
    });

}());

Here func isn't in the index. Move it out of the IIFE or the extend and it shows up.

This is a very common pattern in my code at least.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 14, 2011

Well, regarding I would rather expect "foo" which is more meaningful.

I definitely agree that nesting is too shallow. one more level would be really appreciated :)
Example above is common pattern used eg. by jQuery plugins and lack of deeper nesting is a bit harmful here.

mbuczko commented Dec 14, 2011

Well, regarding I would rather expect "foo" which is more meaningful.

I definitely agree that nesting is too shallow. one more level would be really appreciated :)
Example above is common pattern used eg. by jQuery plugins and lack of deeper nesting is a bit harmful here.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 14, 2011

Collaborator

To recognize the example above, it would require not only increase the level of nesting the index function can handle, but also some new jQuery-specific logic, so it would be able to determine that the extension object belongs inside the child "namespace".

Collaborator

dgutov commented Dec 14, 2011

To recognize the example above, it would require not only increase the level of nesting the index function can handle, but also some new jQuery-specific logic, so it would be able to determine that the extension object belongs inside the child "namespace".

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars Dec 14, 2011

Ouch, that's too bad. jQuery-specific logic definitely does not belong in js2-mode. Also, I am not actually using jQuery to extend my objects. You could for instance use this from ES5:

var child = Object.create(parent, {
    func: { value: function () {} }
});

Or you could use underscore.js _.extend (same API as jQuery).

Too bad that it is such an undertaking to get those rather common examples to work.

magnars commented Dec 14, 2011

Ouch, that's too bad. jQuery-specific logic definitely does not belong in js2-mode. Also, I am not actually using jQuery to extend my objects. You could for instance use this from ES5:

var child = Object.create(parent, {
    func: { value: function () {} }
});

Or you could use underscore.js _.extend (same API as jQuery).

Too bad that it is such an undertaking to get those rather common examples to work.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 14, 2011

It would be nice to have kind of namespace information, but honestly, having only function detected (without the context info) would be fair enough for me. function (or variable) name is what most of javascript-aware editors show (textmate or sublime text)

mbuczko commented Dec 14, 2011

It would be nice to have kind of namespace information, but honestly, having only function detected (without the context info) would be fair enough for me. function (or variable) name is what most of javascript-aware editors show (textmate or sublime text)

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 14, 2011

I also don't really like the idea of recognizing framework-specific constructions in js2-mode. Detected variable/function name only would make me happy.

I wouldn't even consider clashing function names which eventually could be presented with namespace info.

mbuczko commented Dec 14, 2011

I also don't really like the idea of recognizing framework-specific constructions in js2-mode. Detected variable/function name only would make me happy.

I wouldn't even consider clashing function names which eventually could be presented with namespace info.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 14, 2011

Collaborator

I think a framework-specific imenu extension is a useful thing to do. Not in js2-mode proper, maybe, but a separate package dependent on it should be fine.

Support for Object.create will also probably go in there, since it's going to share most of the implementation.

function (or variable) name is what most of javascript-aware editors show (textmate or sublime text)

Personally, I consider this an advantage of js2-mode. For all we know, other users do, too.

Anyway, I'll look into the nesting limit.

Collaborator

dgutov commented Dec 14, 2011

I think a framework-specific imenu extension is a useful thing to do. Not in js2-mode proper, maybe, but a separate package dependent on it should be fine.

Support for Object.create will also probably go in there, since it's going to share most of the implementation.

function (or variable) name is what most of javascript-aware editors show (textmate or sublime text)

Personally, I consider this an advantage of js2-mode. For all we know, other users do, too.

Anyway, I'll look into the nesting limit.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 25, 2011

Collaborator

This should take care of it. Please try it with your code and report any problems, I'll then push it to the main repo.

Regarding <definition-1>, what are you using to access the imenu index? Personally, I'm using anything-imenu, and it shows neat namespaced entries, like in the example.

I've noticed a problem with the built-in imenu command in the trunk build of Emacs 24 I'm using, but this doesn't look like a problem with js2-mode, the generated data looks like it should.

Collaborator

dgutov commented Dec 25, 2011

This should take care of it. Please try it with your code and report any problems, I'll then push it to the main repo.

Regarding <definition-1>, what are you using to access the imenu index? Personally, I'm using anything-imenu, and it shows neat namespaced entries, like in the example.

I've noticed a problem with the built-in imenu command in the trunk build of Emacs 24 I'm using, but this doesn't look like a problem with js2-mode, the generated data looks like it should.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 26, 2011

I did a few tests on my sources and your fix works really nice. thanks!

Regarding <definition-1> I'm using following Emacs build:

GNU Emacs 24.0.90.1 (x86_64-apple-darwin, NS apple-appkit-1038.35) of 2011-09-26 on virtualmac.porkrind.org

so it might be affected by problem you mentioned. <definition-1> appears with anything-imenu and ido-ed imenu which i use (http://www.emacswiki.org/emacs/ImenuMode#toc10).

anyway, your latest patch makes js2-menu close to perfection :)

One more thing you could consider is a bit modified version of bar function from my first post:

(function($) {
    return {
        bar: function() { }
    };
})(jQuery);

which is still not recognized. so far, I was doing a simple workarounds like:

(function($) {
    var bazz = {
        bar: function() { }
    }
    return bazz;
})(jQuery);

which make bar visible, but it would be nice to get rid of these additional assignments.

mbuczko commented Dec 26, 2011

I did a few tests on my sources and your fix works really nice. thanks!

Regarding <definition-1> I'm using following Emacs build:

GNU Emacs 24.0.90.1 (x86_64-apple-darwin, NS apple-appkit-1038.35) of 2011-09-26 on virtualmac.porkrind.org

so it might be affected by problem you mentioned. <definition-1> appears with anything-imenu and ido-ed imenu which i use (http://www.emacswiki.org/emacs/ImenuMode#toc10).

anyway, your latest patch makes js2-menu close to perfection :)

One more thing you could consider is a bit modified version of bar function from my first post:

(function($) {
    return {
        bar: function() { }
    };
})(jQuery);

which is still not recognized. so far, I was doing a simple workarounds like:

(function($) {
    var bazz = {
        bar: function() { }
    }
    return bazz;
})(jQuery);

which make bar visible, but it would be nice to get rid of these additional assignments.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 27, 2011

Collaborator

What I meat to say, seeing <definition-n> entries is normal. They appear in two cases:

  1. The identifier is defined or assigned to several times in the file.
  2. The identifier has "children". Basically, each node can be either "branch" or a "leaf". If "foo" has other functions inside, we need a separate leaf to denote the "foo" itself, and it can't be a valid identifier, so it would never conflict with existing functions.
    jQuery imenu output linked to in the previous message has examples of both kinds.

ido-goto-symbol flattens the imenu data and throws out the namespaces, that's why you're seeing these entries without context. I'm not sure what's the best way to deal with this from my side, but I'm guessing you could modify the flattening logic to replace any <definition-*> entries to include the parent node's name.

The problem with imenu in my current build is of another kind: using imenu command doesn't bring me to function definitions like it should, in certain cases.

Regarding the example, could you expand it a little? As it is now, there's no way to call "bar", since the returned value is not assigned to anything.
I'd also like to know the reason behind using this pattern. (EDIT) Can you point to it used in some open source code?
jQuery guidelines require using this kind of wrapper for publicly available plugins, but in this case you usually have an assignment to $.fn.something inside, and the wrapper spans the entire source file.
Or, if I'm creating a global value, there would be assignment to "window" or "global", passed in as an argument, inside the anonymous function's body.

Collaborator

dgutov commented Dec 27, 2011

What I meat to say, seeing <definition-n> entries is normal. They appear in two cases:

  1. The identifier is defined or assigned to several times in the file.
  2. The identifier has "children". Basically, each node can be either "branch" or a "leaf". If "foo" has other functions inside, we need a separate leaf to denote the "foo" itself, and it can't be a valid identifier, so it would never conflict with existing functions.
    jQuery imenu output linked to in the previous message has examples of both kinds.

ido-goto-symbol flattens the imenu data and throws out the namespaces, that's why you're seeing these entries without context. I'm not sure what's the best way to deal with this from my side, but I'm guessing you could modify the flattening logic to replace any <definition-*> entries to include the parent node's name.

The problem with imenu in my current build is of another kind: using imenu command doesn't bring me to function definitions like it should, in certain cases.

Regarding the example, could you expand it a little? As it is now, there's no way to call "bar", since the returned value is not assigned to anything.
I'd also like to know the reason behind using this pattern. (EDIT) Can you point to it used in some open source code?
jQuery guidelines require using this kind of wrapper for publicly available plugins, but in this case you usually have an assignment to $.fn.something inside, and the wrapper spans the entire source file.
Or, if I'm creating a global value, there would be assignment to "window" or "global", passed in as an argument, inside the anonymous function's body.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 27, 2011

thanks for clarification. indeed, <definition-n> notation makes sense when it's displayed together with namespace (eg. in the way like anything-imenu presents it), although personally I would rather prefer namespace "name" only to denote that particular node.
this is what anything-imenu shows now:

foo / bar
foo / <definition-1>

and this is how i would imagine it:

foo
foo / bar

that's much more concise, as technically foo itself is a function in a code, and foo / bar denotes foo as a namespace for bar. and <definition-n> introduces here unnecessary noise :)

regarding my previous example. it's not the most representative one, but it's something usually called a "module pattern" described eg here: http://javascriptweblog.wordpress.com/2010/12/07/namespacing-in-javascript/ and here: http://dustindiaz.com/namespace-your-javascript/. in my daily work it's very often used (sometimes overused) in projects containing more than one file, as it easily shields the logic from global scope. take a look at first link, which describes this pattern in details.

mbuczko commented Dec 27, 2011

thanks for clarification. indeed, <definition-n> notation makes sense when it's displayed together with namespace (eg. in the way like anything-imenu presents it), although personally I would rather prefer namespace "name" only to denote that particular node.
this is what anything-imenu shows now:

foo / bar
foo / <definition-1>

and this is how i would imagine it:

foo
foo / bar

that's much more concise, as technically foo itself is a function in a code, and foo / bar denotes foo as a namespace for bar. and <definition-n> introduces here unnecessary noise :)

regarding my previous example. it's not the most representative one, but it's something usually called a "module pattern" described eg here: http://javascriptweblog.wordpress.com/2010/12/07/namespacing-in-javascript/ and here: http://dustindiaz.com/namespace-your-javascript/. in my daily work it's very often used (sometimes overused) in projects containing more than one file, as it easily shields the logic from global scope. take a look at first link, which describes this pattern in details.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 27, 2011

Collaborator

that's much more concise

Sure, but imenu uses data arranged in a tree, and I'm not even sure that nodes with the same name are allowed on the same level. Suppose they are, and the user invokes the standard imenu command. It will show two foo entries, without any distinction between them. That's bad.
I'm thinking it could be better to show something like foo<1>, on the same level as foo, instead of foo / <definition-1>, but this representation is definitely less self-explanatory.

something usually called a "module pattern" described eg here

It shouldn't be too hard to add a special case for this construct ("return" node is the last in the function body, result of the invocation is assigned to a var). So the example on the second linked page would turn into:

DED / method_1
DED / method_2

Supporting the variation where the hash references named functions defined within the scope (example) looks more complicated, though.

Collaborator

dgutov commented Dec 27, 2011

that's much more concise

Sure, but imenu uses data arranged in a tree, and I'm not even sure that nodes with the same name are allowed on the same level. Suppose they are, and the user invokes the standard imenu command. It will show two foo entries, without any distinction between them. That's bad.
I'm thinking it could be better to show something like foo<1>, on the same level as foo, instead of foo / <definition-1>, but this representation is definitely less self-explanatory.

something usually called a "module pattern" described eg here

It shouldn't be too hard to add a special case for this construct ("return" node is the last in the function body, result of the invocation is assigned to a var). So the example on the second linked page would turn into:

DED / method_1
DED / method_2

Supporting the variation where the hash references named functions defined within the scope (example) looks more complicated, though.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 27, 2011

one problem I just noticed may appear when you'll try to parse something like this:

var DED = function() {
    var method_1  = function() { ... };

    return {
        method_1 : function() {
        },
        method_2 : function() {
        }
    };
}();

Now, how to distinct between variable method_1 and the hashed function method_1? Maybe something like this:

 DED / method_1
 DED / <hash-1> / method_1
 DED / <hash-1> / method_2

mbuczko commented Dec 27, 2011

one problem I just noticed may appear when you'll try to parse something like this:

var DED = function() {
    var method_1  = function() { ... };

    return {
        method_1 : function() {
        },
        method_2 : function() {
        }
    };
}();

Now, how to distinct between variable method_1 and the hashed function method_1? Maybe something like this:

 DED / method_1
 DED / <hash-1> / method_1
 DED / <hash-1> / method_2
@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 27, 2011

Collaborator

Maybe specially mark "private" functions instead?

DED / () / method_1
DED / method_1
DED / method_2

Only we'd need to do that everywhere, for the sake of consistency. To make this less annoying, it would need "escape analysis" of some sort, to discern functions that do end up accessible from outside, at least for most popular constructs.

Collaborator

dgutov commented Dec 27, 2011

Maybe specially mark "private" functions instead?

DED / () / method_1
DED / method_1
DED / method_2

Only we'd need to do that everywhere, for the sake of consistency. To make this less annoying, it would need "escape analysis" of some sort, to discern functions that do end up accessible from outside, at least for most popular constructs.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 27, 2011

Agree, although I would rather opt for '#' which is more associated with hashes and 'private' (or rather 'protected') stuff. and makes less clutter :)

DED / # / method1
DED / method_1
DED / method_2

mbuczko commented Dec 27, 2011

Agree, although I would rather opt for '#' which is more associated with hashes and 'private' (or rather 'protected') stuff. and makes less clutter :)

DED / # / method1
DED / method_1
DED / method_2
@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 28, 2011

Collaborator

"#"' is used in JavaDoc and JsDoc as delimiter between class name and method name, independent of the method's visibility.
Doesn't seem right. Something else, maybe?

Collaborator

dgutov commented Dec 28, 2011

"#"' is used in JavaDoc and JsDoc as delimiter between class name and method name, independent of the method's visibility.
Doesn't seem right. Something else, maybe?

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko Dec 28, 2011

is it possible to make it configurable?

mbuczko commented Dec 28, 2011

is it possible to make it configurable?

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Dec 28, 2011

Collaborator

Sure.

Collaborator

dgutov commented Dec 28, 2011

Sure.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov May 13, 2012

Collaborator

So, uh, I have some progress to report: https://github.com/mooz/js2-mode/tree/imenu

@mbuczko
I've abandoned the idea of private/public separation for now. Duplicate names are handled like in other cases, with <definition-1/2> subitems.
Regarding those, Semantic does something quite similar for Java ((require 'semantic/java) to see), only it uses earmuffs instead of angle brackets. I can switch to those, too, if it'll make it easier to handle this case inside ido-goto-symbol.

@magnars
I'm now looking into adding support for framework-dependent constructs. Supporting Object.create from ES5 in some form would be nice, but from what I can see it isn't meant to be used in the same way as _.extend and friends. That is, statically determining the name of the resulting constructor/class is going to be harder in most cases, because the method just returns an instance, and it can be used in a variety of ways. Correct?

Collaborator

dgutov commented May 13, 2012

So, uh, I have some progress to report: https://github.com/mooz/js2-mode/tree/imenu

@mbuczko
I've abandoned the idea of private/public separation for now. Duplicate names are handled like in other cases, with <definition-1/2> subitems.
Regarding those, Semantic does something quite similar for Java ((require 'semantic/java) to see), only it uses earmuffs instead of angle brackets. I can switch to those, too, if it'll make it easier to handle this case inside ido-goto-symbol.

@magnars
I'm now looking into adding support for framework-dependent constructs. Supporting Object.create from ES5 in some form would be nice, but from what I can see it isn't meant to be used in the same way as _.extend and friends. That is, statically determining the name of the resulting constructor/class is going to be harder in most cases, because the method just returns an instance, and it can be used in a variety of ways. Correct?

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars May 13, 2012

They both work the same, in that they keep the constructor of the first argument:

> var EventEmitter = require("events").EventEmitter;
undefined
> var e = new EventEmitter();
undefined
> e.constructor.name;
'EventEmitter'
> Object.create(e).constructor.name;
'EventEmitter'
> require("underscore").extend(e, { newProp: true }).constructor.name;
'EventEmitter'
>

Great news, by the way. :-)

magnars commented May 13, 2012

They both work the same, in that they keep the constructor of the first argument:

> var EventEmitter = require("events").EventEmitter;
undefined
> var e = new EventEmitter();
undefined
> e.constructor.name;
'EventEmitter'
> Object.create(e).constructor.name;
'EventEmitter'
> require("underscore").extend(e, { newProp: true }).constructor.name;
'EventEmitter'
>

Great news, by the way. :-)

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov May 13, 2012

Collaborator

Yep, that was a bad comparison. What I should have written is, $/_.extend are different from the class-based inheritance constructs provided by various frameworks, in that the latter usually contain the name of the descendant class in the same statement.
For Object.create and _.extend the best we can probably do is show all such methods together in some shared namespace. Would that be useful at all? The only thing I've ever used _.extend for is to pass additional fields to templates.

Collaborator

dgutov commented May 13, 2012

Yep, that was a bad comparison. What I should have written is, $/_.extend are different from the class-based inheritance constructs provided by various frameworks, in that the latter usually contain the name of the descendant class in the same statement.
For Object.create and _.extend the best we can probably do is show all such methods together in some shared namespace. Would that be useful at all? The only thing I've ever used _.extend for is to pass additional fields to templates.

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars May 13, 2012

Anything that adds functions inside an extension type construct would be useful. How about just using the name of the first argument as a namespace?

Object.create(new EventEmitter(), {
    myMethod: function () {}
});

would be

new EventEmitter()/myMethod

?

Or just a common namespace, a.la:

methods/myMethod

or

object-literal/myMethod

or

{}/myMethod

would be nice.

magnars commented May 13, 2012

Anything that adds functions inside an extension type construct would be useful. How about just using the name of the first argument as a namespace?

Object.create(new EventEmitter(), {
    myMethod: function () {}
});

would be

new EventEmitter()/myMethod

?

Or just a common namespace, a.la:

methods/myMethod

or

object-literal/myMethod

or

{}/myMethod

would be nice.

@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko May 13, 2012

@magnars or maybe something like

EventEmitter / {} / myMethod

I totally agree, "extension" constructs are incredibly popular and it would be nice to see them handled by imenu.

Personally, I often base on backbone.js writing my code, using following construction:

App.Views.Shelf = Backbone.View.extend({
    template: JST['app/shelf'],
    el: '#container .shelf',
    events: {
        'click .dropzone a': 'buttonPressed'
    },
    initialize: function() {
    },
    render: function() {
    }
})

which unfortunately makes all functions invisible from imenu perspective.

@dgutov thanks for information about the progress. nice to hear you're still actively working on that :)

mbuczko commented May 13, 2012

@magnars or maybe something like

EventEmitter / {} / myMethod

I totally agree, "extension" constructs are incredibly popular and it would be nice to see them handled by imenu.

Personally, I often base on backbone.js writing my code, using following construction:

App.Views.Shelf = Backbone.View.extend({
    template: JST['app/shelf'],
    el: '#container .shelf',
    events: {
        'click .dropzone a': 'buttonPressed'
    },
    initialize: function() {
    },
    render: function() {
    }
})

which unfortunately makes all functions invisible from imenu perspective.

@dgutov thanks for information about the progress. nice to hear you're still actively working on that :)

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov May 13, 2012

Collaborator

Anything that adds functions inside an extension type construct would be useful. How about just using the name of the first argument as a namespace?
...

I think a common namespace (<extensions>? ) would be the least error-prone option. From what I can see (e.g. in the Addons SDK sources), the first argument is usually not a direct instantiation, but most often a reference to variable assigned to previously or passed in as a function argument.

By the way, since _.extend modifies its first argument, this usage (from backbone.js) directly shows the "descendant":

 _.extend(Collection.prototype, Events, { ...

Personally, I often base on backbone.js writing my code, using following construction

Backbone.View.extend is actually a case of class-based inheritance, the left side of the assignment has the descendant's name, so there's no question where to show the new methods. Similar to Ext.extend and, to a lesser degree, dojo.declare and $.widget.

Collaborator

dgutov commented May 13, 2012

Anything that adds functions inside an extension type construct would be useful. How about just using the name of the first argument as a namespace?
...

I think a common namespace (<extensions>? ) would be the least error-prone option. From what I can see (e.g. in the Addons SDK sources), the first argument is usually not a direct instantiation, but most often a reference to variable assigned to previously or passed in as a function argument.

By the way, since _.extend modifies its first argument, this usage (from backbone.js) directly shows the "descendant":

 _.extend(Collection.prototype, Events, { ...

Personally, I often base on backbone.js writing my code, using following construction

Backbone.View.extend is actually a case of class-based inheritance, the left side of the assignment has the descendant's name, so there's no question where to show the new methods. Similar to Ext.extend and, to a lesser degree, dojo.declare and $.widget.

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars May 14, 2012

I think the best solution is to avoid supporting just certain packages like underscore and jquery, and instead add all methods in object literals under the common namespace of {}.

magnars commented May 14, 2012

I think the best solution is to avoid supporting just certain packages like underscore and jquery, and instead add all methods in object literals under the common namespace of {}.

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov May 17, 2012

Collaborator

This uses both approaches. Please test it and report back.

(eval-after-load 'js2-mode
  '(progn
     (require 'js2-imenu-extras)
     (js2-imenu-extras-setup)))
Collaborator

dgutov commented May 17, 2012

This uses both approaches. Please test it and report back.

(eval-after-load 'js2-mode
  '(progn
     (require 'js2-imenu-extras)
     (js2-imenu-extras-setup)))
@mbuczko

This comment has been minimized.

Show comment
Hide comment
@mbuczko

mbuczko May 17, 2012

Looks brilliant at the first sight, thanks! I will try to test it in details during next few days.

mbuczko commented May 17, 2012

Looks brilliant at the first sight, thanks! I will try to test it in details during next few days.

@magnars

This comment has been minimized.

Show comment
Hide comment
@magnars

magnars May 19, 2012

Wow, this is great! Thanks. :-)

magnars commented May 19, 2012

Wow, this is great! Thanks. :-)

@dgutov

This comment has been minimized.

Show comment
Hide comment
@dgutov

dgutov Jun 3, 2012

Collaborator

Merged into master.

Collaborator

dgutov commented Jun 3, 2012

Merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment