Add converted for grammar to ebnf #6995
Conversation
edb/edgeql/parser/grammar/convert.py
Outdated
| pass | ||
|
|
||
|
|
||
| class EBNF_Literal(EBNF_Item): |
There was a problem hiding this comment.
Python style is no underscores in class names. I'd probably just drop the entire EBNF from the name
There was a problem hiding this comment.
Moved to separate file. (For now, will move things to tools later)
| if count == len(ebnf_productions): | ||
| break | ||
|
|
||
| # output |
There was a problem hiding this comment.
I think the best place for this code to live would be as a tool in edb/tools. There are a bunch of examples there, and then we'd run it like edb generate_ebnf or some such.
edb/edgeql/parser/grammar/convert.py
Outdated
|
|
||
|
|
||
| def expand_iso_ebnf(item: EBNF_Item) -> str: | ||
| if isinstance(item, EBNF_Literal): |
There was a problem hiding this comment.
This (and a bunch of other places) would be a perfect time to try out python's new pattern matching system (match)
edb/edgeql/parser/grammar/convert.py
Outdated
| class EBNF_Item: | ||
| pass |
There was a problem hiding this comment.
I'd recommend using either dataclass or else our own ast mechanism. (Probably dataclass).
You get to skip writing __init__, the attributes and their types are spelled out more cleanly, and you automatically get string formatting.
|
|
||
| def to_iso_ebnf(productions: List[EBNF_Production]) -> List[str]: | ||
| return [ | ||
| production.name + ' = ' + expand_iso_ebnf(production.item) + ';' |
There was a problem hiding this comment.
Consider using f-strings instead of doing string concatenation.
There was a problem hiding this comment.
(Though use your judgement about whether it's actually an improvement
There was a problem hiding this comment.
I thought this bit was a bit too short to use formatting.
edb/edgeql/parser/grammar/convert.py
Outdated
| def replace_repeatedly( | ||
| item: EBNF_Item, | ||
| funcs: List[Callable[[EBNF_Item], Tuple[EBNF_Item, bool]]] | ||
| ) -> EBNF_Item: |
There was a problem hiding this comment.
We aren't totally consistent about these things, but typically edgedb style for this kind of definition is to have the closing paren lined up with the def and the body indented one step. (This is what black does, among others)
edb/edgeql/parser/grammar/convert.py
Outdated
| return replace_repeatedly( | ||
| item, [ |
No description provided.