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

Teach HLS about different file extensions #2945

Merged
merged 7 commits into from
Jun 22, 2022

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Jun 9, 2022

This PR is the foundation for #2940 and as such, I'd be really happy if we could get a review here before ZuriHac :)
I am working on a minimal example Plugin, that should illustrate that stuff works by tomorrow.

The Idea is to teach PluginDescriptors about the file-extension.

This PR is currently untested and only cherry-picked from #2047, the example plugin will have to suffice as tests.

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

This looks good to me. Are you going to send a companion change to the VSCode Haskell extension to update the metadata?

@fendor
Copy link
Collaborator Author

fendor commented Jun 10, 2022

Yeah, the companion PR is here: haskell/vscode-haskell#618
FYI, currently it doesn't work, .cabal files are type-checked atm 👼 I am trying to fix it today, if I don't manage to, I will just tell everyone they should base their work off this PR while I fix it.

hls-plugin-api/src/Ide/Types.hs Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Types.hs Outdated Show resolved Hide resolved
@@ -6714,7 +6714,7 @@ unitTests recorder logger = do
] ++ Ghcide.descriptors (cmapWithPrio LogGhcIde recorder)

testIde recorder (IDE.testing (cmapWithPrio LogIDEMain recorder) logger){IDE.argsHlsPlugins = plugins} $ do
_ <- createDoc "haskell" "A.hs" "module A where"
_ <- createDoc "A.hs" "haskell" "module A where"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fendor fendor force-pushed the feature/cabal-file-support branch from cea8009 to 8506c27 Compare June 20, 2022 16:39
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.

I think this PR needs a bit of trimming so it just does what it says, otherwise things get a bit confusing.

ghcide/src/Development/IDE/LSP/HoverDefinition.hs Outdated Show resolved Hide resolved
ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated Show resolved Hide resolved
ghcide/src/Development/IDE/Plugin/HLS.hs Show resolved Hide resolved
exe/Plugins.hs Outdated Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
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.

Oops I missed a file. Thanks for all the Haddock!

hls-plugin-api/src/Ide/Types.hs Show resolved Hide resolved
@fendor fendor force-pushed the feature/cabal-file-support branch from 8506c27 to 9d2b79d Compare June 20, 2022 17:52
@fendor fendor force-pushed the feature/cabal-file-support branch 3 times, most recently from dc3d7c2 to 3b6b472 Compare June 21, 2022 09:23
fendor and others added 5 commits June 21, 2022 11:26
NotificationHandler now distinguishes between different file extensions
RequestHandler distinguishes between different file extensions
The hierarchy looks as follows:

            PluginMethod (pluginEnabled)
                          |
         -----------------------------------
         |                                 |
 PluginRequestMethod             PluginNotificationMethod
@fendor fendor force-pushed the feature/cabal-file-support branch from 3b6b472 to e7e60bc Compare June 21, 2022 09:26
@fendor fendor requested a review from michaelpj June 21, 2022 09:27
@fendor fendor force-pushed the feature/cabal-file-support branch from cfe643e to dd56166 Compare June 22, 2022 11:50
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.

LGTM!

@michaelpj michaelpj added the merge me Label to trigger pull request merge label Jun 22, 2022
@fendor
Copy link
Collaborator Author

fendor commented Jun 22, 2022

@michaelpj Thank you for your time and thorough reviews!

@mergify mergify bot merged commit 907a6e6 into haskell:master Jun 22, 2022
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
* Fix parameter switch-up

* Generalise file extension handling for plugins

NotificationHandler now distinguishes between different file extensions
RequestHandler distinguishes between different file extensions

* Introduce PluginMethod Typeclass hierarchy

The hierarchy looks as follows:

            PluginMethod (pluginEnabled)
                          |
         -----------------------------------
         |                                 |
 PluginRequestMethod             PluginNotificationMethod

* Add example plugin

* Improve documentation for plugins

* Simplify Plugin Handling code

Co-authored-by: Jana Chadt <[email protected]>
Co-authored-by: Jana Chadt <[email protected]>
Co-authored-by: Fendor <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants