chshersh / iris

🌈 Haskell CLI Framework supporting Command Line Interface Guidelines
https://hackage.haskell.org/package/iris
Mozilla Public License 2.0
177 stars 22 forks source link

Implement Yes/No reading functions #9

Closed chshersh closed 1 year ago

chshersh commented 2 years ago

The issue is about creating a type for asking and answering simple interactive questions.

Usage example:

app :: App ()
app = do
    answer <- Iris.yesno "Would you like to proceed?" Iris.Yes
    case answer of
        Yes -> proceed
        No -> Iris.outLn "Aborting"
initial-mockingbird commented 1 year ago

Maybe generalizing the Yes/No questions into multiple options via:

data MultipleChoicePrompt a = MultipleChoicePrompt
    { promptMessage :: ByteString -- ^ The prompted message
    , promptOptions :: NonEmpty (MultipleChoiceOption a) -- ^ each option
    }

And packing each option with a way to parse it:

data MultipleChoiceOption a = MultipleChoiceOption
    { displayName   :: ByteString 
    , parsingFun    :: ByteString -> Maybe a
    }

Would allow to write a function that prints the prompt (via Iris.Colour.Formatting.putStdoutColouredLn), reads the input, and return the first parse success

multipleChoiceQuestion
    ::  ( MonadIO m
        , MonadReader (CliEnv cmd appEnv) m
        )
    => MultipleChoicePrompt a
    -> m (Maybe a)

Since colourista isn'tyet in GHC 9.2, we can maybe use ansi-terminal to color things up, or straight up use Ansi Escape codes as a temp meassure. i.e:


-- | Resets colors back to normal
reset :: ByteString -> ByteString
reset s = s <> "\ESC[0m"

boldGreenText :: ByteString -> ByteString
boldGreenText s = "\ESC[1;38;5;82m" <> s <> reset

Finally, maybe this would be a good candidate for the IO module mentioned in Issue#14

chshersh commented 1 year ago

@initial-mockingbird This is a bit more nuanced. I imagine, I'm going to implement colouring entirely from scratch in Iris and I haven't figured the proper design yet.

Another disadvantage of colourista is that it doesn't support disabling colours in pure functions. I'd love to be able to describe how to format pure values of type Text without the need to run them in IO and writing the formatting twice and getting the colour disable based on CLI options.

I have a sketch of the final design but haven't got time to implement it. I've created the following issue to track the implementation of colouring separately:

As for the multiple choice questions. I believe the design for them will be more complicated than for Yes/No. So it can be implemented separately. Feel free to open a separate issue for Multiple-Choice questions and propose your implementation design there 🙂

martinhelmer commented 1 year ago

I'd like to take crack at this. This will be my first contribution ever to a project, so yay! I think i understand what's supposed to be done. Just one question: for getting the actual interactive input, just use Data.Text.IO.getLine ?

martinhelmer commented 1 year ago

also, predicting other types of questions (such as multiplechoice) , maybe yesnoquestion or simplequestion would be a more appropriate name ?

chshersh commented 1 year ago

Hey @martinhelmer 👋🏻

Happy to see your passion for contribution 🤗

Answering your questions,

for getting the actual interactive input, just use Data.Text.IO.getLine ?

Yes, you can use this function 👍🏻

also, predicting other types of questions (such as multiplechoice) , maybe yesnoquestion or simplequestion would be a more appropriate name ?

Good point. I envision only two types of questions: (1) Yes-No and (2) Multiple Choice. In that case, let's name the function just yesno, and the multiple choice one will be called just choice.

I've updated the issue description accordingly.

martinhelmer commented 1 year ago

👍 pls update the usage example as well

martinhelmer commented 1 year ago

also, if yesno has the use of getLine hard-coded it will be hard to write a test, no? either we use

yesno :: (MonadIO m, MonadReader (CliEnv cmd appEnv) m)
    => Text  -- ^ Question
    -> Answer  -- ^ Default answer when --no-input is provided
    -> m Answer  -- ^ Resulting answer
yesno = yesno' TIO.getLine  

yesno' :: (MonadIO m, MonadReader (CliEnv cmd appEnv) m)
    => IO Text -- ^ Actual answer as text 
    -> Text  -- ^ Question
    -> Answer  -- ^ Default answer when --no-input is provided
    -> m Answer  -- ^ Resulting answer
yesno' = undefined 

and test on yesno`

or, provide the (IO Text) function through the appEnv, (default TIO.getLine ) , setting it to something different in the tests.

or maybe there's a 3rd option I don't see?

martinhelmer commented 1 year ago

furthermore: what if the answer is not parseable

  1. use default
  2. keep asking until we encounter something parseable ?
martinhelmer commented 1 year ago

regarding how to test, suggest adding

data CliEnv (cmd :: Type) (appEnv :: Type) = CliEnv
    { ...
    , cliEnvInteractiveInputHandle :: Handle
    -- ^ @since x.x.x.x
    }

which defaults to stdin and can be set to fileinput by a test, or by future functionality which reads the interactive answers from file.

chshersh commented 1 year ago

@martinhelmer

what if the answer is not parseable

I suggest keeping asking in a loop. In the future, a more sophisticated and configurable approach can be implemented but from my experience, this is a good default.

if yesno has the use of getLine hard-coded it will be hard to write a test, no?

The best way to solve this problem is to test only relevant things. Mocks here won't help us test what we want and instead will make the code more complicated. If we have the function parseYesNo :: Text -> Maybe YesNo then it makes sense to test it. There's no benefit in testing a function that reads Text from stdin and calls parseYesNo on the result.

+1 pls update the usage example as well

After giving a consideration, I decided to rename the data type from Answer to YesNo (and changed all the functions accordingly). I believe it makes the API more consistent, self-explainable, and self-discoverable.

martinhelmer commented 1 year ago

roger that. tests look like this atm:

image
martinhelmer commented 1 year ago

is there anything left to do on this or can it be closed?

chshersh commented 1 year ago

@martinhelmer Indeed, this can be closed 🆗

I usually put a line like the following one at the beginning of the PR description to automatically close the issue on the PR merge:

Resolves #9

It's specified in the PR template but looks like it was edited away in #117. I forgot to check whether this line was specified or not when merged, so the issue didn't close automatically 🤖