Skip to content

Handle LSP Initialize request synchronously #1750

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Handle LSP Initialize request synchronously #1750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ghcide/src/Development/IDE/LSP/LanguageServer.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ runLanguageServer options inH outH getHieDbLoc defaultConfig onConfigurationChan
logError (ideLogger ide) $ T.pack $
"Unexpected exception on notification, please report!\n" ++
"Exception: " ++ show e
ReactorRequest _id act k -> void $ async $
ReactorRequestSync _id act k ->
checkCancelled ide clearReqId waitForCancel _id act k
ReactorRequestAsync _id act k -> void $ async $
checkCancelled ide clearReqId waitForCancel _id act k
pure $ Right (env,ide)

Expand Down
10 changes: 8 additions & 2 deletions ghcide/src/Development/IDE/LSP/Server.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import UnliftIO.Chan

data ReactorMessage
= ReactorNotification (IO ())
| ReactorRequest SomeLspId (IO ()) (ResponseError -> IO ())
| ReactorRequestSync SomeLspId (IO ()) (ResponseError -> IO ())
| ReactorRequestAsync SomeLspId (IO ()) (ResponseError -> IO ())

type ReactorChan = Chan ReactorMessage
type ServerM c = ReaderT (ReactorChan, IdeState) (LspM c)
Expand All @@ -43,7 +44,12 @@ requestHandler m k = LSP.requestHandler m $ \RequestMessage{_method,_id,_params}
trace x = otTracedHandler "Request" (show _method) $ \sp -> do
traceWithSpan sp _params
x
writeChan chan $ ReactorRequest (SomeLspId _id) (trace $ LSP.runLspT env $ resp' =<< k ide _params) (LSP.runLspT env . resp' . Left)
item = case _method of
SInitialize ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dead code - handleInit is the actual initialize request handler. lsp will never actually call a InitializeRequest handler defined the regular way since initialize is special. Instead it defines its own handler, into which custom logic can be inserted using doInitialize

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case it sounds like InitializeRequest is already being handled synchronously, is that right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

I've also been confused by the out of order progress messages and something non-obvious is going on. It might even be a bug in lsp-test.

I did try to investigate the ghcide code path and I couldn't figure out how it was possible for us to send progress messages before the initialize response.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second part seems easy to explain: we call shakeOpen inside the doInitialize handler, shakeOpen will evaluate the rules and call kick, and kick may trigger progress reporting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I thought shakeOpen was only initializing state.

ReactorRequestSync (SomeLspId _id) (trace $ LSP.runLspT env $ resp' =<< k ide _params) (LSP.runLspT env . resp' . Left)
_other ->
ReactorRequestAsync (SomeLspId _id) (trace $ LSP.runLspT env $ resp' =<< k ide _params) (LSP.runLspT env . resp' . Left)
writeChan chan item

notificationHandler
:: forall (m :: Method FromClient Notification) c. (HasTracing (MessageParams m)) =>
Expand Down