Auto detect added dependencies

[ChrisPenner]

Currently ghcide doesn't notice when I update either package.yaml or *.cabal; so I need to restart it, up till now I've been closing and re-opening my editor, but now I just run killall ghcide; it picks up on the changes when it restarts.

It'd be nice if it could detect changes to those files and restart itself automagically.

Thanks for all the work you do!

[ndmitchell]

This is super easy to add on the ghcide side once we get the necessary information out of hie-bios. I raised a ticket for that: haskell/hie-bios#35

[domenkozar]

Trickier part will be to report when cradle can't build as per #710

[domenkozar]

@ndmitchell is there some kind of watching api for file changes in ghcide or did you plan to use notify?

[ndmitchell]

@domenkozar there is an LSP API for file watching which we should use.

[sofusmortensen]

bump

(this would be really helpful)

[jinwoo]

Now that hie-bios provides the information about cradle dependencies (haskell/hie-bios#35), can someone implement this? Some editor plugins can stop/restart the LSP server but others don't, and it's really annoying to quit and restarting the editor whenever this happens.

[ndmitchell]

Should be easy to implement when someone has the time. Patch welcome, but otherwise it's waiting for someone to have this at the top of their queue.

I also requested restarting in #979, I agree it would be super useful.

[jinwoo]

Agreed that #979 would be quite useful as an escape hatch.

Can you give me some hints on how this can be implemented? I have a vague idea that the file watching should be used (either using the LSP API if the client supports it, or ghcide's own support otherwise), and then the language server must be restarted somehow if a file being watched is updated or created. But I'm not quite sure how/where it should be hooked into existing code.

[ndmitchell]

I think the approach would involve the code at https://github.com/digital-asset/ghcide/blob/master/exe/Main.hs#L204-L207. You get a cradle back, then in the Action monad you need to do something like need files_it_used to add the dependency on them.

[jinwoo]

Thanks, Neil! I'll give it a shot.

[jinwoo]

@ndmitchell , when you said need, did you mean the needOnDisk in https://github.com/digital-asset/ghcide/blob/master/src/Development/IDE/Core/Shake.hs#L652? And I need to call defineOnDisk (https://github.com/digital-asset/ghcide/blob/master/src/Development/IDE/Core/Shake.hs#L623) to define rules for watching those dependency files?

Or did you mean something else?

[ndmitchell]

I just mean the normal Development.Shake.need. It's possible that those are better because they get tracking through the file watching, but I'm not familiar enough to know if they'd work better.

[jinwoo]

Using need doesn't seem to work because when files like package.yaml or *.cabal change, getCompilerOption from hie-bios must be called again to retrieve the new compile options. But getCompilerOptions is called from cradleToSession, which is memoized from loadSession and we continue to use the old options/session.

So it seems we need a way to invalidate the memoization cache. Whenever any dependency files change, we invalidate the cache, and call getCompilerOption and create a new GHC session. Do you think I'm on the right track?

[ndmitchell]

Ah, good point - you probably need to switch from memoIO to newCacheIO from Shake, which works at the level of Action, and thus can have need called at the right place. See https://hackage.haskell.org/package/shake-0.18.5/docs/Development-Shake.html#v:newCacheIO

Unfortunately, there's an issue around newCacheIO which is why I went for memoIO in the first place ndmitchell/shake#725. Maybe it needs to be lifted into an oracle? I suspect these complexities were why I didn't do it last time I thought about it...

[jinwoo]

Hm.. Not only that. Now I realize that we need to restart the whole language server when files like *.cabal change because loadSession should be called again to have it use a new HscEnvEq that is created from the new GHC options.

Which means we need to add a way to stop the current language server and restart it. This is getting out of hand 😱

[ndmitchell]

No need to restart the language server - you just have to move the setting up of loadSession onto the Shake graph and it should replace the environment when things change. Not necessary straightforward though.

[jinwoo]

All right. Following what you suggested, I made it work barring a few problems. Let me clean it up and hopefully I can send a PR soon.

[jinwoo]

I think this can be closed now.

[ndmitchell]

Thanks for your work on this @jinwoo