Skip to content

Add wrap-type-in-module code action #1547

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

fprasx
Copy link

@fprasx fprasx commented Jul 21, 2025

Adds a code action that takes type declarations and wraps them in a module that includes a type t.

@xvw
Copy link
Collaborator

xvw commented Jul 22, 2025

Hi @fprasx !
First of all, thank you very much for your contribution! It is indeed a feature that we find particularly desirable and useful!

Currently, @Tim-ats-d is coming to the end of his internship working on a Merlin-based refactor engine, and he has an experiment that offers similar features (without preserving formatting, but I'm not sure that's particularly necessary). The advantage of his approach is that it relies on parsetree manipulation rather than strings, which, in my opinion, makes the feature a little more robust.

Furthermore, we would like to preserve the fact that the features extend the Merlin protocol and that we use OCaml-lsp-server as an interface between Merlin and the various clients, greatly simplifying the maintenance of OCaml version upgrades.

We can either share Merlin's experimental branch, which offers this feature, and try to pool the tests, while keeping the code action as the main interaction. We can also take care of the upstreaming on our side. What makes the most sense to you?

@fprasx
Copy link
Author

fprasx commented Jul 22, 2025

Happy to contribute!

I agree that using the parsetree to refactor seems more robust. Given my current development environment, I think it'll be pretty difficult for me to incorporate the new refactor engine. I think it's therefore best if y'all handle the upstreaming. Regardless, I'm happy to answer any questions and otherwise be helpful :)

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