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

DAP and LSP use a common Lookup. #5742

Merged
merged 2 commits into from
Mar 31, 2023
Merged

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Mar 29, 2023

When the user code is run/debug, DAP protocol is used for the launch request. Although the DAP server provided OperationContext for reporting progress, it did not provide any other LSP client's functions to the DAP-initiated processes within NBLS.
So when one runs (using codelen or command) a project or main class in a Maven project, with bad proxy settings, the Maven support wants to ask the user - and fails, since the DAP-initiated maven build has no access to DialogDisplayer that works through LSP connection with the LSP client to display confirmations.

This PR attempts to change DAP + LSP implementation so that the default Lookup for their request processing conains both DAP and LSP lookups (and of course request-specific items and the default Lookup as the last one).

@sdedic sdedic added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests labels Mar 29, 2023
@sdedic sdedic added this to the NB18 milestone Mar 29, 2023
@sdedic sdedic self-assigned this Mar 29, 2023
@sdedic
Copy link
Member Author

sdedic commented Mar 30, 2023

Added a test to check that the default Lookup contents is present just once in commands, despite the proxies to proxies :)

@sdedic sdedic force-pushed the lsp/dap-lsp-commonlookup branch from 73284e9 to 8a59dc3 Compare March 30, 2023 19:09
@sdedic
Copy link
Member Author

sdedic commented Mar 30, 2023

I needed to change the way how the lookup extraction command hooks into the lookup. When I used MockLookup, it interfered with the default Lookup and prevented the module system from starting, so that Indexing support did not run its @OnStart, did not attach appropriate listeners on PathRegistry ... and finally it did not produce 'indexing finished' message at all - because it had no listeners hooked on the RepoUpdater.

@sdedic sdedic merged commit b3cf9b9 into apache:master Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants