Feature/async parse args experiment

[shadowspawn]

This is an experiment based on concepts in #1902 to support async Option.parseArgs and Arguments.parseArgs. Blindly store the return value from async calls to parseArgs as usual (although actually a pending promise), and then settle the promises in a batch before proceeding to next stage of parsing. In particular, settle the promises before calling user hooks or action handler so client code is unlikely to see the promises.

Simplified approach compared to #1902, here:

• no configuration
• built-in support rather than using hooks
• handling previous value promises from repeated calls to parseArgs left to client

---

Reminder to add co-author credit if proceed with this PR.

Co-authored-by: Wee Bit <[email protected]>

[aweebit]

What about checking for whether parseAsync() has been called? Don't we want to leave the option for the user to handle promises returned from parsers manually at least when parse() is used like in my original example in #1900, and thus make the change non-breaking at least for parse() calls?

[shadowspawn]

Good question. From that point of view, either the change is breaking or non-breaking. It does not matter whether parseAsync has been called.

One way of shipping it in a minor version would be to make it opt-in, before making it always-on in the next major version.

A possible fallback is to just add an example file for now! It all looked nice and easy for limited use case like: #1900 (comment)

[aweebit]

Yeah, I know it is going to be breaking in the end anyway, I just meant that by checking for parseAsync(), we could ensure nothing changes from users' perspective at least when parse() is called. After all, a user might want to handle promises returned from parsers manually, but by awaiting them even if parse() is used, we not only deprive them of this option, but also introduce the risk of unhandled promise rejections.

[aweebit]

I have submitted a new PR (#1915) which gets rid of all the complexity due to hook usage in #1902 but keeps all the parts missing in your solution that I think are necessary (the motivation is explained in detail in the new PR's description).

[shadowspawn]

Added support for multiple calls to parseArgs to resolve previous value (known as chainArgParserCalls in #1915). This is what the user would probably expect, and I mainly left it out until now because it is tricky to implement!

[shadowspawn]

After all, a user might want to handle promises returned from parsers manually, ...

I think we probably only need to support built-in async. If the user wants some variant of the promise resolving they can move the async processing to a hook, so not blocked. Less clever but less magic too.

[aweebit]

refineError() throws, so there will be an unhandled promise rejection. The result of the catch() call needs to be assigned to result to prevent it.

But if previous is already thenable, it is better to call catch() in the previous.then() callback (and return the result). Otherwise, catch() might be called on an already failed promise chain in which an error has already been caught and rethrown (because that is what refineError() does), so refineError() would be called again which does not make sense.

[aweebit]

Also, I think calling result.then(x => x, refineError) instead of result.catch(refineError) is more robust.

[shadowspawn]

The result of the catch() call needs to be assigned to result

Oops, yes, good catch.

Otherwise, catch() might be called on an already failed promise chain in which an error has already been caught and rethrown

I think this is correct pattern, but I'll try it before explaining my reasoning.

Also, I think calling result.then(x => x, refineError) instead of result.catch(refineError) is more robust.

Because we only test for .then()? Yes, I have thought about that in past and then forgot. I want to just assume they are promise-like since the primary use-case is for async, but it isn't hard to only rely on then and cover a few more edge uses.

[shadowspawn]

But if previous is already thenable, it is better to call catch() in the previous.then() callback (and return the result). Otherwise, catch() might be called on an already failed promise chain in which an error has already been caught and rethrown (because that is what refineError() does), so refineError() would be called again which does not make sense.

Took me a while to understand this. I was expecting a long catch chain, but see it can be avoided. 👍

[aweebit]

Added support for multiple calls to parseArgs to resolve previous value (known as chainArgParserCalls in #1915). This is what the user would probably expect, and I mainly left it out until now because it is tricky to implement!

But there are use cases where the user might not want that, so making chaining configurable now suggests itself. That is why I kept chainArgParserCalls() in #1915.

[aweebit]

I think we probably only need to support built-in async. If the user wants some variant of the promise resolving they can move the async processing to a hook, so not blocked. Less clever but less magic too.

But all currently available hooks are only called after the available thenable option and argument values are resolved, so this does not seem like a good option to me. The user would have no access to the promises returned by async parsers at all, even when calling parse().

In its simplest non-magical form (independent from whether parse() or parseAsync() was called and inherited via copyInheritedSettings()), awaiting configuration requires 10 lines of code which I don't think is such a big deal.

• 1 line for property initialization in constructor
• 1 line for property inheritance in copyInheritedSettings()
• 4 lines for configuration function (await() in #1915)
• 2 lines for if statements in each of _settleOptionPromises() and _settleArgumentPromises()

But again, an explanation why a little "magic" is meaningful is given in #1915, with additional details in this comment.

[shadowspawn]

But all currently available hooks are only called after the available thenable option and argument values are resolved, so this does not seem like a good option to me. The user would have no access to the promises returned by async parsers at all, even when calling parse().

Yes, the work-around I was suggesting for what are hopefully rare cases was use synchronous parseArg.

I understand this is not the flexibility you have proposed.

[aweebit]

Yes, the work-around I was suggesting for what are hopefully rare cases was use synchronous parseArg.

Did not get it back then, but I see what you mean now 👍

[shadowspawn]

This is pretty cool, but I am not happy enough with it:

• uses an indirect approach to achieve async with reprocessing options (and args), not direct support for async functions
• not a nice place to slot in async for program which has neither action handler nor external command
• would add complexity to one of the longer routines and somewhat fragile routines

User can achieve most of the functionality themselves fairly easily because they know how their program is constructed.

I am currently thinking async action handlers would be a neat example, but not a great built-in feature.