Move symbol/definition to other module

[jonathanmoregard]

When I want to move a Thing (= type, class, function, pattern etc) to another module, I have to:

1. Select the declaration
2. Cut to clipboard
3. Open the destination module
4. Paste from clipboard
5. Remove the Thing from the origin module's "module export list"
6. Add the Thing to the destination module's "module export list"
7. Go through all places where it has been imported and update the import statements to reflect the move.

I propose adding a code action called "Move.." which works like this:

1. Initiate the "Move.." action on a Thing
2. Open up a drop down (or equivalent) where you can filter/select a destination module from a list of all modules in the project.

Then Automatically:

1. Move the Thing you initiated the "Move"-action on to that module.
2. If it was exposed: remove it from the origin module's "module export list", add it to the destination module's "module export list"
3. Rewrite import statements pointing to the Thing in the origin module so that they point to the new location (destination module).

[ndmitchell]

I've wanted something similar - but my UI/use-case was slightly different, so adding it here so they can be considered in tandem.

I want to be able to copy a fragment (using standard copy ideally) then paste it into another module and have all the imports be filled in. Things like removing export, deleting imports are either easy, or have automatic cleanups already. Finding the imports that a fragment required is much more work. I imagine a "Paste with imports" button or similar. Not sure what of that is possible within the constraints of LSP.

[jneira]

It seems the first proposal could be combined with @ndmitchell one to be "complete": if the code being moved is using definitions that are not imported in the destination module, it will not compile so add the needed imports in one go would be really sweet (deleting the no used ones in the original module would be extraordinary)

That said, afaik till the date, we have not added isolate code actions, only quick fixes linked with diagnostics (errors or warnings) or linked to code lenses. I think the reason is we want to avoid define code actions in the extension itself, cause other editors (emacs, vim, etc) will not have them without replicating them. Quick fixes and code lenses are defined in the server and available for all editors for "free".

Maybe @alanz could help to clarifiy/confirm that though.

[alanz]

These features are in the domain of a refactorer, such as HaRe.

In terms of LSP, it should be possible to do something like select a function and be offered a code action to extract it to another module. The problem is, there is no ability to specify which module it should go to. The only affordance is rename, so possibly giving it a fully qualified name could specify the target module, and create it if it doesn't exist already.

[jonathanmoregard]

It seems the first proposal could be combined with @ndmitchell one to be "complete": it the code being moved is using definitions that are not imported in the destination module, it will not compile so add the needed imports in one go would be really sweet (deleting the no used ones in the original module would be extraordinary)

That is what I meant by Redirect any references (import statements in other modules) so that they point to the destination module.. I'll try clarifying.

[jneira]

The problem is, there is no ability to specify which module it should go to. The only affordance is rename, so possibly giving it a fully qualified name could specify the target module, and create it if it doesn't exist already.

I think we could follow this wise advise although ideally it should have support in the lsp spec and vscode, to being able to select the destination module graphically (or create a new one)

Related lsp spec issues:

• Support refactors that require user input/options microsoft/language-server-protocol#1164 : about adding generic refactorings with user input)

But i've not found any issue about move symbols between "modules" or the equivalent in other langs

[jonathanmoregard]

They have something similar in the elm. elm-tooling/elm-language-server#226
Can we use a similar approach to let the user select the module?

Also, @alanz, about HaRe: I like refactoring as I go along. Having to switch to a separate tool is a big hurdle to me. As such, if it's possible to get it in HLS, my life would be much easier.

[jneira]

The only affordance is rename, so possibly giving it a fully qualified name could specify the target module, and create it if it doesn't exist already.

That could be a way to go forward although having to write the module by hand instead pick it from a menu would be error prone (or simply drag the symbol to another module, like other ides do)

[jneira]

@jonathanmoregard thanks for the elm pointer, relevant notes from it:

• that change needed client support. We, for now, are putting all the logic in the server, so all editors get the same features with no extra work
• there the vscode issue How can a server implement "extract method" (how can it trigger rename?) microsoft/language-server-protocol#764 is linked. Ideally it should have native lsp spec support but that issue is stalled since 2 years ago 😟

[jonathanmoregard]

@jneira is the "putting all the logic in the server"-thing still the policy?

Also, is the "putting all the logic in the server" something that would be a blocker if someone submitted a PR to implement this feature in the vscode extension?

[jneira]

I think so but maybe another active maintainer could confirm it, @michaelpj?

[michaelpj]

The fact is that HLS's main problem is maintainability and lack of maintainer bandwidth. The LSP model is a huge blessing for us, because it means we can largely ignore clients. Even then, the vscode extension requires significant maintenance. So I think our general position is probably still to avoid doing custom things that take us back into the bad world of per-client support.

[jonathanmoregard]

@michaelpj I see, thanks for the context

[michaelpj]

I'm closing this as wontfix since I don't see us doing it any time soon.