Skip to content

OCaml: Add type enclosing #4741

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

Merged

Conversation

mattiasdrp
Copy link
Contributor

This is the second of a series of PR I aim to do to improve the ocaml-lsp experience in lsp-mode

This PR exposes lsp-ocaml-type-enclosing that allows to get the type at point and improve it

This is still a draft PR because I need to add more verbosity and index options allowing to show types behind aliases and much more

This PR is based on #4732 and shouldn't be merged until the first one is

@github-actions github-actions bot added documentation client One or more of lsp-mode language clients labels Mar 20, 2025
@mattiasdrp mattiasdrp force-pushed the mattias@ocaml-lsp-type-enclosing branch 7 times, most recently from a32b3b4 to 2fd1ac3 Compare March 25, 2025 22:49
@mattiasdrp mattiasdrp marked this pull request as ready for review March 25, 2025 22:49
@mattiasdrp
Copy link
Contributor Author

This PR is now ready. It implements functions using the custom request defined in https://github.com/ocaml/ocaml-lsp/blob/master/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md

@mattiasdrp mattiasdrp force-pushed the mattias@ocaml-lsp-type-enclosing branch 4 times, most recently from a3991e7 to e1e880c Compare March 25, 2025 23:21
@jcs090218
Copy link
Member

I'm a bit confused. Does the https://github.com/freebroccolo/ocaml-language-server no longer work? It returns 404 now. 😕 Should we remove it?

Can you rebase to resolve the conflict? Thanks! 😋

@mattiasdrp
Copy link
Contributor Author

I'm a bit confused. Does the https://github.com/freebroccolo/ocaml-language-server no longer work? It returns 404 now. 😕 Should we remove it?

I don't know anyone that uses it and it hasn't been maintained for the last 8 years so I think it could be removed, yes

@jcs090218
Copy link
Member

Can you fix the CI? Thank you!

@mattiasdrp mattiasdrp force-pushed the mattias@ocaml-lsp-type-enclosing branch from 3f000d0 to 43220c0 Compare March 31, 2025 07:17
@mattiasdrp
Copy link
Contributor Author

Can you fix the CI? Thank you!

Should be ok now ;-)

This also creates a transient keymap allowing to:

- increase/decrease the verbosity of the displayed type
- copy the computed type
- Increase/Decrease index
- Show the region that is currently being typed
…nature

This function was useful because textDocument/hover returned the type and
documentation of the identifier at point making it hard to kill the type only

Now that ocamllsp/typeEnclosing is used instead there is no reason to keep this
poorly written function (I was young and innocent)
@mattiasdrp mattiasdrp force-pushed the mattias@ocaml-lsp-type-enclosing branch from 43220c0 to 71a54fa Compare March 31, 2025 07:20
@jcs090218 jcs090218 merged commit 7c0df12 into emacs-lsp:master Mar 31, 2025
18 checks passed
@jcs090218
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants