edgedb-cli
edgedb-cli copied to clipboard
Feat: Variable parsing rework (Array input)
Summary
This PR reworks the variable input mechanism to use nom
as a parser; allowing us to parse array inputs and other complex inputs.
Array Inputs
The array input works much like writing an array in EdgeQL, as proposed by Sully:
cli> select <array<int64>>$arg;
Parameter <array>$arg: [1, 3, 5]
{[1, 3, 5]}
and it also forces str
to be quoted with either single quotes or double quotes in the input:
cli> select <array<str>>$arg;
Parameter <array>$arg: ["abc", 'def']
{['abc', 'def']}
Error reporting and hints
With the change to nom, errors and hints have changed too as a side effect, in the current state of this PR it dumps the parser errors pretty verbosely, I would like some feedback on how we should format these errors to be as concise as possible, some examples:
Things get pretty messy with some of the parsers though:
Can you add a few examples of how successful input looks like? What if you have an array of tuples which have arrays in them?
We definitely shouldn't be including things like TakeWhileMN
in error messages
We need to figure out how to make check warnings not appear once for each platform
(Get rid of all the unused imports)
Can you add a few examples of how successful input looks like? What if you have an array of tuples which have arrays in them?
Array of tuples
Array of tuples with arrays
Array of json
The big remaining thing needed is tests. There probably ought to have been some tests before, but now the parsing is much more involved so we definitely need them.
Added named tuples, might wanna gloss over it
Oh, clean up the unused imports please
Do the tuples and array support trailing commas? They should, and we should make sure to test it
Do the tuples and array support trailing commas?
No, any extra comma errors. I'll change that
Hm, actually now that I think about it more, maybe we should require the named tuples to be in order, also? That matches edgeql better (where the order is important), though (for example) the Python bindings will accept dictionaries and namedtuples without caring about order. @elprans thoughts on this?
We should probably also support writing non-named tuples when a named tuple is expected also, which I now realize is what @elprans was probably saying earlier
Oh, and leading spaces after an opening (
or [
We should probably also support writing non-named tuples when a named tuple is expected
this will be exclusive right? ex: cant have 1 named and 1 unnamed part; its either all named or no named?
We should probably also support writing non-named tuples when a named tuple is expected
this will be exclusive right? ex: cant have 1 named and 1 unnamed part; its either all named or no named?
Yeah
I just pushed commits that support trailing commas for tuples, named tuples, and arrays. Mostly I did this as a personal experiment with nom
.
The combinator-y tuple that works by building up a new boxed parser isn't too bad. It took some fiddling because I'm not that good at rust anymore, but is basically reasonable, I think.
The array/namedtuple version I found very frustrating. I implemented a function trailing_separated_list0
, with the intention that it behave like separated_list0
but allow trailing separators. This worked but I wasn't able to figure out a way to do it purely by composing combinators, and needed to invoke the parsers directly, which cuts against the benefit of parser combinators somewhat. The final result isn't actually that bad, though, just unsatisfying.
@quinchs what's the status of this?
@quinchs what's the status of this?
I've got some remaining changes to fix with it which sully has mentioned.