Thanks for continuing the efforts!

This is how it looks in Atom:

The filename is jedie.py. This line used to prevent it from always showing.

Personally I'd prefer to not show x any y per default because things may become pretty cluttered on large files, but I'm happy to go with this and let more people chime in on the discussion.
I think it's OK to go with this for now and see if people complain about pyls showing too many symbols. Since pyls is pre 1.0 we can always iterate later.

pyls/plugins/symbols.py looks like this:

I added the check to remove the top-level module name. But there's not sensible way to get rid of x and y without excluding things like a. So I'm going to leave it in for now.

@lgeiger I just had a thought... the names should be fully qualified. Otherwise a variable defined in constructors of two classes (init method) can't be disambiguated.

I added the check to remove the top-level module name.

👍

But there's not sensible way to get rid of x and y without excluding things like a. So I'm going to leave it in for now.

That's totally possible. That's what this check did.

@lgeiger I just had a thought... the names should be fully qualified. Otherwise a variable defined in constructors of two classes (init method) can't be disambiguated.

I don't know if I fully understand you, but I think they can be distinguished easily by the indentation in the UI since the __init__ methods hase different parents.

What I mean is that a will have containerName __init__, as will b.

I think we already make that check right? https://github.com/palantir/python-language-server/pull/176/files#diff-cee448b8821a222944d7e6f2ca332ddaR28

[edit] I see what's missing. Let me fix.

What I mean is that a will have containerName init, as will b.

Yes, that's why the correct start and end positions are important for the UI to figure out where they belong.

Got it, the positions are used in addition to containerName.

Also, on second thoughts, a variable local to a function is still a symbol. So going to leave them.

I think we already make that check right?

No, the check here also tests if the statement is defined inside a function and omits it. If parent.parent() isn't defined it's in the global scope, otherwise it was defined inside a function.

Got it, the positions are used in addition to containerName.

Yeah, it's not exactly the most elegant way to infer hierarchy 😄

Also, on second thoughts, a variable local to a function is still a symbol. So going to leave them.

I'm OK with this. I don't know what the canonical way would be.

Ok, sounds sounds like we're in agreement. I also don't mind if the client excludes variables whose container symbol is a function. Most outline views tend to be expandable trees though, so I wonder if it can just be solved with default level of expanded subtrees.

Anyway, going to merge to get this out the door. Thanks for all your help / opinions!

so I wonder if it can just be solved with default level of expanded subtrees.

It's not so easy to solve this with default levels since one would also exclude class methods then.

Anyway, going to merge to get this out the door. Thanks for all your help / opinions!

Cool looking forward to the next release.

Do you think that we will have a new tagged release for this soon?

@lgeiger The is a new release now.