-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: Added a file view provider for LSP files that yields fake PSI elements for words. #703
base: main
Are you sure you want to change the base?
Conversation
My first impression is that you are using documentSymbol as AST. I understand that it could fix a lot of problem with IJ integration but my fear is about performance. For little file, documentSymbol is fast but for large file it can be extremely slow and can consume a lotnof CPU. A very good sample is with our XML language server used in vscode-xml. If your xml file is large Outline which consumes documentSymbol can be very slow. The second problem is that language server could not provide documentSymbol or provides it without detailled ast nodes. |
@@ -0,0 +1,78 @@ | |||
/******************************************************************************* | |||
* Copyright (c) 2020 Red Hat, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2024
I love the idea but I need to play with the PR to see if it fixes several issues. |
Oh, it almost certainly needs quite a bit of work to make it do what it should and (more importantly) not do anything it shouldn't. That's why this is still in draft state. I may take a look at it again a bit later this week if I free up from some of my current obligations. |
Please ping me when you will think the PR will be ready. |
There's likely quite a bit that would need to be done to make this ready for actual merge, but I think the underlying concept shows great promise. One of the biggest obstacles that we have right now is the lack of an actual PSI tree for LSP4IJ-managed files. There's quite a bit of fake PSI element creation that occurs. This could help to centralize it by making it part of the actual view provider and responding to
findElementAt(offset)
with a fake PSI element for the word/token under at that offset.For example, we all know the issue right now with Ctrl+Mouse hover where the entire document gets hyperlinked, but this already addresses that by highlighting only the word under the mouse:
I'm going to see whether or not this also makes
breadcrumbsInfoProvider
just work.UPDATE: I can confirm that this general approach does make it possible to implement
breadcrumbsInfoProvider
, but in reality it would work best if we created and maintained a skeletal (fake) PSI tree with the nested declarations fromtextDocument/documentSymbol
and word tokens within those. If we could do that efficiently, I think it would unlock many other EPs and/or advanced EP features.It's also worth investigating whether
findReferenceAt(offset)
can be implemented here to create a reference based on the LSP-derived reference information such that those references just work everywhere that expects aPsiReference
.Like I said, lots of promise, but probably lots of things to do to make sure it works the way we'd want without causing other issues.
Just looking for thoughts on the concept right now...not even close to something that would get merged.