Better argument parser

[Argmaster]

Description

This pull request introduces prototype of declarative, fully comptime argument parser based on introspection. The goal is to use to declaratively describe command syntax and automatically provide autocompletion for each of declared segments.

/tp command has been migrated to Parser based design as proof of concept.

Example

Declaration of parser for /tp command:

const Args = union(enum) {
	@"/tp <x> <y> <z>": struct {x: f64, y: f64, z: ?f64},
	@"/tp <biome>": struct {biome: main.argparse.BiomeId(true)},
};

const ArgParser = main.argparse.Parser(Args, .{.commandName = "/tp"});

Links

Related to: #1380

[Argmaster]

TODO: "unpack" structs

[IntegratedQuantum]

I need @IntegratedQuantum to decide if we keep callbacks as they are, do we try passing arbitrary arguments to them through parse call, or do we remove them completely.

Is this is still relevant? It seems like you removed the callbacks already.

[IntegratedQuantum]

Oh right now I see it, I was only looking at the reference implementation so far.
Yeah, we can just remove it.

[Argmaster]

Yee sorry I figured I will just remove and restore it if requested since it wasn't really usable in form that was included.

[Argmaster]

I also returned this PR to draft state because I figured I should also try how well autocompletion will fit into that design.

[IntegratedQuantum]

Hmm, this function does seem quite easy to miss. It seems you missed it in the reference implementation too.

Maybe a better pattern would be to pass in the result message List. This would allow using regular errors, with their syntax sugar, as well as moving this up one layer into the command code, so we could do the error message handling there. Either way, that would be a bigger rewrite, and doesn't need to be done here.

[Argmaster]

Noted (and fixed), but I don't fully understand the request unfortunately, I guess we will return to this topic. What I would like to preserve is the fact that this parser can be used both for commands and for CLI parameters for Cubyz executable if useful at some point.

[Argmaster]

Oh I see I didn't push the removal of the callback parameter, sorry for my confusion.

[Argmaster]

Idk the message could be better tbh
We could incorporate enum name of given alternative interpretation, then it would explain that this too many arguments was for tp to biome interpretation.

[Argmaster]

IMO much better now:

Although the "Provided argument list..." part is a bit too verbose IMO, any suggestions?
The offset information is also redundant, count should be good enough.

[IntegratedQuantum]

Idk the message could be better tbh

I'd suggest

Couldn't match argument list
/tp <x> <y> <z>
 Expected a number at argument 1 found "f" ← Note that normal people start counting at 1
/tp <biome>
 Invalid biome id "f"

Also if there is only one argument list, I'd suggest to only show the error message without all the extra information.

[Argmaster]

Ewww being normal
Who invented that

[Argmaster]

This kind of message with all the variants is only shown for unions, if there is only a struct with one set of arguments it will stop on first error and will report it.

[Argmaster]

I would leave those, otherwise it is less readable, mind that adding a leading space will not work as good as in Github UI because chat is mach narrower and message is almost guaranteed to wrap.
Mind that system has to be generic, eg. BiomeId can be on different position in different use case, so it should print its position even if it is only position in the command.

[Argmaster]

[Argmaster]

Okay, I applied the requests. I tried returning optional to indicate that it was not possible to parse the string, but that failed miserably, so now I am returning ParseError instead and it seems to work.

[codemob-dev]

Is this still active?

[IntegratedQuantum]

It is waiting for review

[Argmaster]

Closing until integrating such feature becomes a priority.

[thecreare]

Expected a number at argument 1 found "f" ← Note that normal people start counting at 1

To appease both parties, programmers and ""normal people"", we should follow the GulfOfMexico standard and start counting at -1

[IntegratedQuantum]

Alright, one thing is for sure I don't have the time and patience to review a 500 line PR.
First up instead of using /tp, I'd suggest to use a simpler command such as /time for demonstration. /time is a lot simpler but still has enough cases to be interesting.
With that the biome code is no longer needed, and I think it should be added in a future PR instead.

[IntegratedQuantum]

All local allocations should use the stackAllocator

@codemob-dev we need the fix for #1286

[codemob-dev]

@codemob-dev we need the fix for #1286

Not again...

[IntegratedQuantum]

also not needed with #1286

[Argmaster]

Aight, there is nothing I can do about the allocator, so that is still pending, but I have removed tp and biome changes and implemented an example based on time command.
That exposed some issues in field name concatenation, so I tried to improve how this is handled.
I also noticed an issue with handling an empty argument list and/or trailing whitespace, so I also handled those in a more human friendly way.
I agnowledge certain weirdness in how messages for enum values are formulated - descriptions don't explicitly mention names of the arguments, while the error messages do. However, there has to be a time to say enough and I think this is it.