Skip to content

Add support for external Ormolu #3771

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Sep 14, 2023

Conversation

sir4ur0n
Copy link
Collaborator

@sir4ur0n sir4ur0n commented Aug 22, 2023

Related to #411

This PR adds support for a new LSP flag haskell.plugin.ormolu.config.external, boolean, default false. If set to true, it uses the ormolu binary from the path rather than the built-in one.
This is similar to haskell.plugin.fourmolu.config.external.

This PR takes inspiration from:

This PR also documents the flag haskell.plugin.fourmolu.config.external.

Example HLS debug logs when formatting:

2023-08-22T11:48:55.091644Z | Debug | ormolu: Using external ormolu-0.5.3.0
2023-08-22T11:48:55.091712Z | Debug | ormolu: Running: `ormolu -o-XMonomorphismRestriction -o-XDeepSubsumption -o-XRelaxedPolyRec -o-XForeignFunctionInterface -o-XImplicitPrelude -o-XOverloadedStrings -o-XDisambiguateRecordFields -o-XRecordWildCards -o-XNamedFieldPuns -o-XDoAndIfThenElse -o-XEmptyDataDecls -o-XPatternGuards -o-XDatatypeContexts -o-XTraditionalRecordSyntax -o-XLambdaCase -o-XStarIsType -o-XCUSKs -o-XFieldSelectors --stdin-input-file /home/sir4ur0n/code/haskell-language-server/plugins/hls-alternate-number-format-plugin/src/Ide/Plugin/AlternateNumberFormat.hs` in directory /home/sir4ur0n/code/haskell-language-server/plugins/hls-alternate-number-format-plugin/src/Ide/Plugin

I will try to open PRs to the various Haskell LSP Clients to also support those 2 flags (Ormolu and Fourmolu) once this PR is merged.

I see that the CI fails, but it looks like it's unrelated to my PR. Could it be CI flakiness? CI is green now.

sir4ur0n pushed a commit to sir4ur0n/lsp-haskell that referenced this pull request Aug 22, 2023
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.

Thanks for doing this! Looks fine to me, I'll leave it to @georgefst to review the code in more detail :)

sir4ur0n pushed a commit to sir4ur0n/lsp-haskell that referenced this pull request Aug 23, 2023
@sir4ur0n
Copy link
Collaborator Author

@georgefst do you know if you will be able to review this PR soon please? 🙏

@sir4ur0n sir4ur0n force-pushed the support-external-ormolu branch from 004fe72 to 3d08ccf Compare August 31, 2023 09:53
@michaelpj
Copy link
Collaborator

I'll try to look at the code in more detail soon otherwise.

@sir4ur0n
Copy link
Collaborator Author

Is there anything I can do to move this PR forward? 😅

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.

Looks fine. The fourmolu plugin got refactored a little recently, so if you like you could mirror that here to keep them as close together as possible. Optional, though.

@sir4ur0n
Copy link
Collaborator Author

sir4ur0n commented Sep 13, 2023

The fourmolu plugin got refactored a little recently, so if you like you could mirror that here to keep them as close together as possible.

I just tried that, but because of recent commits, I am no longer able to compile the HLS project:

$ nix develop
[sir4ur0n@nixos-sir4ur0n:~/code/haskell-language-server]$ cabal build exe:haskell-language-server
Warning: The package list for 'hackage.haskell.org' is 25 days old.
Run 'cabal update' to get the latest list of available packages.
Warning: Requested index-state 2023-09-08T00:00:00Z is newer than
'hackage.haskell.org'! Falling back to older state (2023-08-18T10:16:06Z).
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
[__0] trying: ghcide-2.2.0.0 (user goal)
[__1] next goal: lsp-types (dependency of ghcide)
[__1] rejecting: lsp-types-2.0.1.1 (conflict: ghcide => lsp-types^>=2.0.2.0)
[__1] skipping: lsp-types-2.0.1.0, lsp-types-2.0.0.1, lsp-types-2.0.0.0,
lsp-types-1.6.0.0, lsp-types-1.5.0.0, lsp-types-1.4.0.1, lsp-types-1.4.0.0,
lsp-types-1.3.0.1, lsp-types-1.2.0.0, lsp-types-1.1.0.0, lsp-types-1.0.0.1,
lsp-types-1.0.0.0, lsp-types-1.3.0.0 (has the same characteristics that caused
the previous version to fail: excluded by constraint '^>=2.0.2.0' from
'ghcide')
[__1] fail (backjumping, conflict set: ghcide, lsp-types)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: ghcide, lsp-types

