Multi-ide structure change handling #19194
Conversation
samuel-williams-da
force-pushed
the
sw/multi-ide-structure-handling-improvements
branch
from
52b5286
to
8e474d3
Compare
| getSourceFileHome miState path = do | ||
| -- If the path is a file, we only care about the directory, as all files in the same directory share the same home | ||
| dirPath <- unsafeIOToSTM $ getDirectoryIfFile path | ||
| sourceFileHomes <- takeTMVar (sourceFileHomesVar miState) |
There was a problem hiding this comment.
Changed this cache to hold the directory name, rather than file name, to avoid many IO lookups when moving an IDE
| (LSP.IsClientEither, LSP.ReqMess (LSP.RequestMessage {_id})) -> replyError method _id | ||
| _ -> pure () | ||
| _ -> pure () | ||
| home <- atomically $ getSourceFileHome miState path |
There was a problem hiding this comment.
getSourceFileHome can no longer fail, so empty reply logic isn't needed
| getSourceFileHome miState path = do | ||
| -- If the path is a file, we only care about the directory, as all files in the same directory share the same home | ||
| dirPath <- unsafeIOToSTM $ getDirectoryIfFile path |
There was a problem hiding this comment.
Can we use a newtype to be sure that every path that we lookup goes through getDirectoryIfFile first? Or is that overkill?
There was a problem hiding this comment.
Given the key is only used in 2 places, I'd say its overkill. to do it properly you'd need to not export the constructor, and thus move the getDirectoryIfFile function to types, which doesn't feel like the right place for it
| newHomeIdeData' = newHomeIdeData {ideDataOpenFiles = Set.insert openFile $ ideDataOpenFiles newHomeIdeData} | ||
| -- If we're moving the file to a disabled IDE, it should get the new warning | ||
| when (ideDataDisabled newHomeIdeData') $ traverse_ (sendClientSTM miState) $ disableIdeDiagnosticMessages newHomeIdeData' | ||
| forM_ (ideDataMain newHomeIdeData) $ \ide -> do |
There was a problem hiding this comment.
Do we want to send these messages even if the ide is disabled?
There was a problem hiding this comment.
Yes, disabled essentially means disallowing the IDE to start. Usually, the IDE is killed before being disabled, so ideDataMain will be Nothing when ideDataDisabled is true, but we handle them separately here to be correct
|
LGTM :) |
samuel-williams-da
deleted the
sw/multi-ide-structure-handling-improvements
branch
* Better support for package structure changes * Rename some handlers
* Daml ide multi ide (#17345) * Multi-ide refactors (#18885) * Reimplement ProgressToken prefixing to be stateless * Refactor parsing logic to not use interleaveIO * Move most verbose logging behind a flag * Address reviews * Multi-IDE Features for 2.9 (#19040) * Implement dar unpacking * Implement packageless IDE * Hot-reloading logic for daml.yaml, multi-package.yaml, *.dar * Implement initial error recovery logic * Switch logging to log levels Replace window reload with LanguageServer restart * Forward args from multi-ide to sub-ides * Change unpacked dar paths to be the unit-id. Update unpacking logic to shutdown previous IDEs * Remove broken experimental flag * Refactor ide restart logic to not lose event handlers * Log subIDE errors to debug logger live * Windows fixes * First review fixes batch * Use newtypes for many FilePaths * Address Dylan's comments * Refactor how SubIDEs are passed around, reduce times it is dropped * Update diagnostic * Multi-ide structure change handling (#19194) * Better support for package structure changes * Rename some handlers * Split up MultiIde.hs into many files (#19198) * Split up MultiIde.hs into many files * Further split SubIde * Multi-IDE Sdk install managament (#19256) * Tweaks from previous version * Implement Sdk Version management * Address review * Fix compilation issues * Fix issues with 2.x
Fixes #19191 and #19192