AngelMunoz / Finny

A cross-platform tool for unbundled front-end development that doesn't depend on Node or requires you to install a complex toolchain
https://perla-docs.web.app/
MIT License
132 stars 11 forks source link

Cannot parse `perla new` with options #119

Closed AngelMunoz closed 1 year ago

AngelMunoz commented 1 year ago

Describe the bug

Running perla new MyProject -t ff or perla new MyProject -id perla.templates.fable.feliz causes InvalidOperationException when the command line argument parsing tried to bind the string to an F# string option.

 perla new WithId -id perla.templates.fable.feliz
Unhandled exception: System.InvalidOperationException: Cannot parse argument 'perla.templates.fable.feliz' for option '-id' as expected type 'Microsoft.FSharp.Core.FSharpOption`1[System.String]'.
   at System.CommandLine.Binding.ArgumentConverter.GetValueOrDefault[T](ArgumentConversionResult result)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
   at System.CommandLine.Parsing.SymbolResult.GetValueForOption[T](Option`1 option)
   at System.CommandLine.Parsing.ParseResult.GetValueForOption[T](Option`1 option)
   at FSharp.SystemCommandLine.CommandBuilders.SetHandlerTask@345-5.Invoke(InvocationContext ctx)
   at System.CommandLine.Invocation.AnonymousCommandHandler.InvokeAsync(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass17_0.<<UseParseErrorReporting>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Perla.CliMiddleware.MiddlewareImpl.previewCheck@43-1.MoveNext() in C:\Users\scyth\repos\Perla\src\Perla\CliMiddleware.fs:line 76
   at Perla.CliMiddleware.MiddlewareImpl.esbuildPluginCheck@80-1.MoveNext() in C:\Users\scyth\repos\Perla\src\Perla\CliMiddleware.fs:line 108
   at Perla.CliMiddleware.MiddlewareImpl.templatesCheck@192-1.MoveNext() in C:\Users\scyth\repos\Perla\src\Perla\CliMiddleware.fs:line 197
   at Perla.CliMiddleware.MiddlewareImpl.esbuildBinCheck@156-1.MoveNext() in C:\Users\scyth\repos\Perla\src\Perla\CliMiddleware.fs:line 166
   at Perla.CliMiddleware.MiddlewareImpl.setupCheck@112-1.MoveNext() in C:\Users\scyth\repos\Perla\src\Perla\CliMiddleware.fs:line 121
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass12_0.<<UseHelp>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass22_0.<<UseVersionOption>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass19_0.<<UseTypoCorrections>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseSuggestDirective>b__18_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass16_0.<<UseParseDirective>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<RegisterWithDotnetSuggest>b__5_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass8_0.<<UseExceptionHandler>b__0>d.MoveNext()

To Reproduce Steps to reproduce the behavior:

Running perla new MyProject -t ff or perla new MyProject -id perla.templates.fable.feliz

Expected behavior It should not throw an exception in these cases

Desktop (please complete the following information):

Additional context

This report was quoted from @kaeedo and reported in issue #118

We Wrongly used Input.Option rather than Input.OptionMaybe in these two places

Changing them over to Input.OptionMaybe should fix this issue, although it may uncover other issues not foreseen when this was introduced.

I started adding tests to the parsing area but I sadly missed this use case, adding one for this command would be useful, example here:

kaeedo commented 1 year ago

I'll start looking into this. I imagine it's a simple way to start to get familiar with the codebase :)

AngelMunoz commented 1 year ago

released in beta 19