JulianEberius / SublimeRope

ST2 only, use SublimePythonIDE with ST3: Adds Python completions and some IDE-like functions to Sublime Text 2, through the use of the Rope library
GNU General Public License v2.0
250 stars 26 forks source link

Add classmethods to globals for jump-to-globals list, similar to Eclipse... #60

Closed jasonmyers closed 11 years ago

jasonmyers commented 11 years ago

You may or may not want this, but I was missing how Eclipse's Globals browser also included classmethod definitions in its Jump-To-Globals list, e.g.

class Foo(object):
    def bar(self): pass
    def baz(self): pass

would include

Foo
bar
baz

as navigable items in the jump-to-global list

Also added a settings key to enable/disable this behavior, since it slows the search slightly (more objects to index)

JulianEberius commented 11 years ago

Hi,

Spend some time on this patch as well. The idea is nice, but I didn't really like the implementation. Sorry ;-) Firstly, it littered all parts of the code base with the "class_methods" parameter, passing it around in far to many places, when it can be simply set as an attribute of the AutoImporter one time when caches are regenerated. Secondly, the names added would not only appear in the jump-to-globals list, but also on import-assist, leading to wrong auto-imports. (E.g. if there is a method ModuleX.ClassA.methodB, typing "meth" and using autoimport would lead to "from ModuleX import methodB".) Lastly, the names where added without any reference to the class they belong to, which makes the whole jump-to-globals list less navigable as methods appear as global functions (which leads is also the reason for Problem 2!).

So I did my own implementation, only using your additions to AutoImporter.get_name_locations and AutoImporter._add_names, and rewriting everything else. The methods now appear as ClassA.methodB in AutoImporter.names, which creates a nicer jump-to-globals list (e.g. you can type ClassA and see all its methods), and removes problem number 2 (as methods are now prefixed with their class). I also removed all the parameter passing. Only the AutoImporter's attribute is used, and it is set only on cache regenerates directly from settings.

I would be happy if you could have a look at the commit (3e5db3e68bf258fc7aa9ae87e8e4bc318fed1f3c) and test this new implementation.

Julian

jasonmyers commented 11 years ago

Sounds good, I'll give it try

jasonmyers commented 11 years ago

Seems to all be working nicely. Thanks!