$ nix develop .\#haskell-language-server-94-dev-nix
[sir4ur0n@nixos-sir4ur0n:~/code/haskell-language-server]$ cabal build exe:haskell-language-server
Warning: The package list for 'hackage.haskell.org' is 25 days old.
Run 'cabal update' to get the latest list of available packages.
Warning: Requested index-state 2023-09-08T00:00:00Z is newer than
'hackage.haskell.org'! Falling back to older state (2023-08-18T10:16:06Z).
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
[__0] trying: hls-fourmolu-plugin-2.2.0.0 (user goal)
[__1] next goal: fourmolu (dependency of hls-fourmolu-plugin)
[__1] rejecting: fourmolu-0.11.0.0/installed-DAGoielLAG0DNoCqRf49OQ (conflict:
hls-fourmolu-plugin => fourmolu^>=0.14)
[__1] skipping: fourmolu-0.13.1.0, fourmolu-0.13.0.0, fourmolu-0.12.0.0,
fourmolu-0.11.0.0, fourmolu-0.10.1.0, fourmolu-0.10.0.0, fourmolu-0.9.0.0,
fourmolu-0.8.2.0, fourmolu-0.8.1.0, fourmolu-0.8.0.0, fourmolu-0.7.0.1,
fourmolu-0.7.0.0, fourmolu-0.6.0.0, fourmolu-0.5.0.1, fourmolu-0.5.0.0,
fourmolu-0.4.0.0, fourmolu-0.3.0.0, fourmolu-0.2.0.0, fourmolu-0.1.0.0,
fourmolu-0.0.6.0 (has the same characteristics that caused the previous
version to fail: excluded by constraint '^>=0.14' from 'hls-fourmolu-plugin')
[__1] fail (backjumping, conflict set: fourmolu, hls-fourmolu-plugin)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: hls-fourmolu-plugin, fourmolu

If you have any tip to support developing HLS on Nix again, I will gladly proceed, but otherwise, this is the end for me 🤷

@michaelpj
Copy link
Collaborator

cabal is failing because it can't find the new versions of packages that have been recently released, you just need to cabal update.

@sir4ur0n
Copy link
Collaborator Author

This goes further but still fails:

$ 
[...]
Failed to build zlib-0.6.3.0. The failure occurred during the configure step.
Build log (
/home/sir4ur0n/.cabal/logs/ghc-9.4.6/zlib-0.6.3.0-e267166a4eefc365d471ea50cf722432ba5b3b4870d706e41bc123b513fc7aec.log
):
Configuring library for zlib-0.6.3.0..
Error: .cabal-wrapped: Missing dependency on a foreign library:
* Missing (or bad) header file: zlib.h
* Missing (or bad) C library: z
This problem can usually be solved by installing the system package that
provides this library (you may need the "-dev" version). If the library is
already installed but in a non-standard location then you can use the flags
--extra-include-dirs= and --extra-lib-dirs= to specify where it is.If the
library file does exist, it may contain errors that are caught by the C
compiler at the preprocessing stage. In this case you can re-run configure
with the verbosity flag -v3 to see the error messages.
If the header file does exist, it may contain errors that are caught by the C
compiler at the preprocessing stage. In this case you can re-run configure
with the verbosity flag -v3 to see the error messages.

Error: cabal: Failed to build zlib-0.6.3.0 (which is required by
exe:haskell-language-server from haskell-language-server-2.2.0.0). See the
build log above for details.

I really don't want to go down the path of installing more and more things globally on my system 😐 I stopped doing that in the past precisely because it made me unable to work on several projects in parallel.

@michaelpj
Copy link
Collaborator

That's strange, zlib is in the devshell already, so that should work. I do agree that the Nix build shell should work. I use it daily (on NixOS), so I'm surprised it's not working for you.

@sir4ur0n
Copy link
Collaborator Author

@michaelpj would you be ok with merging the PR as is? The re-alignment can be done in a follow-up PR

@michaelpj michaelpj merged commit 5241101 into haskell:master Sep 14, 2023
@michaelpj
Copy link
Collaborator

Sure

@sir4ur0n sir4ur0n deleted the support-external-ormolu branch September 14, 2023 12:55
sir4ur0n pushed a commit to sir4ur0n/lsp-haskell that referenced this pull request Oct 11, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants