Skip to content

Add option to enable/disable .cabal file support #1223

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

Merged
merged 2 commits into from
Apr 13, 2025

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Apr 5, 2025

Support for .cabal files was implemented in HLS 2.0.0.0. But the vscode-haskell extension does not enable this Language Server support, because it would break for older HLS binaries, as these were unable to handle file updates for .cabal files.
To work-around, we had a pre-release of the vscode-haskell plugin, which enabled support for .cabal plugins in an opt-in fashion.

It's been a while, and we want to enable language server support for .cabal files unconditionally. There are still HLS binaries around that can't handle .cabal files, but it much less.
For the unfortunate souls left behind with GHC versions such as 8.6.5, we add a new vscode-haskell option:

haskell.supportCabalFiles

This option is true by default, but if set to false it will unconditionally disable sending File Notifications for .cabal files, which allows us to keep working for older HLS binaries.

This change would be difficult to test, and incur an, in my opinion, unreasonable price on our CI times.
I manually tested the changes, by using HLS 1.8.0.0 (last released binary for GHC 8.6.5) on a project with haskell.supportCabalFiles set to true and false respectively.
HLS seemed to behave correctly, showing Diagnostics for Haskell files, but not show any logs for .cabal files.
The only potential change in behaviour is that vscode-haskell now also acitvates when a .cabal file has been opened. This may be overly eager to some, but since no Haskell files are typechecked, the impact on memory usage is negligible in my opinion.

@fendor fendor requested review from hasufell, michaelpj and Ailrun April 5, 2025 13:57
@fendor
Copy link
Collaborator Author

fendor commented Apr 5, 2025

@hasufell This is just an FYI for you that we are going to enable Language Server support for .cabal files by default, but there is an option to disable it: "haskell.supportCabalFiles": false

@fendor fendor force-pushed the enhance/cabal-file branch from de429ac to 3c0421c Compare April 5, 2025 13:59
Support for `.cabal` files was implemented in HLS 2.0.0.0.
But the vscode-haskell extension does not enable this Language Server
support, because it would break for older HLS binaries, as these were
unable to handle file updates for `.cabal` files.
To work-around, we had a pre-release of the vscode-haskell plugin, which
enabled support for `.cabal` plugins in an opt-in fashion.

It's been a while, and we want to enable language server support for
`.cabal` files unconditionally. There are still HLS binaries around that
can't handle `.cabal` files, but it much less.
For the unfortunate souls left behind with GHC versions such as 8.6.5,
we add a new vscode-haskell option:

    haskell.supportCabalFiles

This option is `true` by default, but if set to `false` it will
unconditionally **disable** sending File Notifications for `.cabal`
files, which allows us to keep working for older HLS binaries.

This change would be difficult to test, and incur an, in my opinion,
unreasonable price on our CI times.
I manually tested the changes, by using HLS 1.8.0.0 (last released
binary for GHC 8.6.5) on a project with `haskell.supportCabalFiles` set
to `true` and `false` respectively.
HLS seemed to behave correctly, showing Diagnostics for Haskell files,
but not show any logs for `.cabal` files.
The only potential change in behaviour is that vscode-haskell now also
activates when a `.cabal` file has been opened. This may be overly eager to
some, but since no Haskell files are typechecked, the impact on memory
usage is negligible in my opinion.
@fendor fendor force-pushed the enhance/cabal-file branch from 3c0421c to a5605c6 Compare April 5, 2025 15:02
@hasufell
Copy link
Member

hasufell commented Apr 6, 2025

Can't the extension detect whether the server supports this? LSP can give that information afaik.

@fendor
Copy link
Collaborator Author

fendor commented Apr 6, 2025

This happens before the LSP client is started. We create the LSP client, and we need to specify for which file types we want to receive didChange notifications.
AFAICT, the LSP protocol doesn't have built-in support for changing the set of supported languages, since we arguably should have a separate Language Server for each "language", i.e. one for .cabal and one for .hs files.

We could probably hack something together using custom requests and notifications, but it would not solve the issue, as we would still have to wait a very long time before all older HLS versions that do not come with this hack are phased out.

Another possible workaround is to base the decision of the language support on the HLS binary version, as we know when the hls-cabal-plugin was added and which HLS version can actually handle .cabal files gracefully.
While this would work, this is only relevant for users of HLS 1.8.0.0 (last version supporting GHC 8.6.5). I started on implementing this check, but got carried away in refactorings, so this workaround is still in the making and will take more time.

@hasufell
Copy link
Member

hasufell commented Apr 6, 2025

Then can't we simply do a check against hls --numeric-version and have 3 values for the option:

  • enable
  • disable
  • automatic (default)

Add option to automatically determine whether the Haskell Language
Server binary we are about to launch supports '.cabal' files.
If not, i.e. the HLS binary is older than '1.9.0.0', then we do not send
file notifications for '.cabal' files. Otherwise, we do send these
notifications.

The user can explicitly overwrite this option in order to avoid the
version check.
@fendor
Copy link
Collaborator Author

fendor commented Apr 6, 2025

Done. I feel like this is a bit overkill for the amount of users, but here we are now.

@fendor fendor merged commit aa294fc into haskell:master Apr 13, 2025
13 checks passed
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.

3 participants