feat: argparse

[peter-jerry-ye]

This PR adds an arg parser that is inspired by clap but with reduced API because we want to focus on the clarity of the behavior. A lot of extra checks were imposed.

The API of this PR is roughly determined, but the internal implementation can still be improved drastically.

@FlyCloudC Do you think you would like to give it a review in terms of the API? (I know the internal implementation is a bit messy).

---

[coveralls]

Pull Request Test Coverage Report for Build 2734

Details

• 858 of 1012 (84.78%) changed or added relevant lines in 19 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.9%) to 95.679%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
argparse/runtime_exit_native.mbt	0	1	0.0%
argparse/runtime_exit_wasm.mbt	0	1	0.0%
argparse/error.mbt	17	19	89.47%
argparse/parser_suggest.mbt	35	37	94.59%
argparse/runtime_exit_js.mbt	0	2	0.0%
argparse/parser_lookup.mbt	25	30	83.33%
argparse/command.mbt	47	53	88.68%
argparse/runtime_exit.mbt	0	6	0.0%
argparse/help_render.mbt	156	165	94.55%
argparse/parser_positionals.mbt	31	41	75.61%

Totals
Change from base Build 2730:	-0.9%
Covered Lines:	12932
Relevant Lines:	13516

---

💛 - Coveralls

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 7 additional findings.

[FlyCloudC]

I have the following suggestions:

The parameter (args: Array[&ToArg]) in Command::new could be replaced with (flags: Array[FlagArg], options: Array[OptionArg], positionals: Array[PositionalArg])(or flags and options could be merged into nameds : Array[&ToArg]). This avoids mixing three different types of parameters together. After all, these three are not grouped together in the help display either. Additionally, this approach eliminates the need to write package names since the types are already determined.

Another optional suggestion is to consider merging the three parameters (required, env, and default_values) using sum type modeling (though the current approach seems acceptable as well)

[FlyCloudC]

A question: Why are errors generated when building a Command deferred until argument parsing time to be raised?

It would be better if they could be raised immediately. I mean: changing the signature of Command::new to raise ArgBuildError.

[FlyCloudC]

Positional::new already accepted num_args, so there is no need to accept required

[peter-jerry-ye]

I have the following suggestions:

The parameter (args: Array[&ToArg]) in Command::new could be replaced with (flags: Array[FlagArg], options: Array[OptionArg], positionals: Array[PositionalArg])(or flags and options could be merged into nameds : Array[&ToArg]). This avoids mixing three different types of parameters together. After all, these three are not grouped together in the help display either. Additionally, this approach eliminates the need to write package names since the types are already determined.

Another optional suggestion is to consider merging the three parameters (required, env, and default_values) using sum type modeling (though the current approach seems acceptable as well)

The rational was to avoid making them to verbose, because we would have

Command(flags=[FlagArg(xxx)], options=[OptionArg(xxx)])

instead of having just

Command(args=[FlagArg(xxx), OptionArg(xxx)])

[peter-jerry-ye]

A question: Why are errors generated when building a Command deferred until argument parsing time to be raised?

It would be better if they could be raised immediately. I mean: changing the signature of Command::new to raise ArgBuildError.

The rational is that the final check involves checking the configuration from the parent + the subcommand. And it is not expected to be raised too often because when we finish defining the arg parser, it is likely to be stable. I think for clap in Rust it's actually checked only under debug mode.

[FlyCloudC]

I have the following suggestions:
The parameter (args: Array[&ToArg]) in Command::new could be replaced with (flags: Array[FlagArg], options: Array[OptionArg], positionals: Array[PositionalArg])

The rational was to avoid making them to verbose, because we would have

Command(flags=[FlagArg(xxx)], options=[OptionArg(xxx)])

instead of having just

Command(args=[FlagArg(xxx), OptionArg(xxx)])

Actually, here's how it looks when users use it:

@argparse.Command(args=[@argparse.FlagArg(xxx), @argparse.OptionArg(xxx)])

instead of

@argparse.Command(flags=[FlagArg(xxx)], options=[OptionArg(xxx)])

Unless the user imports it with using

[FlyCloudC]

A question: Why are errors generated when building a Command deferred until argument parsing time to be raised?
It would be better if they could be raised immediately. I mean: changing the signature of Command::new to raise ArgBuildError.

The rational is that the final check involves checking the configuration from the parent + the subcommand. ...

I'm not entirely sure what you mean. Do you mean that this situation might exist?

let sub1 = Command(...) // invalid
let sub2 = Command(...) // invalid
let parent = Command(..., subcommands=[sub1, sub2]) // valid

So the error cannot be raised in the first two lines

[FlyCloudC]

A question: Why are errors generated when building a Command deferred until argument parsing time to be raised?
It would be better if they could be raised immediately. I mean: changing the signature of Command::new to raise ArgBuildError.

The rational is that the final check involves checking the configuration from the parent + the subcommand. And it is not expected to be raised too often because when we finish defining the arg parser, it is likely to be stable. I think for clap in Rust it's actually checked only under debug mode.

I just thought about it again, and I believe ArgBuildError is not an Error. If it occurs, it indicates a bug in the program that defines the Command (which shouldn't exist in a released CLI tool) and is unrecoverable, so it should just abort instead.

[peter-jerry-ye]

@FlyCloudC I agree. Having &ArgLike makes it more verbose. I also agree that it should be a panic, but I would like to leave it for the user to handle.