Skip to content
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

Implement Yes/No reading functions #9

Closed
5 tasks done
chshersh opened this issue May 15, 2022 · 13 comments
Closed
5 tasks done

Implement Yes/No reading functions #9

chshersh opened this issue May 15, 2022 · 13 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@chshersh
Copy link
Owner

chshersh commented May 15, 2022

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

  • Create the module Iris.Interactive.Question.
  • Implement the following type there:
    data YesNo = No | Yes
  • Add the following function to handle various cases like y, Yes, NO, etc. with the following type:
    parseYesNo :: Text -> Maybe YesNo
  • Additionally, create the following function that should ask a question, read the answer and return it
    yesno
        :: (MonadIO m, MonadReader (CliEnv cmd appEnv))
        => Text  -- ^ Question
        -> YesNo  -- ^ Default answer when --no-input is provided
        -> m YesNo  -- ^ Resulting answer
  • Write tests in Test.Iris.Interactive.Question

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"
@chshersh chshersh added enhancement New feature or request terminal and removed enhancement New feature or request labels May 15, 2022
@chshersh chshersh moved this to Todo in Iris Project May 15, 2022
@chshersh chshersh added this to the Big launch milestone May 15, 2022
@chshersh chshersh moved this from Todo to In Progress in Iris Project May 21, 2022
@chshersh chshersh moved this from In Progress to Todo in Iris Project May 22, 2022
@chshersh chshersh moved this from Todo to In Progress in Iris Project Jun 13, 2022
@chshersh chshersh self-assigned this Jun 13, 2022
@chshersh chshersh removed this from the v0.0.0.0: Initial Launch milestone Jun 13, 2022
@chshersh chshersh moved this from In Progress to Todo in Iris Project Jun 13, 2022
@initial-mockingbird
Copy link

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

initial-mockingbird added a commit to initial-mockingbird/iris that referenced this issue Oct 2, 2022
    * Added Yes/No functions and multiple choice functions (Issue chshersh#9).
    * Added a protected writefile function (Issue chshersh#14)

possible improvements: move to ansi-terminal or other couloring library in order to manage colours in a more scalable way (currently ANSI Escape codes are used)
@chshersh
Copy link
Owner Author

chshersh commented Oct 3, 2022

@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
Copy link
Collaborator

martinhelmer commented Mar 18, 2023

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
Copy link
Collaborator

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

@chshersh
Copy link
Owner Author

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
Copy link
Collaborator

👍 pls update the usage example as well

@martinhelmer
Copy link
Collaborator

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
Copy link
Collaborator

furthermore: what if the answer is not parseable

  1. use default
  2. keep asking until we encounter something parseable
    ?

@martinhelmer
Copy link
Collaborator

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
Copy link
Owner Author

@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
Copy link
Collaborator

roger that. tests look like this atm:
image

@chshersh chshersh assigned martinhelmer and unassigned chshersh Mar 23, 2023
chshersh added a commit that referenced this issue Mar 25, 2023
* Add tests (mainly)

* Add Changelog

* Remove unnecessary changes (store handle in env)

* fix formatting

* fix more formatting (test)

* PR fixes

* Update test/Test/Iris/Interactive/Question.hs

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>

* test recactoring and changelog fixed

---------

Co-authored-by: Dmitrii Kovanikov <kovanikov@gmail.com>
@martinhelmer
Copy link
Collaborator

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

@chshersh
Copy link
Owner Author

chshersh commented Apr 4, 2023

@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 🤖

@chshersh chshersh closed this as completed Apr 4, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Iris Project Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants