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

Support fourmolu ^>= 0.7 #2944

Merged
merged 4 commits into from
Jun 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion plugins/hls-fourmolu-plugin/hls-fourmolu-plugin.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,27 @@ license: Apache-2.0
license-file: LICENSE
author: The Haskell IDE Team
copyright: The Haskell IDE Team
homepage: https://github.com/haskell/haskell-language-server
bug-reports: https://github.com/haskell/haskell-language-server/issues
Comment on lines +12 to +13
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It took me a minute to figure out the Git repository for this package, so I added these fields to the .cabal file. Technically an unrelated change, would be easy to put somewhere else.

maintainer: alan.zimm@gmail.com
category: Development
build-type: Simple
extra-source-files:
LICENSE
test/testdata/**/*.hs

source-repository head
type: git
location: git://github.com/haskell/haskell-language-server.git

library
exposed-modules: Ide.Plugin.Fourmolu
hs-source-dirs: src
ghc-options: -Wall
build-depends:
, base >=4.12 && <5
, filepath
, fourmolu ^>=0.3 || ^>=0.4 || ^>= 0.6
, fourmolu ^>=0.3 || ^>=0.4 || ^>= 0.6 || ^>= 0.7
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is in the wrong commit, right? Not necessarily actually worth changing now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am regrettably a complete git noob and generally just squash+merge all my stuff 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ha, I'm very much in the "git is a wonderful concept with a horrible CLI" camp. Since I finally got around to setting up good editor integration, I haven't looked back.

