-
-
Notifications
You must be signed in to change notification settings - Fork 407
Add cradle dependencies to session loading errors #3779
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
Conversation
Related to haskell/cabal#9171 which provides the initial terrible error message.
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.
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.
Remind me again why we aren't just doing structured errors from hie-bios? 😅
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.
No reason :) Ill put it on my long term list of things to do after the release is finished.
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.
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!
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.
Does that mean we don't want it?
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.
IDK, potentially good to have both?
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 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.
0121a07 to
41118c6
Compare
41118c6 to
9adea75
Compare
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.