-
-
Notifications
You must be signed in to change notification settings - Fork 388
Add an option to run Fourmolu via the CLI interface of a separate binary, rather than the bundled library #2763
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
Changes from 10 commits
9392fb4
0c21b0d
ace226d
b214278
42f8df3
1bd90af
3d9f964
74cab28
594777a
dc64fc4
236d244
129e584
558a3bc
bb0f45f
510e180
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,91 +2,127 @@ | |
{-# LANGUAGE LambdaCase #-} | ||
{-# LANGUAGE OverloadedStrings #-} | ||
{-# LANGUAGE TypeApplications #-} | ||
{-# LANGUAGE DataKinds #-} | ||
{-# LANGUAGE OverloadedLabels #-} | ||
|
||
module Ide.Plugin.Fourmolu ( | ||
descriptor, | ||
provider, | ||
) where | ||
|
||
import Control.Exception (try) | ||
import Control.Exception (IOException, try) | ||
import Control.Lens ((^.)) | ||
import Control.Monad | ||
import Control.Monad.IO.Class | ||
import Data.Bifunctor (first) | ||
import Data.Maybe | ||
import qualified Data.Text as T | ||
import qualified Data.Text.IO as T | ||
import Development.IDE hiding (pluginHandlers) | ||
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.PluginUtils (makeDiffTextEdit) | ||
import Ide.Plugin.Properties | ||
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 System.Exit | ||
import System.FilePath | ||
|
||
-- --------------------------------------------------------------------- | ||
import System.IO (stderr) | ||
import System.Process.Run (proc, cwd) | ||
import System.Process.Text (readCreateProcessWithExitCode) | ||
|
||
descriptor :: PluginId -> PluginDescriptor IdeState | ||
descriptor plId = | ||
(defaultPluginDescriptor plId) | ||
{ pluginHandlers = mkFormattingHandlers provider | ||
{ pluginHandlers = mkFormattingHandlers $ provider plId | ||
} | ||
|
||
-- --------------------------------------------------------------------- | ||
|
||
provider :: FormattingHandler IdeState | ||
provider ideState typ contents fp fo = withIndefiniteProgress title Cancellable $ do | ||
ghc <- liftIO $ runAction "Fourmolu" ideState $ use GhcSession fp | ||
fileOpts <- case hsc_dflags . hscEnv <$> ghc of | ||
Nothing -> return [] | ||
Just df -> liftIO $ convertDynFlags df | ||
|
||
let format printerOpts = | ||
first (responseError . ("Fourmolu: " <>) . T.pack . show) | ||
<$> try @OrmoluException (makeDiffTextEdit contents <$> ormolu config fp' (T.unpack contents)) | ||
where | ||
config = | ||
defaultConfig | ||
{ cfgDynOptions = fileOpts | ||
, cfgRegion = region | ||
, cfgDebug = True | ||
, cfgPrinterOpts = | ||
fillMissingPrinterOpts | ||
(printerOpts <> lspPrinterOpts) | ||
defaultPrinterOpts | ||
} | ||
properties :: Properties '[ 'PropertyKey "cli" 'TBoolean] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bikeshedding: "external"? "shell"? I think "cli" seems fine though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, surpisingly no word I could think of here seems perfect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've decided I do prefer "external": 558a3bc. Not going to bother changing the variable names though. |
||
properties = | ||
emptyProperties | ||
& defineBooleanProperty | ||
#cli | ||
"Call out to \"fourmolu\" executable, rather than using the bundled library" | ||
georgefst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
False | ||
|
||
liftIO (loadConfigFile fp') >>= \case | ||
ConfigLoaded file opts -> liftIO $ do | ||
putStrLn $ "Loaded Fourmolu config from: " <> file | ||
format opts | ||
ConfigNotFound searchDirs -> liftIO $ do | ||
putStrLn | ||
. unlines | ||
$ ("No " ++ show configFileName ++ " found in any of:") : | ||
map (" " ++) searchDirs | ||
format mempty | ||
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 | ||
provider :: PluginId -> FormattingHandler IdeState | ||
provider plId ideState typ contents fp fo = withIndefiniteProgress title Cancellable $ do | ||
fileOpts <- | ||
maybe [] (convertDynFlags . hsc_dflags . hscEnv) | ||
<$> liftIO (runAction "Fourmolu" ideState $ use GhcSession fp) | ||
useCLI <- usePropertyLsp #cli plId properties | ||
if useCLI | ||
then liftIO | ||
. fmap (join . first (mkError . show)) | ||
. try @IOException | ||
$ do | ||
(exitCode, out, err) <- | ||
readCreateProcessWithExitCode | ||
( proc "fourmolu" $ | ||
["-d"] | ||
<> catMaybes | ||
[ ("--start-line=" <>) . show <$> regionStartLine region | ||
, ("--end-line=" <>) . show <$> regionEndLine region | ||
] | ||
<> map ("-o" <>) fileOpts | ||
){cwd = Just $ takeDirectory fp'} | ||
contents | ||
T.hPutStrLn stderr err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should get logged properly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's no worse than with the non-CLI version. I'll open a new issue to track better logging for both. |
||
case exitCode of | ||
ExitSuccess -> | ||
pure . Right $ makeDiffTextEdit contents out | ||
ExitFailure n -> | ||
pure . Left . responseError $ "Fourmolu failed with exit code " <> T.pack (show n) | ||
else do | ||
let format printerOpts = | ||
first (mkError . show) | ||
<$> try @OrmoluException (makeDiffTextEdit contents <$> ormolu config fp' (T.unpack contents)) | ||
where | ||
config = | ||
defaultConfig | ||
{ cfgDynOptions = map DynOption fileOpts | ||
, cfgRegion = region | ||
, cfgDebug = True | ||
, cfgPrinterOpts = | ||
fillMissingPrinterOpts | ||
(printerOpts <> lspPrinterOpts) | ||
defaultPrinterOpts | ||
} | ||
in liftIO (loadConfigFile fp') >>= \case | ||
ConfigLoaded file opts -> liftIO $ do | ||
putStrLn $ "Loaded Fourmolu config from: " <> file | ||
format opts | ||
ConfigNotFound searchDirs -> liftIO $ do | ||
putStrLn | ||
. unlines | ||
$ ("No " ++ show configFileName ++ " found in any of:") : | ||
map (" " ++) searchDirs | ||
format mempty | ||
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 | ||
where | ||
fp' = fromNormalizedFilePath fp | ||
title = "Formatting " <> T.pack (takeFileName fp') | ||
mkError = responseError . ("Fourmolu: " <>) . T.pack | ||
lspPrinterOpts = mempty{poIndentation = Just $ fromIntegral $ fo ^. tabSize} | ||
region = case typ of | ||
FormatText -> | ||
RegionIndices Nothing Nothing | ||
FormatRange (Range (Position sl _) (Position el _)) -> | ||
RegionIndices (Just $ fromIntegral $ sl + 1) (Just $ fromIntegral $ el + 1) | ||
|
||
convertDynFlags :: DynFlags -> IO [DynOption] | ||
convertDynFlags :: DynFlags -> [String] | ||
convertDynFlags df = | ||
let pp = ["-pgmF=" <> p | not (null p)] | ||
p = sPgm_F $ Compat.settings df | ||
|
@@ -95,4 +131,4 @@ convertDynFlags df = | |
showExtension = \case | ||
Cpp -> "-XCPP" | ||
x -> "-X" ++ show x | ||
in return $ map DynOption $ pp <> pm <> ex | ||
in pp <> pm <> ex |
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.
Maybe document this function while you're here