fsprojects / Argu

A declarative CLI argument parser for F#
https://fsprojects.github.io/Argu
MIT License
451 stars 75 forks source link

feat(ParseResults): Alias *PostProcessResult* -> *GetResult* #211

Closed bartelink closed 6 months ago

bartelink commented 8 months ago

Preamble:

I have used Argu for many years but PostProcessResult always scared me (and I had not seen it organically).

The naming put me off - it sounds like something some metaprogrammer genius would be doing in some esoteric scenario.

In recent times, I actually saw/looked at the tutorial for the first time, and realised it was actually quite useful (I used to do Option.map and think no more of it, but having exceptions be trapped and reported wrt that specific argument is very desirable)

The other issue is that short names matter


Now that there is a GetResult with a fun () -> <default>, it would seem some additional overloads would be useful:

The main questions for me are:

I propose to:

nojaf commented 8 months ago

Hi, this proposal seems reasonable.

I don't have the biggest issue with the current naming after reading the tutorial. But I don't think I've used this anywhere myself, so I'm probably not a good candidate to offer feedback here.

What is the benefit of passing the mapping to Argu instead of piping the result afterwards?

let x = args.GetResult(Thing) |> mapThing

I'm just trying to understand things better.

dawedawe commented 8 months ago

Hey @bartelink , thanks for making me aware of PostProcessResult. I might have a use case that could profit from it 😃

bartelink commented 8 months ago

What is the benefit of passing the mapping to Argu instead of piping the result afterwards?

Key difference is that exceptions are trapped and mapped to ArguParseException (the existing Catch method enables that generically, and anything that takes a function traps it similarly) - and it's contextual, i.e. if defaulting for some nested item needs to say "we can't load this secret", you're not producing a top level help message only

The main question here is whether to focus on making the API intuitive for intellisense discovery should matter, as its never ideal to change names

The other thing in play is that these defaulting / fallback mechanisms is a lower level tactical mechanism - it's conceivable that #107 / #65 / #143 (if implemented) might dictate another mechanism and render this a point solution that distracts from a higher level approach

bartelink commented 6 months ago

Am thinking I'm going to do a v6.2 PR fairly soon that adds the GetResult overloads that alias PostProcessResult per the OP here

And add a Factory method that does command line parsing only with more general program name derivation (see https://github.com/fsprojects/Argu/issues/171#issuecomment-1951184161)

Then in V7 the PostProcess stuff gets obsoleted, and in V8 they get removed

If anyone else has things that are confusing and/or want to pave the way for future changes, it might be useful to do them as a batch in a 6.2 release cc @numpsy