Skip to content

Add command [ocaml-eglot-type-annotate]#74

Closed
arvidj wants to merge 1 commit into
tarides:mainfrom
arvidj:aj/type-annotate-enclosing
Closed

Add command [ocaml-eglot-type-annotate]#74
arvidj wants to merge 1 commit into
tarides:mainfrom
arvidj:aj/type-annotate-enclosing

Conversation

@arvidj

@arvidj arvidj commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

Add a new command ocaml-eglot-type-annotate to ocaml-eglot-type-enclosing-map that inserts a type annotation for the current enclosing with its inferred type:

Screencast.From.2025-11-07.12-31-53.mp4

TBH, not yet sure how useful it is.

@arvidj arvidj force-pushed the aj/type-annotate-enclosing branch from 18bd7f9 to 0d93eda Compare November 7, 2025 11:35
@arvidj arvidj marked this pull request as ready for review November 7, 2025 11:35
@xvw xvw self-requested a review November 7, 2025 12:11
@arvidj

arvidj commented Nov 10, 2025

Copy link
Copy Markdown
Contributor Author

Just learned about this: ocaml/ocaml-lsp#397

so this PR might be redundant w.r.t. that work, or rather, perhaps there should rather be an Emacs binding to that existing LSP action?

@xvw

xvw commented Nov 10, 2025

Copy link
Copy Markdown
Member

Hi ! Thanks a lot for your contribution!
I think attaching the type annotation to the enclosing makes a lot of sense, so would you mind if I experiment with the code action before reviewing/merging?

@arvidj

arvidj commented Nov 10, 2025

Copy link
Copy Markdown
Contributor Author

Hi ! Thanks a lot for your contribution! I think attaching the type annotation to the enclosing makes a lot of sense, so would you mind if I experiment with the code action before reviewing/merging?

Sure, there's no hurry on my end. It'd be good for me to test run this feature for a bit and gauge how helpful it is in the end.

@arvidj

arvidj commented Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

I ran into an issue today where the inserted annotation was the definition of a type (a record) instead of the name of the type. Basically, I ended up with my_record_value being annotated as my_record_value : { my_field : int; ...} instead of my_record_value : my_record_type as desired. I think we should look into whether the LSP command does this in a more sane manner.

@xvw

xvw commented Nov 17, 2025

Copy link
Copy Markdown
Member

Yes ! I guess that we can make the feature a little bit more ... dynamic (toggleTypeAnot) for example?

@xvw xvw mentioned this pull request Jan 20, 2026
@xvw

xvw commented Jan 21, 2026

Copy link
Copy Markdown
Member

Closed in favor of #84
Thanks a lot, I co-authored my commit with your!

@xvw xvw closed this Jan 21, 2026
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.

2 participants