Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cradle dependencies to session loading errors #3779

Merged
merged 6 commits into from
Sep 13, 2023

Conversation

VeryMilkyJoe
Copy link
Collaborator

Extracted part of #3778, should be merged first.

Improve error message and information in cradle error related to unknown module.

When a cradle error is found to be related to an unknown module, we edit the error by adding information (the module's path, the most likely cabal file to add the module to) such that the cabal plugin can offer code actions for adding the module to a cabal file.

@fendor
Copy link
Collaborator

fendor commented Aug 28, 2023

Related to haskell/cabal#9171 which provides the initial terrible error message.

@michaelpj
Copy link
Collaborator

We're packing extra information into the diagnostic, but we're using the diagnostic text for that. There are at least two other places we could put that: data, or relatedInformation. relatedInformation actually seems useful: even without the code action, indicating the location of the relevant cabal file is directly useful to the user.

We'll still have to parse it out later, but at least it'll be slightly more structured.

} deriving (Show)

-- | Attempt to parse a multi-cradle message
parseMultiCradleErr :: [String] -> Maybe MultiCradleErr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me again why we aren't just doing structured errors from hie-bios? 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason :) Ill put it on my long term list of things to do after the release is finished.

tests :: TestTree
tests = testGroup "behaviour on malformed projects" [
testCase "no test executed" $ True @?= True
tests = testGroup "behaviour on malformed projects"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much more sophisticated than what I was thinking (although I imagine we'll want it anyway). I was just thinking of a unit test for the string matching function!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean we don't want it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, potentially good to have both?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tests are there and we shouldn't have them forever, any way. Ideally cabal gives us better error messages and hie-bios doesn't demand the parsing. Should be good enough, imo.

@VeryMilkyJoe VeryMilkyJoe force-pushed the improve-unknown-module-error branch 3 times, most recently from 0121a07 to 41118c6 Compare August 29, 2023 08:36
@fendor fendor force-pushed the improve-unknown-module-error branch from 41118c6 to 9adea75 Compare August 31, 2023 19:06
@michaelpj michaelpj self-requested a review September 6, 2023 11:31
@fendor fendor merged commit 0e6a81b into haskell:master Sep 13, 2023
28 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants