Add test for '--help' option parser about iris HOT 5 CLOSED

@CThuleHansen You understand the required changes correctly with only a single change 😌

• Instead of passing cmdParserInfo as an argument to mkCliEnv, let's make cmdParserInfo accept CliEnvSettings. And then we can pass all the settings from mkCliEnv to cmdParserInfo

This way, in tests, we can construct ParserInfo using cmdParserInfo by specifying our test settings.

As I write this, I realise that it'll require to move CliEnvSettings into their own module Iris.Settings to avoid cyclic dependencies. But I'm fine with this, let's do this 😌

from iris.

Hi. So I have a looked a bit on this and I have a question.
"Extract cmdParserInfo from Iris.Env to Iris.Cli"
does this mean to move cmdParserInfo into a file Iris/Cli/ParserInfo.hs within the module Iris.Cli.ParserInfo?
And pass it as an argument to mkCliEnv?

from iris.

Okay, so I am approaching the target at https://github.com/CThuleHansen/iris and here is a small status and a few questions.
I have extracted and moved some bits according to our discussion, but I also moved cmd to Settings.hs, which seems off. But I need to access it from ParserInfo, so it is to avoid cyclic dependencies.
Question: Is a Types.hs file needed?

When I run my test, then --no-input parses successfully. However, --help gives a parserFailure.
I am thinking this is because --help is baked in and not added manually, whereas --no-input is not.
The output for --help seems correct besides the ParserFailure:

Question: What to pass for ParserPrefs in execParserPure? These are not very well documented in https://hackage.haskell.org/package/optparse-applicative-0.17.0.0/docs/Options-Applicative-Extra.html#t:ParserPrefs
I have used Opt.ParserPrefs "suffix" False False False OptTypes.Backtrack 80 False False 0
What is prefMultiSuffix? For the others, I just took default.

I am not getting the option for version and numeric-version because I have not passed any version settings yet.
This might fix the issue with help as well. So I do not have any questions for this as of yet :)

from iris.

I have extracted and moved some bits according to our discussion, but I also moved cmd to Settings.hs, which seems off. But I need to access it from ParserInfo, so it is to avoid cyclic dependencies.
Question: Is a Types.hs file needed?

I see the dilemma. Putting Cmd inside the Settings modules feels off to me as well. In that case, let's put it into a separate module Iris.Cli.Cmd. It's a part of CLI so it makes sense to keep it under a separate module. The data type is small at the moment but it'll get bigger. And putting it into a separate module will enable to write more detailed documentation on it 😌

I'm generally against generic modules like Types.hs, Helper.hs, Utils.hs, Common.hs and etc. So I'd like to avoid such modules in my projects. They tend to become a kitchen-sink of everything. It's worth thinking about a more appropriate place to put generic stuff in.

However, --help gives a parserFailure.
I am thinking this is because --help is baked in and not added manually, whereas --no-input is not.
The output for --help seems correct besides the ParserFailure

Indeed, this is the optparse-applicative-specific behaviour. The --help option is implemented ParserFailure to stop parsing immediately and output the result of --help. In that case, in tests for the --help option, we should expect ParserFailure and match the expected text with the hardcoded text of --help.

Question: What to pass for ParserPrefs in execParserPure?

The Iris.Env module uses the execParser function to parse the CLI arguments. I see from its code that it uses the defaultPrefs value. So I propose to use it in the tests as well.

from iris.

Pull-request created: #68

I'm generally against generic modules like Types.hs, Helper.hs, Utils.hs, Common.hs and etc. So I'd like to avoid such modules in my projects. They tend to become a kitchen-sink of everything. It's worth thinking about a more appropriate place to put generic stuff in.

Yes, they certainly do.

The Iris.Env module uses the execParser function to parse the CLI arguments. I see from its code that it uses the defaultPrefs value. So I propose to use it in the tests as well.

I should have found this, thanks :)

from iris.