Add cabal-gild as a cabal file formatter plugin #4101
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| {-# LANGUAGE DataKinds #-} | ||
| {-# LANGUAGE LambdaCase #-} | ||
| {-# LANGUAGE OverloadedLabels #-} | ||
| {-# LANGUAGE OverloadedStrings #-} | ||
|
|
||
| module Ide.Plugin.CabalGild where | ||
|
|
||
| import Control.Monad.Except (throwError) | ||
| 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 Language.LSP.Protocol.Types | ||
| import Prelude hiding (log) | ||
| import System.Directory | ||
| import System.Exit | ||
| import System.FilePath | ||
| import System.Process.ListLike | ||
| import qualified System.Process.Text as Process | ||
|
|
||
| data Log | ||
| = LogProcessInvocationFailure Int T.Text | ||
| | LogReadCreateProcessInfo [String] | ||
| | LogInvalidInvocationInfo | ||
| | LogFormatterBinNotFound FilePath | ||
| deriving (Show) | ||
|
|
||
| instance Pretty Log where | ||
| pretty = \case | ||
| LogProcessInvocationFailure exitCode err -> | ||
| vcat | ||
| [ "Invocation of cabal-gild failed with code" <+> pretty exitCode | ||
| , "Stderr:" <+> pretty err | ||
| ] | ||
| LogReadCreateProcessInfo args -> | ||
| "Formatter invocation: cabal-gild " <+> pretty args | ||
| LogInvalidInvocationInfo -> "Invocation of cabal-gild with range was called but is not supported." | ||
| LogFormatterBinNotFound fp -> "Couldn't find formatter executable 'cabal-gild' at:" <+> pretty fp | ||
|
|
||
| descriptor :: Recorder (WithPriority Log) -> PluginId -> PluginDescriptor IdeState | ||
| descriptor recorder plId = | ||
| (defaultCabalPluginDescriptor plId "Provides formatting of cabal files with cabal-gild") | ||
| { pluginHandlers = mkFormattingHandlers (provider recorder plId) | ||
| , pluginConfigDescriptor = defaultConfigDescriptor{configCustomConfig = mkCustomConfig properties} | ||
| } | ||
|
|
||
| properties :: Properties '[ 'PropertyKey "path" 'TString] | ||
| properties = | ||
| emptyProperties | ||
| & defineStringProperty | ||
| #path | ||
| "Set path to 'cabal-gild' executable" | ||
| "cabal-gild" | ||
|
|
||
| -- | Formatter provider of cabal gild. | ||
| -- 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) -> PluginId -> FormattingHandler IdeState | ||
| provider recorder _ _ _ (FormatRange _) _ _ _ = do | ||
| logWith recorder Info LogInvalidInvocationInfo | ||
| throwError $ PluginInvalidParams "You cannot format a text-range using cabal-gild." | ||
|
There was a problem hiding this comment. You kind of can, depending on what exactly "range" means. For example: $ echo 'build-depends:{base>=4.19,bytestring>=0.12}' | cabal-gild
build-depends:
base >=4.19,
bytestring >=0.12But I wonder if anyone would actually want this behavior anyway. There was a problem hiding this comment. I think people could find range formatting useful. But supporting only top-level stanzas and fields... Not sure it's the most intuitive behaviour. There was a problem hiding this comment. Shall we make an issue so we can put a link to the issue in the error? 😈 There was a problem hiding this comment. I'd be happy to add support for formatting ranges if someone can point me to a resource that explains how it should work! As shown, Gild can format any subsection of a document that's still syntactically valid. But it can't, for example, format any arbitrary range. Like if you tried to format just the contents of the $ echo 'base>=4.19,bytestring>=0.12' | cabal-gild
base >= 4.19 , bytestring >= 0.12There was a problem hiding this comment.
You mean "The document range formatting request is sent from the client to the server to format a given range in a document" isn't good enough for you? 😂
Does it take into account the surrounding context? Perhaps it doesn't matter, but an easy way this kind of formatting could go wrong is if you format the range in isolation and therefore treat it as if it is at the top-level of the structure, which it might not be. But otherwise I think it's fine if range formatting just doesn't do anything unless you've selected a range that includes a whole element that can be sensibly formatted. There was a problem hiding this comment. I think the only way it could take surrounding context into account is to maintain leading blank spaces. Otherwise it has no way of knowing that it's "inside" a How does range formatting work exactly? If Gild would only get the range's content as input, then there's not much it can do. But if it gets the entire buffer's input along with a span identifying the range to format, then potentially it could do something nice. (Although that might be a lot of work for me.) There was a problem hiding this comment.
That's what you get, indeed. There was a problem hiding this comment. I created an issue for this: tfausak/cabal-gild#42. No idea if or when I'll get around to implementing it though. |
||
| provider recorder plId ideState _ FormatText contents nfp _ = do | ||
| let cabalGildArgs = ["--stdin=" <> fp, "--input=-"] -- < Read from stdin | ||
|
|
||
| cabalGildExePath <- fmap T.unpack $ liftIO $ runAction "cabal-gild" ideState $ usePropertyAction #path plId properties | ||
| x <- liftIO $ findExecutable cabalGildExePath | ||
| case x of | ||
| Just _ -> do | ||
| log Debug $ LogReadCreateProcessInfo cabalGildArgs | ||
| (exitCode, out, err) <- | ||
| liftIO $ Process.readCreateProcessWithExitCode | ||
| ( proc cabalGildExePath cabalGildArgs | ||
| ) | ||
| { cwd = Just $ takeDirectory fp | ||
| } | ||
| contents | ||
| case exitCode of | ||
| ExitFailure code -> do | ||
| log Error $ LogProcessInvocationFailure code err | ||
| throwError (PluginInternalError "Failed to invoke cabal-gild") | ||
| ExitSuccess -> do | ||
| let fmtDiff = makeDiffTextEdit contents out | ||
| pure $ InL fmtDiff | ||
| Nothing -> do | ||
| log Error $ LogFormatterBinNotFound cabalGildExePath | ||
| 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 | ||
| log = logWith recorder | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| {-# LANGUAGE CPP #-} | ||
| {-# LANGUAGE OverloadedStrings #-} | ||
| module Main | ||
| ( main | ||
| ) where | ||
|
|
||
| import qualified Ide.Plugin.CabalGild as CabalGild | ||
| import System.Directory (findExecutable) | ||
| import System.FilePath | ||
| import Test.Hls | ||
|
|
||
| data CabalGildFound = Found | NotFound | ||
|
|
||
| isTestIsolated :: Bool | ||
| #if hls_isolate_cabalgild_tests | ||
| isTestIsolated = True | ||
| #else | ||
| isTestIsolated = False | ||
| #endif | ||
|
|
||
| isCabalFmtFound :: IO CabalGildFound | ||
| isCabalFmtFound = case isTestIsolated of | ||
| True -> pure Found | ||
| False -> do | ||
| cabalGild <- findExecutable "cabal-gild" | ||
| pure $ maybe NotFound (const Found) cabalGild | ||
|
|
||
| main :: IO () | ||
| main = do | ||
| foundCabalFmt <- isCabalFmtFound | ||
| defaultTestRunner (tests foundCabalFmt) | ||
|
|
||
| cabalGildPlugin :: PluginTestDescriptor CabalGild.Log | ||
| cabalGildPlugin = mkPluginTestDescriptor CabalGild.descriptor "cabal-gild" | ||
|
|
||
| tests :: CabalGildFound -> TestTree | ||
| tests found = testGroup "cabal-gild" | ||
| [ cabalGildGolden found "formats a simple document" "simple_testdata" "formatted_document" $ \doc -> do | ||
| formatDoc doc (FormattingOptions 2 True Nothing Nothing Nothing) | ||
|
|
||
| , cabalGildGolden found "formats a document with expand:src comment" "commented_testdata" "formatted_document" $ \doc -> do | ||
| formatDoc doc (FormattingOptions 2 True Nothing Nothing Nothing) | ||
|
|
||
| , cabalGildGolden found "formats a document with lib information" "lib_testdata" "formatted_document" $ \doc -> do | ||
| formatDoc doc (FormattingOptions 10 True Nothing Nothing Nothing) | ||
| ] | ||
|
|
||
| cabalGildGolden :: CabalGildFound -> TestName -> FilePath -> FilePath -> (TextDocumentIdentifier -> Session ()) -> TestTree | ||
| cabalGildGolden NotFound title _ _ _ = | ||
| testCase title $ | ||
| assertFailure $ "Couldn't find cabal-gild on PATH or this is not an isolated run. " | ||
| <> "Use cabal flag 'isolateCabalGildTests' to make it isolated or install cabal-gild locally." | ||
| cabalGildGolden Found title path desc act = goldenWithCabalDocFormatter def cabalGildPlugin "cabal-gild" conf title testDataDir path desc "cabal" act | ||
| where | ||
| conf = def | ||
|
|
||
| testDataDir :: FilePath | ||
| testDataDir = "plugins" </> "hls-cabal-gild-plugin" </> "test" </> "testdata" |
There was a problem hiding this comment.
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 becausecabal-fmtwas particularly difficult to build? But maybecabal-gildis easier?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cabal-gildis easier, but it doesn't compile with ghc 9.2. We can disable the plugin for that GHC version, I don't feel strongly.There was a problem hiding this comment.
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
mainWithfunction can be used that way, but it's not very convenient. I could add aformatfunction. What should the type signature be? Something likeByteString -> IO ByteString?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
formatfunction would be nice, but we need to be able to pass theFilePathoption forcabal-gild: discoverfeature to work.Perhaps a function
Opts -> ByteString -> IO ByteString?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
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
mainWithfunction 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