feat: Path patching API #709
Conversation
angelozerr
force-pushed
the
file_uri_api
branch
7 times, most recently
from
ba3bb52 to
13e625d
Compare
|
@SCWells72 is it possible that you test my PR just to check that I have not broken something. I would like to merge it ASAP to use it when we will develop other features and discover some potential problem. Thanks! @InSyncWithFoo you can test the PR if you wish by installing build zip. |
There was a problem hiding this comment.
Given the breadth of this change, it's difficult for me to ascertain the risk of it. Conceptually it makes sense, but I think you're largely going to have to rely on the unit tests (and perhaps some additional manual testing?) to make sure it's solid.
I did add a comment on at least one potential NPE (via a mismatched nullability annotation/usage) in this review, and I'd definitely recommend that you make a pass and confirm that all parameters that could be null are being treated as such...and ideally add nullability annotations that make those contracts clear.
| @@ -73,9 +73,11 @@ protected CompletableFuture<List<DocumentSymbolData>> doLoad(DocumentSymbolParam | |||
| } | |||
|
|
|||
| private static CompletableFuture<List<DocumentSymbolData>> getDocumentSymbolsFor(@NotNull DocumentSymbolParams params, | |||
| @NotNull PsiFile file, | |||
There was a problem hiding this comment.
Is there a reason that this signature changed to reverse the order of the second and third parameters? Perhaps this order just makes more sense, but wanted to confirm this wasn't just an accident with no motivation.
There was a problem hiding this comment.
I did that to align this rule with other LSP*Support (see LSPCompletionSupport).
angelozerr
force-pushed
the
file_uri_api
branch
2 times, most recently
from
7fbab55 to
3f6c56a
Compare
Fixes redhat-developer#702 Signed-off-by: azerr <[email protected]>
angelozerr
force-pushed
the
file_uri_api
branch
from
3f6c56a to
fb6cdb9
Compare
|
Thanks @SCWells72 for your review! |
All unit test that we have for completion, etc uses the default behavior of the new API. It would be nice to write tests with customization of the API https://github.com/redhat-developer/lsp4ij/blob/main/docs/LSPApi.md but it is very hard for me to find time to do that |
Path patching API
Fixes #702