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

Implement basic symbol provider v.2 #164

Open
wants to merge 4 commits into
base: dev
from

Conversation

@mrkam2
Copy link
Contributor

commented Apr 14, 2019

This PR implements second version of basic symbol provider. It is now using apropos to retrieve name type. It also avoids adding symbols with incorrect file information.

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

@PEZ WDYT? If you reproduce some of the issues mentioned before, could you please provide more specific logs and steps to reproduce?

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented Apr 14, 2019

This time around I do not get any symbols at all. Could you have something you have not pushed yet left on your machine?

return client.info(ns, v);
}));
return items.map((info: any) => new vscode.SymbolInformation(
const aproposResponse = await client.apropos({

This comment has been minimized.

Copy link
@mrkam2

mrkam2 Apr 15, 2019

Author Contributor

@PEZ I switched to use apropos and its middleware.
Mind checking the value of aproposResponse trying this branch your system?

This comment has been minimized.

Copy link
@PEZ

PEZ Apr 15, 2019

Collaborator

There are no apropos-matches in the response when I run this. It is a done response... Let's chat on Slack, maybe we can use Hangout or something to figure it out together.

This comment has been minimized.

Copy link
@PEZ

PEZ Apr 15, 2019

Collaborator

Actually, I now see that I have an appropos-matches, an empty array, in the response. I'm sorry for misleading you with my previous comment.

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented Apr 20, 2019

OK. So now I've fiddled around with the dependencies and gone full circle with them, ending up with the same that I started with, and now the symbols appear for some reason.

Findings after a quick test drive:

  • The problem with strange symbols appearing is gone.
  • There are now no symbols for the user namespace in my test project. (Not a stopper, IMO, but that's how it is anyway.)
  • The Outline pane sometimes populates with the symbols now. But sometimes not. I guess that needs some attention. Would be a super extra benefit to get that working consistently.

Are you planning on presenting functions and regular defs differently? I don't know what you get from apropos that could help with that, but if there is such info, it would make the symbols least easier to parse, I think.

Let me know if you need assistance merging in the updates from dev.

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Are you planning on presenting functions and regular defs differently?

I though I already did that in "Go To Symbol..." list.

The Outline pane sometimes populates with the symbols now. But sometimes not.

I'll look into Outline separately. My goal was to fix "Go To Symbol..." list, this seems like a byproduct.

There are now no symbols for the user namespace in my test project.

It hides any symbols that have no file information. Maybe that's the case with user ones. What would be a desired workflow for those?

Let me know if you need assistance merging in the updates from dev.

I probably don't need. Why can't this be merged into master first? What's the plan with dev?

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented Apr 23, 2019

When I tested def and defn symbols looked the same in the list.

About the Outline pane, I agree it is a side product, but right now this feature leaks out there, so either we need to stop that, or fix it so that we populate it consistently. IMO.

The user namespace is special. I think that no, symbols works for now. But desired would be to see all the symbols that can be accessed from there.

About master and dev. Right now master is really just a maintenance branch for Calva v1 and all new development should go into Calva 2, which happens in dev. When Calva 2 is released, we'll be back with maintaining that in master. But right now they differ too much for me to have the time to maintain both with new features.

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Bumping this!

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

I did some investigation. Seems like Outline is directly based on SymbolProvider output that is a requirement for Go To Symbol feature. However, they seem to have different lifecycle: outline only queries symbols once when document is first opened. Go to symbol does the same query every time "Go to Symbol" dialog is shown. When REPL is not connected or namespace is not yet evaluated, provideDocumentSymbols function would return nothing. Therefore, any documents opened during this time would have empty Outline. So far I wasn't able to find a way to refresh the Outline later.

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

Since I'm new to VSCode programming I might be missing something. But so far it seems that if we want to fully support the Outline view, we should be evaluating and loading symbols for each document upon opening. Does it sound reasonable? @PEZ

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented May 12, 2019

Yes, it sounds reasonable. But maybe there is also a way to explicitly tell vscode that we do not support the outline view yet? It'll be a matter about how much work there is with any path we choose.

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

I'm not an expert in VSCode but from what I've learned so far it seems like it's not customizable that way.

@PEZ

This comment has been minimized.

Copy link
Collaborator

commented May 30, 2019

I have dropped this ball a bit. What is the status as far as you know, @mrkam2 ?

@mrkam2

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.