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.

Sign up for GitHub

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

rm41339 wants to merge 1 commit into haskell:master

from rm41339:cabal-project-plugin

Draft

DRAFT: Cabal project plugin diagnostics #4615

rm41339 wants to merge 1 commit into haskell:master from rm41339:cabal-project-plugin

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

Bodigrim reviewed

View reviewed changes

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Orphans.hs Outdated Show resolved Hide resolved

@fendor fendor mentioned this pull request

Support cabal.project and cabal.project.local files JustusAdam/language-haskell#237

Open

@rm41339 rm41339 mentioned this pull request

Add NFData instances haskell/cabal#11097

Merged

3 tasks

@VeryMilkyJoe VeryMilkyJoe self-requested a review

July 24, 2025 15:59

@rm41339 rm41339 force-pushed the cabal-project-plugin branch from cc35958 to 5876ac7 Compare

July 28, 2025 13:15

VeryMilkyJoe requested changes

View reviewed changes

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Show resolved Hide resolved

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Outdated

let perr = NE.head errs

in Left $

Diagnostics.fatalParseErrorDiagnostic file

("Failed to parse cabal.project file: " <> T.pack (show perr))

Copy link

Collaborator

@VeryMilkyJoe VeryMilkyJoe Jul 31, 2025

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 •

edited

Loading

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?

plugins/hls-cabal-project-plugin/test/Main.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/test/Main.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/test/Utils.hs Outdated Show resolved Hide resolved

plugins/hls-cabal-project-plugin/test/Utils.hs Outdated Show resolved Hide resolved

test.cpp Outdated Show resolved Hide resolved

@fendor fendor mentioned this pull request

DRAFT: Completions for cabal.project files, hls-cabal-project-plugin #4680

Draft

@rm41339 rm41339 changed the title ~~(削除) DRAFT: Cabal project plugin (削除ここまで)~~ (追記) DRAFT: Cabal project plugin diagnostics (追記ここまで)

VeryMilkyJoe requested changes

View reviewed changes

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Outdated

Comment on lines 73 to 74

cacheDir :: String

cacheDir = "ghcide"

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.

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

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Outdated

(_warnings, Right fields) ->

Right fields

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

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.

Suggested change

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

-- Returns unique cache directory for given cabal.project file

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Outdated

Comment on lines 50 to 52

-- Extract fields from cabal.project file

readCabalProjectFields

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.

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

plugins/hls-cabal-project-plugin/src/Ide/Plugin/CabalProject/Parse.hs Outdated

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

@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.

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.

plugins/hls-cabal-project-plugin/test/Main.hs Outdated

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

]

-- ------------------------ ------------------------------------------------

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.

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 reviewed

View reviewed changes

.github/actions/setup-build/action.yml Show resolved Hide resolved

@fendor fendor requested a review from VeryMilkyJoe

August 30, 2025 10:13

fendor reviewed

View reviewed changes

cabal.project Outdated Show resolved Hide resolved

@fendor @rm41339


 Diagnostics for cabal-project-plugin

1a56c77

@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

4 participants

@rm41339 @Bodigrim @fendor @VeryMilkyJoe