Add a new command to extract expression into a fresh let binding

[Tim-ats-d]

This PR adds a new command refactoring-extract-region to merlin protocol which allows extracting an expression into a let binding. See the interface comments for supported cases

Implementation

The implementation of the substitution is a bit hacky and relies on string manipulation, especially when the generated binding is a and. I couldn't deal with typedtree locations for the generated code, so I had to make a few manual location calculations that I tried to make as explicit as possible. Please tell me if there's a better way to achieve it.

It still lacks a bit of testing, I'll be adding more in the next few days.

cc @xvw @voodoos

[Tim-ats-d]

Just realized that I should probably add the corresponding documentation in the /doc/dev/PROTOCOL.md file

[xvw]

There is one test that seems fail. Is that an issue or you need to repromote it?

[Tim-ats-d]

There is one test that seems fail. Is that an issue or you need to repromote it?

No, it wasn't an issue, strictly speaking. It was an example of extraction that better not work, given that the extracted expression is much smaller than the size of the selected region the result is a bit confusing.

I think I'll replace this test with another one.

[Tim-ats-d]

There is one test that seems fail. Is that an issue or you need to repromote it?

I replaced it by other more relevant tests from ocaml-lsp (https://github.com/ocaml/merlin/pull/1948/commits/38055ff0515a76ea2e7cceae75cfcd4aeaaafb11#diff-1b5f000f9708e3dfe32d04e4ca43f63eb3d55ce173e7c8cd63bbca41e61639d1)

[voodoos]

Also, the Ci breakage should be fixed :-)

[Tim-ats-d]

I don't know if this PR can be merged: there are still a few bugs in the extraction of values belonging to local modules that could be improved but on the other hand, it works well for most basic use cases.

The extraction of let is also a thing to consider, as Xavier pointed out here (https://github.com/ocaml/merlin/pull/1948#discussion_r2211964274)

[voodoos]

I don't know if this PR can be merged: there are still a few bugs in the extraction of values belonging to local modules that could be improved but on the other hand, it works well for most basic use cases.

The extraction of let is also a thing to consider, as Xavier pointed out here (#1948 (comment))

At the very least, these "bugs" that you have identified should be reproduced in the testsuite with a FIXME stating the expected result. Maybe in separate test files to make them more easy to find ?

[xvw]

And after FIXME are added, I'll also make a last round of review this week!

[Tim-ats-d]

I have added the /tests/test-dirs/refactor-extract-region/extraction-issue.t directory and includes some FIXME tests

[voodoos]

@Tim-ats-d what do you think of 9bb9388c61cb18b340519f54c7473e58d2acf051 ?

This still feels a bit sketchy, but at least more cases are working now.

[Tim-ats-d]

@Tim-ats-d what do you think of 9bb9388 ?

This still feels a bit sketchy, but at least more cases are working now.

It's perfectly fine, thanks. I think that will do the trick for now.