Closed feltech closed 5 years ago
Specifically, I found this when trying to use the cquery (C++) language server. Opening a .cpp
file then <C-S-t>
too quickly would lead to a search box with a loading spinner that never stops loading. This was caused by an exception like
Error: /path/to/file.cpp is being indexed.
at new ResponseError (/path/to/oni/node_modules/vscode-jsonrpc/lib/messages.js:46)
at handleResponse (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:430)
at processMessageQueue (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:258)
at Immediate._onImmediate (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:242)
at runCallback (timers.js:789)
at tryOnImmediate (timers.js:751)
at processImmediate [as _immediateCallback] (timers.js:722)
which I traced to a specific error code coming from the language server.
Hmm, those "WARNING: head commit changed" commits are annoying, but harmless (they're empty). I think I accidentally tried to amend a commit I'd already pushed. Not sure how those messages got put there, I've not seen that before. Something to do with eclipse (which I use as a history/conflict viewer), probably.
@feltech thanks for the PR its nice to see the language server get some much needed love and tests as well 😍, re. the retry strategy I'm wondering if its possible to also delay queries till the server is initialised. There is an intialize request
sent by the client in the spec and possibly we can await its result before taking other requests?
Merging #2634 into master will increase coverage by
0.3%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #2634 +/- ##
=========================================
+ Coverage 45.38% 45.68% +0.3%
=========================================
Files 361 361
Lines 14576 14586 +10
Branches 1915 1916 +1
=========================================
+ Hits 6615 6664 +49
+ Misses 7737 7698 -39
Partials 224 224
Impacted Files | Coverage Δ | |
---|---|---|
browser/src/Editor/NeovimEditor/Symbols.ts | 64.38% <100%> (+58.03%) |
:arrow_up: |
...lugins/Api/LanguageClient/LanguageClientHelpers.ts | 65.85% <0%> (+2.43%) |
:arrow_up: |
browser/src/Utility.ts | 50.74% <0%> (+3.73%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update fdc740c...46d619a. Read the comment docs.
Indeed, that initialize
sounds like the real answer, I'll look into it.
Unfortunately, at least for cquery, the initialize
request seems inadequate. Oni logs "[LanguageClientManager]: Initialized" after the initialize
request completes, but we still see the exception after that (see pic of debug console)
@feltech based on the protocol it seems you have to wait for the InitializeResult
to be sent back from the lsp, does the initialize
call return with the InitializeResult or is that sent back separately? I think based on this example
there might be separate notification event from the lsp when the initialization is complete
Btw don't let the initialisation issue be a blocker for now, I'm planning to have a look at bits of our lsp client at some point to facilitate things like #1717
Good stuff! I have a couple more PRs incoming, so I'll perhaps take a deeper looks at language server initialize later.
Can this be merged now then? (I assume it was waiting on some checks when you approved it @Akin909)
@CrossR was good to merge just forgot to hit the button