chshersh/iris

Rethink 'CliEnvSettings' and 'defaultCliEnvSettings'

chshersh opened this issue · 3 comments

Currently, the CliEnvSettings data type is defined in the following way:

iris/src/Iris/Env.hs

Lines 53 to 71 in 0b9c50d

data CliEnvSettings (cmd :: Type) (appEnv :: Type) = CliEnvSettings
{ -- | @since 0.0.0.0
cliEnvSettingsCmdParser :: Opt.Parser cmd
-- | @since 0.0.0.0
, cliEnvSettingsAppEnv :: appEnv
-- | @since 0.0.0.0
, cliEnvSettingsHeaderDesc :: String
-- | @since 0.0.0.0
, cliEnvSettingsProgDesc :: String
-- | @since 0.0.0.0
, cliEnvSettingsVersionSettings :: Maybe VersionSettings
-- | @since 0.0.0.0
, cliEnvSettingsRequiredTools :: [Tool cmd]
}

And defaultCliEnvSettings:

iris/src/Iris/Env.hs

Lines 78 to 86 in 0b9c50d

defaultCliEnvSettings :: CliEnvSettings () ()
defaultCliEnvSettings = CliEnvSettings
{ cliEnvSettingsCmdParser = pure ()
, cliEnvSettingsAppEnv = ()
, cliEnvSettingsHeaderDesc = "Simple CLI program"
, cliEnvSettingsProgDesc = "CLI tool build with iris - a Haskell CLI framework"
, cliEnvSettingsVersionSettings = Nothing
, cliEnvSettingsRequiredTools = []
}

The idea of using () in the type signature was to rely on type-changing record updates. Unfortunately, the following code doesn't compile:

cmdP :: Parser Cmd
cmdP = ...

appSettings :: Iris.CliEnvSettings Cmd ()
appSettings = Iris.defaultCliEnvSettings
    { Iris.cliEnvSettingsCmdParser = cmdP
    }

It fails with the error:

... location ...
    • Couldn't match type ‘()’ with ‘Cmd’
      Expected: CliEnvSettings Cmd ()
        Actual: CliEnvSettings () ()
    • In the expression:
        Iris.defaultCliEnvSettings {cliEnvSettingsCmdParser = cmdP}
      In an equation for ‘appSettings’:
          appSettings
            = Iris.defaultCliEnvSettings {cliEnvSettingsCmdParser = cmdP}
   |
26 | appSettings = Iris.defaultCliEnvSettings
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^...

... location ...
    • Couldn't match type ‘Cmd’ with ‘()’
      Expected: Options.Applicative.Types.Parser ()
        Actual: Options.Applicative.Types.Parser Cmd
    • In the ‘cliEnvSettingsCmdParser’ field of a record
      In the expression:
        Iris.defaultCliEnvSettings {cliEnvSettingsCmdParser = cmdP}
      In an equation for ‘appSettings’:
          appSettings
            = Iris.defaultCliEnvSettings {cliEnvSettingsCmdParser = cmdP}
   |
27 |     { cliEnvSettingsCmdParser = cmdP
   |                                 ^^^^

The problem here because the cliEnvSettingsRequiredTools is also parametrised by cmd. So to change the type, you need to update both fields like this:

appSettings :: Iris.CliEnvSettings Cmd ()
appSettings = Iris.defaultCliEnvSettings
    { Iris.cliEnvSettingsCmdParser = cmdP
    , Iris.cliEnvSettingsRequiredTools = []
    }

Which is a shame. You need to set an extra field you don't care about all the time because you want to change the type of the CLI command.

It would be great to improve this interface. However, I don't have ideas at the moment. This requires some thinking 🤔

Okay, I was thinking about the interface and here is the proposed solution:

  • Remove the cliEnvSettingsRequiredTools :: [Tool cmd] field from the CliEnvSettings options
  • Change the type of Tool to not have the cmd type parameter
  • Implement a separate function need :: Monad ... => [Tool] -> m ()

So, instead of specifying a global list of tools, users can call the need function in the required commands:

app :: App ()
app = Iris.asksCliEnv Iris.cliEnvCmd >>= \case
    Download url -> do
        need ["curl"]
        runDownload URL
    Evaluate hs -> do
        need ["ghc", "cabal"]
        runEvaluate hs

Is anyone working on this? I can take a look otherwise 😄

@german1608 I believe no one is looking (or at least haven't written about this explicitly). So feel free to work on this 🙂