, ghc
, ghc-boot-th
, ghcide ^>=1.7
Expand Down
47 changes: 39 additions & 8 deletions plugins/hls-fourmolu-plugin/src/Ide/Plugin/Fourmolu.hs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
{-# LANGUAGE CPP #-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used to provide backwards compatibility.

{-# LANGUAGE DataKinds #-}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This extension is reshuffled because I ran stylish-haskell (which appears to be how this file is formatted) and it automatically did that change.

Same for the reordered imports below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So stylish-haskell isn't enforced by CI? If not, I'd be tempted to go back to fourmolu.

This file was formatted with stylish-haskell when it looked like that was becoming necessary (#693, #1439). But I lost track of where the decision on formatting this codebase ended up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a pre-commit hook that runs stylish-haskell, mentioned in the contributing guidelines. But it is not enforced by CI:

### Formatter pre-commit hook
We are using [pre-commit-hook.nix](https://github.com/cachix/pre-commit-hooks.nix) to configure git pre-commit hook for formatting. Although it is possible to run formatting manually, we recommend you to use it to set pre-commit hook as our CI checks pre-commit hook is applied or not.
You can configure the pre-commit-hook by running
``` bash
nix-shell
```
If you don't want to use [nix](https://nixos.org/guides/install-nix.html), you can instead use [pre-commit](https://pre-commit.com) with the following config.
```json
{
"repos": [
{
"hooks": [
{
"entry": "stylish-haskell --inplace",
"exclude": "(^Setup.hs$|test/testdata/.*$|test/data/.*$|test/manual/lhs/.*$|^hie-compat/.*$|^plugins/hls-tactics-plugin/.*$|^ghcide/src/Development/IDE/GHC/Compat.hs$|^ghcide/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs$|^ghcide/src/Development/IDE/GHC/Compat/Core.hs$|^ghcide/src/Development/IDE/Spans/Pragmas.hs$|^ghcide/src/Development/IDE/LSP/Outline.hs$|^plugins/hls-splice-plugin/src/Ide/Plugin/Splice.hs$|^ghcide/test/exe/Main.hs$|ghcide/src/Development/IDE/Core/Rules.hs|^hls-test-utils/src/Test/Hls/Util.hs$)",
"files": "\\.l?hs$",
"id": "stylish-haskell",
"language": "system",
"name": "stylish-haskell",
"pass_filenames": true,
"types": [
"file"
]
}
],
"repo": "local"
},
{
"repo": "https://github.com/pre-commit/pre-commit-hooks",
"rev": "v4.1.0",
"hooks": [
{
"id": "mixed-line-ending",
"args": ["--fix", "lf"],
"exclude": "test/testdata/.*CRLF*.hs$"
}
]
}
]

{-# LANGUAGE DisambiguateRecordFields #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedLabels #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE TypeApplications #-}
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE OverloadedLabels #-}

module Ide.Plugin.Fourmolu (
descriptor,
Expand All @@ -23,16 +24,18 @@ import Development.IDE.GHC.Compat as Compat hiding (Cpp)
import qualified Development.IDE.GHC.Compat.Util as S
import GHC.LanguageExtensions.Type (Extension (Cpp))
import Ide.Plugin.Properties
import Ide.PluginUtils (makeDiffTextEdit, usePropertyLsp)
import Ide.PluginUtils (makeDiffTextEdit,
usePropertyLsp)
import Ide.Types
import Language.LSP.Server hiding (defaultConfig)
import Language.LSP.Types
import Language.LSP.Types.Lens (HasTabSize (tabSize))
import Ormolu
import Ormolu.Config
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is redundant for fourmolu <= 0.7. I didn't gate it behind CPP because CPP is ugly.

import System.Exit
import System.FilePath
import System.IO (stderr)
import System.Process.Run (proc, cwd)
import System.Process.Run (cwd, proc)
import System.Process.Text (readCreateProcessWithExitCode)

descriptor :: PluginId -> PluginDescriptor IdeState
Expand Down Expand Up @@ -78,10 +81,17 @@ provider plId ideState typ contents fp fo = withIndefiniteProgress title Cancell
ExitFailure n ->
pure . Left . responseError $ "Fourmolu failed with exit code " <> T.pack (show n)
else do
let format printerOpts =
let format fourmoluConfig =
first (mkError . show)
<$> try @OrmoluException (makeDiffTextEdit contents <$> ormolu config fp' (T.unpack contents))
where
printerOpts =
#if MIN_VERSION_fourmolu(0,7,0)
cfgFilePrinterOpts fourmoluConfig
#else
fourmoluConfig

#endif
config =
defaultConfig
{ cfgDynOptions = map DynOption fileOpts
Expand All @@ -91,6 +101,10 @@ provider plId ideState typ contents fp fo = withIndefiniteProgress title Cancell
fillMissingPrinterOpts
(printerOpts <> lspPrinterOpts)
defaultPrinterOpts
#if MIN_VERSION_fourmolu(0,7,0)
, cfgFixityOverrides =
cfgFileFixities fourmoluConfig
#endif
}
in liftIO (loadConfigFile fp') >>= \case
ConfigLoaded file opts -> liftIO $ do
Expand All @@ -101,16 +115,33 @@ provider plId ideState typ contents fp fo = withIndefiniteProgress title Cancell
. unlines
$ ("No " ++ show configFileName ++ " found in any of:") :
map (" " ++) searchDirs
format mempty
ConfigParseError f (_, err) -> do
format emptyOptions
where
emptyOptions =
#if MIN_VERSION_fourmolu(0,7,0)
FourmoluConfig
{ cfgFilePrinterOpts = mempty
, cfgFileFixities = mempty
}
#else
mempty
#endif

ConfigParseError f err -> do
sendNotification SWindowShowMessage $
ShowMessageParams
{ _xtype = MtError
, _message = errorMessage
}
return . Left $ responseError errorMessage
where
errorMessage = "Failed to load " <> T.pack f <> ": " <> T.pack err
errorMessage = "Failed to load " <> T.pack f <> ": " <> T.pack (convertErr err)
convertErr =
#if MIN_VERSION_fourmolu(0,7,0)
show
#else
snd
#endif
where
fp' = fromNormalizedFilePath fp
title = "Formatting " <> T.pack (takeFileName fp')
Expand Down