Make casesplit a HaRe action #917
Conversation
|
What do you think about refactoring all the mkCodeActions in HaRe.hs? |
|
I think the intention behind this ticket is to make the case split command generally available, by adding a case to the HaRe Plugin code action generator to offer a command to split the case, which activates this command. So moving the mechanics of it to the HaRe Plugin is half the job, the other half is to modify codeActionProvider to generate the command. Doing that would mean it is available in all HIE clients, and can be removed from the vscode client, as a special case. |
|
|
||
| mkCaseSplitAction name = do | ||
| let args = [J.toJSON $ HP (docId ^. J.uri) pos] | ||
| title = "Case split on " <> name |
There was a problem hiding this comment.
@alanz This generates the command. It works on neovim.
|
Oops, sorry. I saw the matching change on vscode, and assumed it was just a switch over. I think to ease the transition we should keep it exposed in ghc-mod for the time being. This would involve re-instating the command there, and getting it to call the function from HaRe. Which hopefully does not cause an import loop. |
|
I'll give moving splitCaseCmd and HarePoint to a new module a try tomorrow. I think that will avoid circular dependencies. It's easy to miss in all the duplicate mkCodeAction definitions. That's why I'm advocating for the linked refactor. |
|
The problem with the linked refactor is that not everyone updates their hie installation all that often, and vscode pushed updates pretty much immediately. So we need to support the old and the new for a while, else things will break. |
|
In fact, the simplest might be to leave the command in the ghc-mod plugin, but just add the code action generation to it. Then we can make an issue to migrate it to the HaRe module at a future date, when we have removed the explicit code from the vscode plugin. |
|
Can we have the ghc-mod action trigger the now migrated HaRe action via the client? I think that would be the cleanest migration path |
| debugm $ show name | ||
| IdeResultOk <$> sequence [ | ||
| mkCaseSplitAction name | ||
| ] |
There was a problem hiding this comment.
The command generation stuff LGTM
|
@lorenzo I'm not clear how we could call the HaRa command with a GhcMod command. What I have done is move anything that would case circular dependence to HieExtras. |
|
@alanz I rebased the refactor on the new changes that don't break compatibility. What are your thoughts on: Avi-D-coder/haskell-ide-engine@9f91084 |
|
@Avi-D-coder Its looking good, and the refactoring of the HaRe command construction makes it a lot clearer, in my opinion. |
|
@Avi-D-coder I was meaning something like this https://github.com/haskell/haskell-ide-engine/pull/889/files#diff-df7cc8b4c1792382d5165b401f9ba6d0R536 |
|
So, playing with it, I see we test that the case split command happens, but not that a code action is generated. It should probably be tested together with the other code actions in https://github.com/haskell/haskell-ide-engine/blob/master/test/functional/FunctionalCodeActionsSpec.hs#L226 |
Move
splitCaseCmdfrom the GhcMod plugin to the HaRe plugin in order to avoid a circular dependency.Move the
casesplittests to HaRe tests file.Add a
casesplitCodeAction test.I have created an associated pull request to avoid breaking vscode-hie-server use of
ghcmod:casesplit.closes: #907