gluegun icon indicating copy to clipboard operation
gluegun copied to clipboard

Coalesce boolean arguments to actual Boolean type instead of string

Open joshuayoes opened this issue 2 years ago • 5 comments

When we pass --key=false flag arguments to gluegun, we get { options: { key: 'false' } } instead of { options: { key: 'false' } }.

This isn't how I expect to consume these in commands. This can handled on the program level, but I feel like that it should be handled on the gluegun level since I can't imagine a use case where we'd want true or false to be a string

joshuayoes avatar Aug 25 '22 18:08 joshuayoes

I've added failing tests to demonstrate desired behavior, you can see a sample of those failed test runs here

joshuayoes avatar Aug 25 '22 18:08 joshuayoes

Only problem I see is that this would definitely be a breaking change in my eyes

joshuayoes avatar Aug 25 '22 18:08 joshuayoes

It also may be better to take a PR like this, where we could provide the yargs-parser object optionally, and then allow for opting into coalescing options

joshuayoes avatar Aug 25 '22 18:08 joshuayoes

Got some input from Jamon. Gluegun already doesn't have great type-safety guarantees on command parsing, so this quality of life improvement would be worth a potentially breaking change

joshuayoes avatar Aug 25 '22 18:08 joshuayoes

Screen Shot 2022-08-25 at 4 43 04 PM https://dev.azure.com/infinitered/gluegun/_build?view=runs

I think something changed about ts-node or typescript dependency between now and May.

I don't think these changes are causing this issue, but I will investigate more before merging

joshuayoes avatar Aug 25 '22 23:08 joshuayoes

Merged into #783 since this will be a breaking change too.

jamonholmgren avatar Sep 28 '23 17:09 jamonholmgren