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

Implement code action to add module to cabal file #3778

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

VeryMilkyJoe
Copy link
Collaborator

Related to #3695.
Closes #3595.

When a cradle error occurs that the current module is unknown, we now offer code actions for each possible field to add the module to the most likely cabal file.

This PR is split into three commits which group the changes into:

  • changes to ghcide;
  • a cabal file parser package which will be moved out of hls in the future;
  • and changes to the cabal-plugin.

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.
For each other-module or exposed-module field in a stanza, we check whether any source directory of the stanza contains the module. If this is the case, we construct a code action which adds the module to the cabal file, while adhering to the cabal file's previous formatting as best as possible.

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Lots going on here, just a couple of high-level comments. I'll try to review it more later.

@@ -0,0 +1,71 @@
cabal-version: 3.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd probably just make this an internal library of the cabal plugin for now. We can split it out later, but additional packages do mean we have to do more releases etc.

annotateSrcLoc a = do
start <- getSourcePos
parsed <- a
end <- getSourcePos
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this handle whitespace okay? I've had that problem with doing this before: since lexer-less parsing encourages you to consume whitespace as part of your syntactic productions, you end up with the source span including the whitespace.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually wonder whether this parser would be easier to write with a lexer pass. I think megaparsec supports that...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why cabal parser implemented by cabal would not work for this task?

https://github.com/haskell/cabal/tree/master/Cabal-syntax

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would work, given the existence https://github.com/Bodigrim/cabal-add/. However, this was deemed a somewhat interesting idea to add a cabal exactprinter, but it is likely not the best way forward.

@Bodigrim
Copy link
Contributor

I'm not sure that reimplementing Cabal parser is a sustainable approach.

https://github.com/Bodigrim/cabal-add/ inserts build-depends largely re-using Cabal-syntax, and probably inserting modules could be done in a similar fashion.

@fendor
Copy link
Collaborator

fendor commented Mar 11, 2024

@Bodigrim Yeah, the cabal-add approach feels more viable than the custom parser.

Independently, I believe we should have a third-party parser for cabal files.

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.

Feature Request: Add current file to Cabal (exposed/other) modules
5 participants