Skip to content

DRAFT: Cabal project plugin diagnostics #4615

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rm41339
Copy link
Contributor

@rm41339 rm41339 commented Jun 6, 2025

Draft of cabal.project plugin for GSoC 2025

Still in progress!

@rm41339 rm41339 requested review from fendor and michaelpj as code owners June 6, 2025 21:43
@fendor fendor marked this pull request as draft June 9, 2025 12:15
@rm41339 rm41339 force-pushed the cabal-project-plugin branch from cc35958 to 5876ac7 Compare July 28, 2025 13:15
let perr = NE.head errs
in Left $
Diagnostics.fatalParseErrorDiagnostic file
("Failed to parse cabal.project file: " <> T.pack (show perr))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a nicely readable view for the user?
Also, you are only showing the first error in the list, we also want to know about the other ones.

Copy link
Collaborator

@VeryMilkyJoe VeryMilkyJoe Aug 10, 2025

Choose a reason for hiding this comment

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

@rm41339 i dont think this is resolved?

@rm41339 rm41339 changed the title DRAFT: Cabal project plugin DRAFT: Cabal project plugin diagnostics Aug 3, 2025
Comment on lines +73 to +74
cacheDir :: String
cacheDir = "ghcide"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should expose this constant from haskell-language-server/ghcide/session-loader/Development/IDE/Session.hs instead of hard-coding it here.

(_warnings, Right fields) ->
Right fields

-- Helper for parseCabalProjectFileContents, returns unique cache directory for given cabal.project file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- Helper for parseCabalProjectFileContents, returns unique cache directory for given cabal.project file
-- Returns unique cache directory for given cabal.project file

Comment on lines +50 to +51
-- Extract fields from cabal.project file
readCabalProjectFields
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use haddock comments, otherwise the documentation won't show up.

Comment on lines +57 to +59
-- we don't want to double report diagnostics, all diagnostics are produced by 'parseCabalProjectFileContents'.
(_warnings, Left (_mbVer, errs)) ->
Left (map (Diagnostics.errorDiagnostic file) (NE.toList errs))
Copy link
Collaborator

Choose a reason for hiding this comment

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

After some thinking, we should report these warning diagnostics after all, the same will go for the cabal-plugin.
Since we ignore the file diagnostics in the rule (ParseCabalProjectFields) where this is called already, I would return warning diagnostics as well as error diagnostics in here for consistency.

False @? "Expected parse to fail (missing import), but it succeeded"
]

-- ------------------------ ------------------------------------------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it was like that, from where you copied it but please remove the space 🙃

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.

4 participants