Remove IdeResponse, integrate it into IdeM as FreeT IdeDefer #777
Conversation
Fix module cache signature
Co-authored-by: Luke Lau <luke_lau@icloud.com> Co-authored-by: Gurkenglas <gurkenglas@hotmail.de>
|
|
||
| -- | A computation that is deferred until the module is cached. | ||
| -- Note that the module may not typecheck, in which case 'UriCacheFailed' is passed | ||
| data IdeDefer a = IdeDefer FilePath (UriCache -> a) deriving Functor |
There was a problem hiding this comment.
This is probably not the right time to do it, but at some point we should let the module cache be more lenient, in the sense that if we get anything out, we call the function.
Here anything is ParsedSource, RenamedSource, TypeCheckedSource.
There was a problem hiding this comment.
I strongly agree with this! I think this would be a (hopefully) easy win to make more features available more often, and not just whenever the module typechecks. Mainly since a lot of features e.g. document symbols and goto definition don't depend on the type checked source, just the parsed source.
I can imagine some sort of tiered system where you can request a module at the level of compilation needed:
data Source = Parsed | Renamed | TypeChecked
withCachedModule :: Uri -> Source -> (CachedModule -> IdeM a) -> IdeM aThere was a problem hiding this comment.
I wrote this review 5 days ago, but didn't realise you had to submit it for you to see it...
src/Haskell/Ide/Engine/Dispatcher.hs
Outdated
| ideDispatcher :: forall void m. DispatcherEnv -> ErrorHandler -> CallbackHandler m -> TChan (IdeRequest m) -> IdeM void | ||
| ideDispatcher env errorHandler callbackHandler pin = forever $ do | ||
| ideDispatcher :: forall void m. TVar IdeState -> J.ClientCapabilities -> DispatcherEnv -> ErrorHandler -> CallbackHandler m -> TChan (IdeRequest m) -> IO void | ||
| ideDispatcher stateVar caps env errorHandler callbackHandler pin = flip runReaderT stateVar $ flip runReaderT caps $ forever $ do |
There was a problem hiding this comment.
Long lines again, difficult to review
|
Nothing obvious jumps out at me. |
Shouldn't be necessary anymore since its caught at the dispatcher level
|
Are we ok with this? Should I merge? @wz1000 any comment? |
|
@alanz Seems reasonable to me. |
There was a problem hiding this comment.
Afaik PluginsIdeMonads.hs:229 never triggers up to now. We should reflect this. For example, 229 could use error. I think a better solution will be rendered unnecessary if we proceed to replace FreeT with making threads wait.
|
@Gurkenglas 229? |
|
Inserted a link. I thought writing the review while that line is marked would attach it to there, but apparently that needs another button or something ahhhhh interfaces |
| -- --------------------------------------------------------------------- | ||
|
|
||
| type IdeGhcM = GM.GhcModT IdeM | ||
| type IdeGhcM = GM.GhcModT IdeBase |
There was a problem hiding this comment.
This definition should be grouped with the other three, just below.
lukel97
changed the title
|
@Gurkenglas @alanz Instead of throwing an error/having an invariant, how would you feel about making |
|
@bubba I do not have strong feelings either way, except I prefer simplicity in general. And we need to merge this thing some time. |
|
|
||
| instance ExceptionMonad IdeM where | ||
| FreeT m `gcatch` f = FreeT $ fmap (fmap (`gcatch` f)) m `gcatch` (runFreeT . f) | ||
| gmask f = FreeT (runFreeT $ gmask f) |
There was a problem hiding this comment.
@Gurkenglas are there any better ways of implementing this?
|
|
||
| -- | A computation that is deferred until the module is cached. | ||
| -- Note that the module may not typecheck, in which case 'UriCacheFailed' is passed | ||
| data IdeDefer a = IdeDefer FilePath (UriCache -> a) deriving Functor |
There was a problem hiding this comment.
I strongly agree with this! I think this would be a (hopefully) easy win to make more features available more often, and not just whenever the module typechecks. Mainly since a lot of features e.g. document symbols and goto definition don't depend on the type checked source, just the parsed source.
I can imagine some sort of tiered system where you can request a module at the level of compilation needed:
data Source = Parsed | Renamed | TypeChecked
withCachedModule :: Uri -> Source -> (CachedModule -> IdeM a) -> IdeM aThis reverts commit ef14368.
4 participants
This removes the whole
IdeResponse/pluginGetFileResponseheadache. UseIdeResultnow instead.It also removes the versions of
withCachedModulethat didn't take a default. Now they must provide a default fallback if there is no module (which most of them did anyway), removing theNoModuleAvailableerror code.The dispatcher now also handles all the logic for deferred responses now, whereas previously half of it was in
ModuleCache.hsas well.