-
-
Notifications
You must be signed in to change notification settings - Fork 410
cabal-add integration as a CodeAction #4360
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
Conversation
Bodigrim
left a comment
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.
Thanks for working on this!
|
Have made some significant changes to the solution.
|
| addDependency :: FilePath -> Maybe String -> NonEmpty String -> IO () | ||
| addDependency cabalFilePath buildTarget dependency = do | ||
|
|
||
| cnfOrigContents <- readCabalFile cabalFilePath |
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 don't know if HLS does it in other cases, but it could be a good idea to lock the handle using https://hackage.haskell.org/package/lukko-0.1.2/docs/Lukko.html#v:hLock.
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.
Or without lukko, open file for read/write access and manipulate it without closing?.. Or am I talking nonsense?..
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.
Instead of manually reading and parsing, use https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs#L260, which hooks into the rule system.
E.g.
liftIO $ runAction "cabal-plugin.cabal-add" ide $ useWithStale ParseCabalFile $ toNormalizedFilePath $ toNormalizedFilePath cabalFilePathThis will give you the GPD already parsed and cached.
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.
Or if you need the fields, the original link will give you that
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.
Since the handle using approach is not very in the style of hls I used runAction approach instead. It falls back on reading files, if the data wasn't found in rules.
35c38e5 to
883e1fd
Compare
|
Have made a lot of changes to the solution:
|
VeryMilkyJoe
left a comment
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.
Thank you for this awesome feature, I just have a few nitpicks and will check it out and manually test a little bit :)
fendor
left a comment
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.
Very nice, just a couple of changes, then this is fine by me :)
fendor
left a comment
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.
LGTM, a couple nitpicks left 😇
Co-authored-by: fendor <[email protected]>
e123f49 to
385d83b
Compare
ee55bc1 to
1a4c37c
Compare
1a4c37c to
e8c6051
Compare
|
I built HLS from sources, but apparently something is off in my configuration, because when clicking on "Add dependency ..." I keep getting log messages like Do I need to enable something specific in my HLS config? I put there but seemingly I didn't get plugin name right. |
|
The config shouldn't need any modification as it is disabled by default. I also get errors such: But it works otherwise fine on my machine after building from source. Could you post the full logs? |
If HLS detects a message like "module `Bla.Bla.Bla` is a member of a hidden package `bla-1.2.3`" it suggests a quick fix, that finds the closest cabal file and adds the dependency there. Solution uses [`Distribution.Client.Add`](https://hackage.haskell.org/package/cabal-add-0.1/candidate/docs/Distribution-Client-Add.html) from the `cabal-add` and automaticly adds version requirement, if it's detected. For now, the `cabal-add` project was linked using [remote package specification](https://cabal.readthedocs.io/en/3.4/cabal-project.html#specifying-packages-from-remote-version-control-locations). Some parts were heavily inspired by the `cabal-add` code in the main module and might be rewritten later. `CodeAction` works by parsing all haskell diagnostics, and is constructed if a suited message was found. Parsed information is passed down to a new command, which itself uses tools provided by `cabal-add`. The command conducts IO actions with found cabal file.
If HLS detects a message like "module `Bla.Bla.Bla` is a member of a hidden package `bla-1.2.3`" it suggests a quick fix, that finds the closest cabal file and adds the dependency there. Solution uses [`Distribution.Client.Add`](https://hackage.haskell.org/package/cabal-add-0.1/candidate/docs/Distribution-Client-Add.html) from the `cabal-add` and automaticly adds version requirement, if it's detected. For now, the `cabal-add` project was linked using [remote package specification](https://cabal.readthedocs.io/en/3.4/cabal-project.html#specifying-packages-from-remote-version-control-locations). Some parts were heavily inspired by the `cabal-add` code in the main module and might be rewritten later. `CodeAction` works by parsing all haskell diagnostics, and is constructed if a suited message was found. Parsed information is passed down to a new command, which itself uses tools provided by `cabal-add`. The command conducts IO actions with found cabal file.
This is a proof-of-concept solution for the issue #3853
If HLS detects a message like "module
Bla.Bla.Blais a member of a hidden packagebla-1.2.3" it suggests a quick fix, that finds the closest cabal file and adds the dependency there.Solution uses
Distribution.Client.Addfrom thecabal-addand automaticly adds version requirement, if it's detected.CC @Bodigrim
Video with an example:
cabalAddCodeAction.mp4
Implementation details
For now, the
cabal-addproject was linked using remote package specification. Some parts were heavily inspired by thecabal-addcode in the main module and might be rewritten later.CodeActionworks by parsing all haskell diagnostics, and is constructed if a suited message was found. Parsed information is passed down to a new command, which itself uses tools provided bycabal-add. The command conducts IO actions with found cabal file.What isn't impemented