Skip to content
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

Extraneous <global> symbol from Salsa #11230

Closed
waderyan opened this issue Sep 28, 2016 · 6 comments
Closed

Extraneous <global> symbol from Salsa #11230

waderyan opened this issue Sep 28, 2016 · 6 comments
Labels
External Relates to another program, environment, or user action which we cannot control. VS Code Tracked There is a VS Code equivalent to this issue

Comments

@waderyan
Copy link

From @mousetraps on September 28, 2016 20:40

Testing #12598

image

Copied from original issue: microsoft/vscode#12936

@waderyan waderyan self-assigned this Sep 28, 2016
@waderyan waderyan added the VS Code Tracked There is a VS Code equivalent to this issue label Sep 28, 2016
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Sep 28, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 28, 2016

The top level entry in NavBar call will always be <global> or <module name>. if VSCode does not want to show that, then the top entry should be filtered out.

I do not think this is a change on the server side, but on VSCode side. closing.

@mhegazy mhegazy closed this as completed Sep 28, 2016
@mhegazy mhegazy added External Relates to another program, environment, or user action which we cannot control. and removed Bug A bug in TypeScript labels Sep 28, 2016
@dbaeumer
Copy link
Member

dbaeumer commented Sep 29, 2016

@mhegazy what is the reason to have that in in the first place for JS. This is requesting symbols for a specific document. As I understand the it is about global symbols.

And for TS files it looks like we are not receiving such a global entry. So fixing this on our end seems a little wired.

@dbaeumer
Copy link
Member

dbaeumer commented Sep 29, 2016

I debugged it a little and the returned structure is as follows;

Response.body:
   - function foo
   - <global>
      - function foo

So there is no single entry :-)

@dbaeumer
Copy link
Member

There is more wiredness:

function foo() {
    function bar() {
        function faz() {

        }   
    }
}

Produces the following Nav item tree

body
   - <global>
   - foo
        - bar (with no children which is wrong as well)
   - bar <== this is wrong
        - faz

@mhegazy
Copy link
Contributor

mhegazy commented Sep 29, 2016

The issue here is that NavBar really returns output suitable for two-drop-down list format that VS uses.

I think the best solution here is to add a new command to the server, getLexicalStrucutre, or getNavigationTree, that has the tree form. this should be fairly simple for us, since we already generate the output as a tree, then serialize it in the two-drop-down-list format for VS.

I have logged #11250 to track adding the new command.

If you are interested in reverse-serializing the output of navBar, here is the algorithm:

  • Remember all top-level entries in a dictionary, keyed by name + indent.
  • Start by the first top-level item (this is going to be <global> or a module)
    • walk its childItems, and serialize them recursively
      • For each item, check your top level dictionary first, if an entry was found at the top level use it, if not then use this one.

@dbaeumer
Copy link
Member

dbaeumer commented Oct 4, 2016

@mhegazy thanks for the clarification. Could be prioritize #11250 for 2.0.5. Sounds easy on your end. Easier than use added the reverse-serializing code.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
External Relates to another program, environment, or user action which we cannot control. VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

4 participants