-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Consider Access to the source input #16
Comments
Having the I agree that users should never have to unwrap the Regarding the member this.GetValue(parseResult: System.CommandLine.Parsing.ParseResult) =
match this.Source with
| ParsedOption o -> o :?> Option<'T> |> parseResult.GetValueForOption
| ParsedArgument a -> a :?> Argument<'T> |> parseResult.GetValueForArgument
| Context -> failwith "Cannot get value from InvcationContext input source." |
I was just checking out your Perla wizard code and I glanced at the way you are organizing your commands and HandlerInputs into modules. Looks interesting. At some point I need to pull your code down and analyze they way you are organizing it so I can have a better perspective of what's good or bad about FS.SCL in the context of a large project with logs of commands/inputs. |
I was iterating on what would be a good setup to test my commands but when I started looking into the SCL library itself it seems that if you want to add typed handlers you need to have access to the options/arguments e.g. let a = Argument(...)
let b = Option(...)
let command1 = Command()
command1.AddOption(a)
command1.AddArgument(b)
// here you need to be able to point at the options/arguments
// which we may not care at all that's what your project is about not having
// to deal with this
command1.SetHandler(Func<_,_, int>(fun a b -> 0), a, b)
let result = command1.Parse("the thing --and-that")
result. // <- at this point is when things get tricky
// and you need to have either the option/argument around to do anything meaningful I agree with you that there are not many cases where one may need to access the option/argument outside testings that's what is driving my extension here, for the moment This is what my tests are starting to look like and I think it's good enough for my case: let GetRootCommand (handler: Command, command: string) =
let root = RootCommand()
root.AddCommand(handler)
root.Parse(command)
[<Fact>]
let ``Commands.Setup can parse options`` () =
let result =
GetRootCommand(Commands.Setup, "setup -y --skip-playwright false")
let skipPlaywright: bool option =
// with your updated extension to avoid wrapping options within options
SetupInputs.skipPlaywright.GetValue result
let skipPrompts: bool option =
SetupInputs.skipPrompts.GetValue result
Assert.Empty(result.Errors)
Assert.True(skipPrompts |> Option.defaultValue false)
Assert.False(skipPlaywright |> Option.defaultValue true) For the moment I don't care too much about if the execution is well tested, but I do want to be sure that my custom inputs are parsing values correctly as I've run into some bug issues myself when testing the CLI app. So if I had to distil a core need here would be...
Ahh! I recently merged in dev my current approach to this so feel free to take a look at the dev branch. Don't pay too much attention to the logging stuff as I think I screwed up there, but I think command related things... are good enough for my needs. |
I like the way your wizard prefixes the output with a prefix ( Probably good that your wizard needs to be explicitly started by calling the |
Hello Jordan 👋 here I am once again!
I was trying to add tests for my commands and I figure out that one may want to access the source input to do multiple kind of operations on them given that the SLC library seems to use the inputs as contextual information to to either request values or to access the machinery under it
After looking at your code I came up with this:
And I can use it like this:
In this case I only have access to the ParseResult but to be able to get the value I need access to the source, thankfully you have already a similar function, but it requires the invocation context rather than the parsing results.
For my purposes I think this is just what I need but I'll leave it here in case this seems useful
Feel free to discuss/close as this is not a feature request
The text was updated successfully, but these errors were encountered: