Skip to content

Sending workspace/configuration request while handling Initilized #3745

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

Closed
wants to merge 1 commit into from

Conversation

July541
Copy link
Collaborator

@July541 July541 commented Aug 6, 2023

We can rely on workspace/configuration to keep track of the configuration after we resolve the following problems.

  • How to upgrade the config in the Initialized handler, seems we can't modify the IdeState in place.
  • We have dozens of configurations, do these config field names all available for different clients like vscode and emacs?
  • How to test this pr? Ideally I think we can add a test in vscode-haskell.

I'll try to investigate these questions, and also very welcome any suggestions or guidance in case I have to browse through lsp protocol and some of their implementations :)

@michaelpj
Copy link
Collaborator

We should have handling for this in the part of ServerDefinition that deals with didChangeConfiguration today. So we should just be able to port that over to here. I do think that lsp should handle this generically, but if we can patch this in here that's a good short term fix!

void $ sendRequest LSP.SMethod_WorkspaceConfiguration params $ \res -> do
liftIO $ logWarning (ideLogger ide) $ "zltest: " <> (Text.pack $ show res)
pure ()

Copy link
Collaborator

Choose a reason for hiding this comment

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

you probably want to block waiting for the response to SMethod_WorkspaceConfiguration here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we don't need to. We have a default config, so we should always be able to operate and just update the config dynamically when we get the response, as we would if we got didChangeConfiguration.

@michaelpj
Copy link
Collaborator

I've made a decent amount of progress on this. I have it fixed generically in lsp, I'm just making lsp-test handle it properly, and I think I've also got HLS working on top of that. I'm going away for the weekend but I think I should be able to finish it off early next week.

@July541
Copy link
Collaborator Author

July541 commented Aug 21, 2023

@michaelpj I have some hours available this Sunday, so I'll continue your work if you don't have enough time.

@July541 July541 closed this Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants