Skip to content

Add cabal-gild as a cabal file formatter plugin #4101

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.

Sign up for GitHub

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 4 commits into from
Mar 11, 2024
Merged

Add cabal-gild as a cabal file formatter plugin #4101

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d1bcfe
Add cabal-gild as a cabal file formatter plugin
fendor Feb 28, 2024
ee4170c
Add support for multiple cabal formatters
fendor Mar 10, 2024
77a4409
Fix cabal formatter test flags
fendor Mar 10, 2024
83f522c
Merge branch 'master' into feature/cabal-gild-plugin
fendor Mar 11, 2024
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
78 changes: 41 additions & 37 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
- ubuntu-latest
- macOS-latest
- windows-latest
test:
test:
- true
- false
exclude:
Expand Down Expand Up @@ -112,139 +112,143 @@ jobs:

- if: matrix.test
name: Test hls-graph
run: cabal test hls-graph
run: cabal test hls-graph

- if: needs.pre_job.outputs.should_skip_ghcide != 'true' && matrix.test
name: Test ghcide
# run the tests without parallelism to avoid running out of memory
run: cabal test ghcide || cabal test ghcide
run: cabal test ghcide || cabal test ghcide

- if: matrix.test
name: Test hls-plugin-api
run: cabal test hls-plugin-api || cabal test hls-plugin-api
run: cabal test hls-plugin-api || cabal test hls-plugin-api

- if: matrix.test
name: Test func-test suite
env:
HLS_TEST_EXE: hls
HLS_WRAPPER_TEST_EXE: hls-wrapper
run: cabal test func-test || cabal test func-test
run: cabal test func-test || cabal test func-test

- if: matrix.test
name: Test wrapper-test suite
env:
HLS_TEST_EXE: hls
HLS_WRAPPER_TEST_EXE: hls-wrapper
run: cabal test wrapper-test
run: cabal test wrapper-test

- if: matrix.test
name: Test hls-refactor-plugin
run: cabal test hls-refactor-plugin-tests || cabal test hls-refactor-plugin-tests
run: cabal test hls-refactor-plugin-tests || cabal test hls-refactor-plugin-tests

- if: matrix.test
- if: matrix.test
name: Test hls-floskell-plugin
run: cabal test hls-floskell-plugin-tests || cabal test hls-floskell-plugin-tests
run: cabal test hls-floskell-plugin-tests || cabal test hls-floskell-plugin-tests

- if: matrix.test
name: Test hls-class-plugin
run: cabal test hls-class-plugin-tests || cabal test hls-class-plugin-tests
run: cabal test hls-class-plugin-tests || cabal test hls-class-plugin-tests

- if: matrix.test
name: Test hls-pragmas-plugin
run: cabal test hls-pragmas-plugin-tests || cabal test hls-pragmas-plugin-tests
run: cabal test hls-pragmas-plugin-tests || cabal test hls-pragmas-plugin-tests

- if: matrix.test
name: Test hls-eval-plugin
run: cabal test hls-eval-plugin-tests || cabal test hls-eval-plugin-tests
run: cabal test hls-eval-plugin-tests || cabal test hls-eval-plugin-tests

- if: matrix.test
name: Test hls-splice-plugin
run: cabal test hls-splice-plugin-tests || cabal test hls-splice-plugin-tests
run: cabal test hls-splice-plugin-tests || cabal test hls-splice-plugin-tests

- if: matrix.test && matrix.ghc != '9.2'
name: Test hls-stan-plugin
run: cabal test hls-stan-plugin-tests || cabal test hls-stan-plugin-tests
run: cabal test hls-stan-plugin-tests || cabal test hls-stan-plugin-tests

- if: matrix.test
name: Test hls-stylish-haskell-plugin
run: cabal test hls-stylish-haskell-plugin-tests || cabal test hls-stylish-haskell-plugin-tests
run: cabal test hls-stylish-haskell-plugin-tests || cabal test hls-stylish-haskell-plugin-tests

- if: matrix.test
- if: matrix.test
name: Test hls-ormolu-plugin
run: cabal test hls-ormolu-plugin-tests || cabal test hls-ormolu-plugin-tests
run: cabal test hls-ormolu-plugin-tests || cabal test hls-ormolu-plugin-tests

- if: matrix.test
- if: matrix.test
name: Test hls-fourmolu-plugin
run: cabal test hls-fourmolu-plugin-tests || cabal test hls-fourmolu-plugin-tests
run: cabal test hls-fourmolu-plugin-tests || cabal test hls-fourmolu-plugin-tests

- if: matrix.test
name: Test hls-explicit-imports-plugin test suite
run: cabal test hls-explicit-imports-plugin-tests || cabal test hls-explicit-imports-plugin-tests
run: cabal test hls-explicit-imports-plugin-tests || cabal test hls-explicit-imports-plugin-tests

- if: matrix.test
name: Test hls-call-hierarchy-plugin test suite
run: cabal test hls-call-hierarchy-plugin-tests || cabal test hls-call-hierarchy-plugin-tests
run: cabal test hls-call-hierarchy-plugin-tests || cabal test hls-call-hierarchy-plugin-tests

- if: matrix.test && matrix.os != 'windows-latest'
name: Test hls-rename-plugin test suite
run: cabal test hls-rename-plugin-tests || cabal test hls-rename-plugin-tests
run: cabal test hls-rename-plugin-tests || cabal test hls-rename-plugin-tests

- if: matrix.test
- if: matrix.test
name: Test hls-hlint-plugin test suite
run: cabal test hls-hlint-plugin-tests || cabal test hls-hlint-plugin-tests
run: cabal test hls-hlint-plugin-tests || cabal test hls-hlint-plugin-tests

- if: matrix.test
name: Test hls-module-name-plugin test suite
run: cabal test hls-module-name-plugin-tests || cabal test hls-module-name-plugin-tests
run: cabal test hls-module-name-plugin-tests || cabal test hls-module-name-plugin-tests

- if: matrix.test
name: Test hls-alternate-number-format-plugin test suite
run: cabal test hls-alternate-number-format-plugin-tests || cabal test hls-alternate-number-format-plugin-tests
run: cabal test hls-alternate-number-format-plugin-tests || cabal test hls-alternate-number-format-plugin-tests

- if: matrix.test
name: Test hls-qualify-imported-names-plugin test suite
run: cabal test hls-qualify-imported-names-plugin-tests || cabal test hls-qualify-imported-names-plugin-tests
run: cabal test hls-qualify-imported-names-plugin-tests || cabal test hls-qualify-imported-names-plugin-tests

- if: matrix.test
name: Test hls-code-range-plugin test suite
run: cabal test hls-code-range-plugin-tests || cabal test hls-code-range-plugin-tests
run: cabal test hls-code-range-plugin-tests || cabal test hls-code-range-plugin-tests

- if: matrix.test
name: Test hls-change-type-signature test suite
run: cabal test hls-change-type-signature-plugin-tests || cabal test hls-change-type-signature-plugin-tests
run: cabal test hls-change-type-signature-plugin-tests || cabal test hls-change-type-signature-plugin-tests

- if: matrix.test
name: Test hls-gadt-plugin test suit
run: cabal test hls-gadt-plugin-tests || cabal test hls-gadt-plugin-tests
run: cabal test hls-gadt-plugin-tests || cabal test hls-gadt-plugin-tests

- if: matrix.test
name: Test hls-explicit-fixity-plugin test suite
run: cabal test hls-explicit-fixity-plugin-tests || cabal test hls-explicit-fixity-plugin-tests
run: cabal test hls-explicit-fixity-plugin-tests || cabal test hls-explicit-fixity-plugin-tests

- if: matrix.test
name: Test hls-explicit-record-fields-plugin test suite
run: cabal test hls-explicit-record-fields-plugin-tests || cabal test hls-explicit-record-fields-plugin-tests
run: cabal test hls-explicit-record-fields-plugin-tests || cabal test hls-explicit-record-fields-plugin-tests

## version needs to be limited since the tests depend on cabal-fmt which only builds using specific ghc versions
- if: matrix.test && matrix.ghc == '9.2'
name: Test hls-cabal-fmt-plugin test suite
run: cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests || cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests
run: cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests || cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests

- if: matrix.test
name: Test hls-cabal-gild-plugin test suite
run: cabal test hls-cabal-gild-plugin-tests --flag=isolateCabalGildTests || cabal test hls-cabal-gild-plugin-tests --flag=isolateCabalGildTests

- if: matrix.test
name: Test hls-cabal-plugin test suite
run: cabal test hls-cabal-plugin-tests || cabal test hls-cabal-plugin-tests
run: cabal test hls-cabal-plugin-tests || cabal test hls-cabal-plugin-tests

- if: matrix.test
name: Test hls-retrie-plugin test suite
run: cabal test hls-retrie-plugin-tests || cabal test hls-retrie-plugin-tests
run: cabal test hls-retrie-plugin-tests || cabal test hls-retrie-plugin-tests

- if: matrix.test
name: Test hls-overloaded-record-dot-plugin test suite
run: cabal test hls-overloaded-record-dot-plugin-tests || cabal test hls-overloaded-record-dot-plugin-tests
run: cabal test hls-overloaded-record-dot-plugin-tests || cabal test hls-overloaded-record-dot-plugin-tests

- if: matrix.test
name: Test hls-semantic-tokens-plugin test suite
run: cabal test hls-semantic-tokens-plugin-tests || cabal test hls-semantic-tokens-plugin-tests
run: cabal test hls-semantic-tokens-plugin-tests || cabal test hls-semantic-tokens-plugin-tests


test_post_job:
Expand Down
7 changes: 4 additions & 3 deletions CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@
# Plugins
/plugins/hls-alternate-number-format-plugin @drsooch
/plugins/hls-cabal-fmt-plugin @VeryMilkyJoe @fendor
/plugins/hls-cabal-gild-plugin @fendor
/plugins/hls-cabal-plugin @fendor
/plugins/hls-call-hierarchy-plugin @July541
/plugins/hls-change-type-signature-plugin
/plugins/hls-class-plugin
/plugins/hls-class-plugin
/plugins/hls-code-range-plugin @kokobd
/plugins/hls-eval-plugin
/plugins/hls-explicit-fixity-plugin
Expand All @@ -33,7 +34,7 @@
/plugins/hls-pragmas-plugin @eddiemundo
/plugins/hls-qualify-imported-names-plugin @eddiemundo
/plugins/hls-refactor-plugin @santiweight
/plugins/hls-rename-plugin
/plugins/hls-rename-plugin
/plugins/hls-retrie-plugin @pepeiborra
/plugins/hls-semantic-tokens-plugin @soulomoon
/plugins/hls-splice-plugin @konn
Expand All @@ -48,7 +49,7 @@
/docs @michaelpj

# CI
/.circleci
/.circleci
/.github @michaelpj @fendor

# Build
Expand Down
53 changes: 53 additions & 0 deletions haskell-language-server.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,58 @@ test-suite hls-cabal-fmt-plugin-tests

if flag(isolateCabalfmtTests)
build-tool-depends: cabal-fmt:cabal-fmt ^>=0.1.6
cpp-options: -Dhls_isolate_cabalfmt_tests

-----------------------------
-- cabal-gild plugin
-----------------------------

flag cabalgild
description: Enable cabal-gild plugin
default: True
manual: True

common cabalgild
if flag(cabalgild)
build-depends: haskell-language-server:hls-cabal-gild-plugin
cpp-options: -Dhls_cabalgild

flag isolateCabalGildTests
description: Should tests search for 'cabal-gild' on the $PATH or shall we install it via build-tool-depends?
-- By default, search on the PATH
default: False
manual: True

library hls-cabal-gild-plugin
import: defaults, pedantic, warnings
exposed-modules: Ide.Plugin.CabalGild
hs-source-dirs: plugins/hls-cabal-gild-plugin/src
build-depends:
, base >=4.12 && <5
, directory
, filepath
, ghcide == 2.7.0.0
, hls-plugin-api == 2.7.0.0
, lsp-types
, text
, mtl
, process-extras

test-suite hls-cabal-gild-plugin-tests
import: defaults, pedantic, test-defaults, warnings
type: exitcode-stdio-1.0
hs-source-dirs: plugins/hls-cabal-gild-plugin/test
main-is: Main.hs
build-depends:
, base
, directory
, filepath
, haskell-language-server:hls-cabal-gild-plugin
, hls-test-utils == 2.7.0.0

if flag(isolateCabalGildTests)
build-tool-depends: cabal-gild:cabal-gild ^>=1.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to do it like cabal-fmt? Or can we do it more like the other formatters, where we use the library by default and have an option to use an executable optionally? I thought we did it like this because cabal-fmt was particularly difficult to build? But maybe cabal-gild is easier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cabal-gild is easier, but it doesn't compile with ghc 9.2. We can disable the plugin for that GHC version, I don't feel strongly.

Copy link

Choose a reason for hiding this comment

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

I don't think it would be terribly hard to add support for GHC 9.2. Let me see what I can do.

Aside from that, Gild doesn't really expose a high-level "format" function. That's because I primarily intended for it to be an application rather than a library. The mainWith function can be used that way, but it's not very convenient. I could add a format function. What should the type signature be? Something like ByteString -> IO ByteString?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A format function would be nice, but we need to be able to pass the FilePath option for cabal-gild: discover feature to work.
Perhaps a function Opts -> ByteString -> IO ByteString?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's because I primarily intended for it to be an application rather than a library.

That's a good reason for us to interact with it that way.

It can be nice to at least have the option to use a compiled-in library version instead of shelling out to a process, but that does lead to the dread #411

Copy link

Choose a reason for hiding this comment

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

I added support for GHC 9.2 in Gild version 1.1.1.0: https://github.com/tfausak/cabal-gild/releases/tag/1.1.1.0

I'll noodle on a high level formatting function. The existing mainWith function can be used in a pure manner, it's just kind of annoying. See the test suite for an example: https://github.com/tfausak/cabal-gild/blob/747550fb20c4ab4003a199ebacd07130ce16796c/source/test-suite/Main.hs#L999

cpp-options: -Dhls_isolate_cabalgild_tests

-----------------------------
-- cabal plugin
Expand Down Expand Up @@ -1642,6 +1694,7 @@ library
, cabal
, callHierarchy
, cabalfmt
, cabalgild
, changeTypeSignature
, class
, eval
Expand Down
4 changes: 3 additions & 1 deletion hls-plugin-api/src/Ide/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ instance ToJSON Config where
object [ "checkParents" .= checkParents
, "checkProject" .= checkProject
, "formattingProvider" .= formattingProvider
, "cabalFormattingProvider" .= cabalFormattingProvider
, "maxCompletions" .= maxCompletions
, "plugin" .= Map.mapKeysMonotonic (\(PluginId p) -> p) plugins
]
Expand All @@ -189,7 +190,8 @@ instance Default Config where
, formattingProvider = "ormolu"
-- , formattingProvider = "floskell"
-- , formattingProvider = "stylish-haskell"
, cabalFormattingProvider = "cabal-fmt"
, cabalFormattingProvider = "cabal-gild"
-- , cabalFormattingProvider = "cabal-fmt"
-- this string value needs to kept in sync with the value provided in HlsPlugins
, maxCompletions = 40
, plugins = mempty
Expand Down
33 changes: 23 additions & 10 deletions plugins/hls-cabal-fmt-plugin/src/Ide/Plugin/CabalFmt.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedLabels #-}
{-# LANGUAGE OverloadedStrings #-}

module Ide.Plugin.CabalFmt where
Expand All @@ -9,6 +11,7 @@ import Control.Monad.IO.Class
import qualified Data.Text as T
import Development.IDE hiding (pluginHandlers)
import Ide.Plugin.Error (PluginError (PluginInternalError, PluginInvalidParams))
import Ide.Plugin.Properties
import Ide.PluginUtils
import Ide.Types
import qualified Language.LSP.Protocol.Lens as L
Expand All @@ -24,7 +27,7 @@ data Log
= LogProcessInvocationFailure Int
| LogReadCreateProcessInfo T.Text [String]
| LogInvalidInvocationInfo
| LogCabalFmtNotFound
| LogFormatterBinNotFound FilePath
deriving (Show)

instance Pretty Log where
Expand All @@ -35,29 +38,39 @@ instance Pretty Log where
["Invocation of cabal-fmt with arguments" <+> pretty args]
++ ["failed with standard error:" <+> pretty stdErrorOut | not (T.null stdErrorOut)]
LogInvalidInvocationInfo -> "Invocation of cabal-fmt with range was called but is not supported."
LogCabalFmtNotFound -> "Couldn't find executable 'cabal-fmt'"
LogFormatterBinNotFound fp -> "Couldn't find formatter executable 'cabal-fmt' at:" <+> pretty fp

descriptor :: Recorder (WithPriority Log) -> PluginId -> PluginDescriptor IdeState
descriptor recorder plId =
(defaultCabalPluginDescriptor plId "Provides formatting of cabal files with cabal-fmt")
{ pluginHandlers = mkFormattingHandlers (provider recorder)
{ pluginHandlers = mkFormattingHandlers (provider recorder plId)
, pluginConfigDescriptor = defaultConfigDescriptor{configCustomConfig = mkCustomConfig properties}
}

properties :: Properties '[ 'PropertyKey "path" 'TString]
properties =
emptyProperties
& defineStringProperty
#path
"Set path to 'cabal-fmt' executable"
"cabal-fmt"

-- | Formatter provider of cabal fmt.
-- Formats the given source in either a given Range or the whole Document.
-- If the provider fails an error is returned that can be displayed to the user.
provider :: Recorder (WithPriority Log) -> FormattingHandler IdeState
provider recorder _ _ (FormatRange _) _ _ _ = do
provider :: Recorder (WithPriority Log) -> PluginId -> FormattingHandler IdeState
provider recorder _ _ _ (FormatRange _) _ _ _ = do
logWith recorder Info LogInvalidInvocationInfo
throwError $ PluginInvalidParams "You cannot format a text-range using cabal-fmt."
provider recorder _ide _ FormatText contents nfp opts = do
provider recorder plId ideState _ FormatText contents nfp opts = do
let cabalFmtArgs = [ "--indent", show tabularSize]
x <- liftIO $ findExecutable "cabal-fmt"
cabalFmtExePath <- fmap T.unpack $ liftIO $ runAction "cabal-gild" ideState $ usePropertyAction #path plId properties
x <- liftIO $ findExecutable cabalFmtExePath
case x of
Just _ -> do
(exitCode, out, err) <-
liftIO $ Process.readCreateProcessWithExitCode
( proc "cabal-fmt" cabalFmtArgs
( proc cabalFmtExePath cabalFmtArgs
)
{ cwd = Just $ takeDirectory fp
}
Expand All @@ -71,8 +84,8 @@ provider recorder _ide _ FormatText contents nfp opts = do
let fmtDiff = makeDiffTextEdit contents out
pure $ InL fmtDiff
Nothing -> do
log Error LogCabalFmtNotFound
throwError (PluginInternalError "No installation of cabal-fmt could be found. Please install it into your global environment.")
log Error $ LogFormatterBinNotFound cabalFmtExePath
throwError (PluginInternalError "No installation of cabal-gild could be found. Please install it globally, or provide the full path to the executable")
where
fp = fromNormalizedFilePath nfp
tabularSize = opts ^. L.tabSize
Expand Down
Loading