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

Added support for documentSymbolProvider and workspaceSymbolProvider #22

Open
wants to merge 3 commits into
base: parser
Choose a base branch
from

Conversation

simonvanbernem
Copy link

Most of the code is in the newly added file symbols.jai, which does the nitty gritty. To have this not be slow, I did a couple of things:

2 acceleration structures were added to Program_File. One of them contains json-serialized workspace symbols, because profiling showed that json serialization took about 98% of the response time. To be able to properly use the cached serialized symbols in the response, I added lsp_respond_with_already_json_serialized_result_data, which lets you specify the result as already serialized json, puts that in the complete response and sends it back to the lsp client.

Additionally, the number of workspace symbols returned can be limited with MAX_NUMBER_OF_WORKSPACE_SYMBOLS_TO_RETURN_TO_PREVENT_VSCODE_FROM_BEING_SUPER_SLOW, because in my codebase, there were 80,000 symbols, and while generating the response with the cached json symbols was pretty quick, visual studio code takes like half a second to handle the response, which feels horrible. This shouldn't be a problem, because when you type to narrow down the search, the LSP still searches all symbols without the limit.

For the search query, we consider a symbol to match in the following case:
Each part of the search query that is separated by a space is considered its own search term. (Currently VSCode doesn't even include the space and anything after it, so this is not strictly useful, but this is sublime text 3 behavior that I like and want to replicate with another extension maybe, and since it is just "inactive" currently, I added it). All search terms have to match a symbol name for the symbol to be considered to match. A search term matches a symbol name, if all characters of the search term appear in the same order in the name.

This is a very loose matching system, but vscode actively advocates for this in favor of search for sub-strings, so it has the most options when displaying search results to you, which is reasonable I think.

@simonvanbernem
Copy link
Author

I also added a hacky fix to get_identifier_decl, which failed to find declarations for many, many functions in my program when trying to use goto definition. I didn't realize it would end up in the same pull request if I commited it in the same branch, sorry.

@simonvanbernem
Copy link
Author

simonvanbernem commented Sep 27, 2024

Has this repo been abandoned? I think the features in this PR could be useful for many people using Jails, so leaving it without merge or comment for 3 months seems unfortunate.

I'm using Jails regularly now and and like it, but also think it could use quite some work (providing it via the marketplace, memory leaks, soft locks where it stops providing symbols...), so I just wanted to check in on what the current state of this repository is.

Edit: I just saw there was activity here, just not on this PR. Nevermind the question then, I didn't mean to insinuate anything.

@SogoCZE
Copy link
Owner

SogoCZE commented Sep 27, 2024

Has this repo been abandoned? I think the features in this PR could be useful for many people using Jails, so leaving it without merge or comment for 3 months seems unfortunate.

I'm using Jails regularly now and and like it, but also think it could use quite some work (providing it via the marketplace, memory leaks, soft locks where it stops providing symbols...), so I just wanted to check in on what the current state of this repository is.

Edit: I just saw there was activity here, just not on this PR. Nevermind the question then, I didn't mean to insinuate anything.

Hello, this repository is definitely not abandoned. I still want to work on it, but right now I have too much on my plate to dedicate any spare time. I’ll try to find some time in the near future to work on Jails again and review this PR.

By the way, I really appreciate your work on this PR. I just haven’t had the chance to properly look at it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants