Support for resolve for class-plugin lenses #3769
Conversation
joyfulmantis
changed the title
| codeLens = makeLens <$> mapMaybe getRangeWithSig targetSigs | ||
|
|
||
| pure $ InL codeLens | ||
| (InstanceBindLensResult (InstanceBindLens{lensRendered}), _) |
There was a problem hiding this comment.
It looks like we are doing the same thing that codeLens did, do we really need this duplication?
There was a problem hiding this comment.
All three methods: the code lens, the code lens resolve, and the command handler rely on the same rule. However, we extract different data from the rule. For the first code lens handler, we only return the range of each code lens and a unique ID that allows us to resolve it later. To resolve the lens we match the unique ID the client sends us to the text edit of the lens (this allows us to extract the title). Finally, for the command, we actually generate a correct textEdit with position mapping properly adjusted and use that.
There was a problem hiding this comment.
I should add that, the code lens method tends to be called with every file update (a lot). The resolve method gets called for each visible lens. Finally, the command only gets called when someone wants to apply a lens (compared to the other two methods practically never)
There was a problem hiding this comment.
My point is about the action, we are calling the same classplugin.GetInstanceBindLens action for different functions, not sure if we can omit some calls, but overall this is good enough now.
| getInstanceBindLensRule recorder = do | ||
| defineNoDiagnostics (cmapWithPrio LogShake recorder) $ \GetInstanceBindLens nfp -> runMaybeT $ do | ||
| tmr@(tmrRenamed -> (hs_tyclds -> tycls, _, _, _)) <- useMT TypeCheck nfp | ||
| (InstanceBindTypeSigsResult allBinds) <- useMT GetInstanceBindTypeSigs nfp |
There was a problem hiding this comment.
This is the only place this rule is called... so is there any reason not to just inline the logic here???
There was a problem hiding this comment.
It is used also for code actions, hence the need to create a seperate rule for lens specific stuff.
This PR only changes the lenses in the class plugin to rely on resolve. I'm unsure of the benefit of doing the same for the code actions. The resolve was implemented by moving the logic from the code lens method to a separate rule, and then relying on that for the code lens method, the code lens resolve method, and the command.