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

merlin: show behavior with non-default context #10035

Closed
wants to merge 2 commits into from

Conversation

jchavarri
Copy link
Collaborator

The test shows how dune ocaml merlin errors out when trying to get artifacts generated in a non-default context. Not sure if this is the expected behavior, but it might be helpful to have it documented.

Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com>
Signed-off-by: Javier Chávarri <javier.chavarri@gmail.com>
@rgrinberg
Copy link
Member

Yes, this is expected. There's a merlin field that can be used to determine which context is used for merlin.

@jchavarri
Copy link
Collaborator Author

I see, thanks for the pointers. I could find the test/blackbox-tests/test-cases/merlin/default-based-context.t test, which contains similar checks than the ones added in this PR.

For additional context 😅 I am investigating how to make dune choose one context or another more dynamically, as we would like to have ocaml-lsp query dune for artifacts in a specific context (as part of the work in #10222). For this, I understand something like the Dune context configuration would not be enough because it can't be changed from the clients. What I'm not sure about is if the change would need to be added in the interface between ocaml-lsp and dune (I understand, the ocaml-merlin server).

I could find the commands exposed by the ocaml-merlin server:

| File of string
| Halt
| Unknown of string

Would it be necessary to add some parameter to the File command to specify the context merlin is querying for?

@jchavarri jchavarri closed this Apr 1, 2024
@jchavarri jchavarri deleted the merlin-show-non-default-ctxt branch April 1, 2024 08:05
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.

None yet

2 participants