Skip to content

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
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 52
-- 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 🙃

@rm41339 rm41339 force-pushed the cabal-project-plugin branch 2 times, most recently from 144aeb3 to c33a7f1 Compare August 29, 2025 21:35
@fendor fendor requested a review from VeryMilkyJoe August 30, 2025 10:13
@rm41339 rm41339 force-pushed the cabal-project-plugin branch from 4f13a72 to 1a56c77 Compare August 30, 2025 13:09
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