-
Notifications
You must be signed in to change notification settings - Fork 207
Load all possible haskell source files #1569
Conversation
isHaskellSource | ||
(\path -> any (\p -> p path) [isHidden, isSpecialDir]) | ||
absFp | ||
else filterM doesFileExist [absFp] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
silently discarding unknown filepaths does not seem pretty, but neither does throwing an exception.
-> FilePath | ||
-> IdeGhcM (IdeResult (Ghc.Diagnostics, Ghc.AdditionalErrs)) | ||
compileTarget dynFlags fp = do | ||
let pubDiags _ _ _ = return () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capturing this could make sense.
, projectRoot :: Maybe String | ||
, optBiosVerbose :: Bool | ||
, optCaptureFile :: Maybe FilePath | ||
, optExamplePlugin :: Bool | ||
, optDryRun :: Bool | ||
, optFiles :: [FilePath] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better name required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optSanityCheck
? optProjectLoadCheck
?
Can you list directories as well, in optFiles
, and have the recursive contents checked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. If it is a directory, it is recursed into it and if it is a FilePath, its existence is checked and filtered if it does not exist. Which is maybe not the desired behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following question remain for me:
Should --dry-run
be the default? So that it doesnt by default try to load every file in the project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's that dangerous to load every file in the project, it's not like its destructively editing anything. I'm happy if it loads them by default
Indicate that some options dont do anything in combination with the `--lsp` flag. We do this, because we cant express this invariant in the type-system, yet. A follow-up PR, refactoring the command line flags is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! As an aside, would it make sense to try and mechanically get the modules required for each component inside the cradle (module graphs, other-modules field, etc.), and load those? Searching for hs files is fine, but not all hs files are used in reality. This is just a thought for something to maybe look at later, I'm happy for this to go in
, projectRoot :: Maybe String | ||
, optBiosVerbose :: Bool | ||
, optCaptureFile :: Maybe FilePath | ||
, optExamplePlugin :: Bool | ||
, optDryRun :: Bool | ||
, optFiles :: [FilePath] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's that dangerous to load every file in the project, it's not like its destructively editing anything. I'm happy if it loads them by default
I think so, too, but the API doesnt really expose it at the moment, as far as I can tell. |
Extends PR #1538
Whole log run (this is huge)
Example Output in short:
TODO:
hie-test-utils
Grouping of modules into components.Will not make sense if a module is not correctly matched to a component and has a doubtful use.To discuss:
Do we want colour? (possible with Windows?)Maybe follow-up PR.Extra exposed module ok, or should everything be inline in
MainHie.hs
?Adds: