shnewto / bnf

Parse BNF grammar definitions
MIT License
256 stars 22 forks source link

mermaid formatting of parse trees #99

Closed CrockAgile closed 2 years ago

CrockAgile commented 2 years ago

This adds mermaid output for parse trees. Honestly, not sure if useful enough to warrant being merged. But it sure is nice to include in github docs, such as PRs!

flowchart TD
0["sum"] --> 1["sum"]
1["sum"] --> 2["product"]
2["product"] --> 3["factor"]
3["factor"] --> 4["number"]
4["number"] --> 5["digit"]
5["digit"] --> 6["1"]
0["sum"] --> 7["add"]
7["add"] --> 8["+"]
0["sum"] --> 9["product"]
9["product"] --> 10["factor"]
10["factor"] --> 11["("]
10["factor"] --> 12["sum"]
12["sum"] --> 13["sum"]
13["sum"] --> 14["product"]
14["product"] --> 15["product"]
15["product"] --> 16["factor"]
16["factor"] --> 17["number"]
17["number"] --> 18["digit"]
18["digit"] --> 19["2"]
14["product"] --> 20["mult"]
20["mult"] --> 21["*"]
14["product"] --> 22["factor"]
22["factor"] --> 23["number"]
23["number"] --> 24["digit"]
24["digit"] --> 25["3"]
12["sum"] --> 26["add"]
26["add"] --> 27["-"]
12["sum"] --> 28["product"]
28["product"] --> 29["factor"]
29["factor"] --> 30["number"]
30["number"] --> 31["digit"]
31["digit"] --> 32["4"]
10["factor"] --> 33[")"]

Implementation

each node gets assigned a unique, auto incrementing ID. But instead of displaying that ID in the Mermaid diagram, use the Term string.

Dangers

I do think this draft implementation can produce invalid input. Because BNF allows for arbitrary tokens in Terms, Mermaid expects some of those characters to be escaped.

I am not sure if the right next step is to build a "fully compliant" Mermaid filter layer, or maybe just accept that some grammars may not build parse trees that Mermaid can parse.

shnewto commented 2 years ago

😍 🤩 This is so cool!

As far as your concerns about the danger of running into broken graphs when the grammar and the mermaid spec don't agree, my take is that we're good. I appreciate the the mermaid Display is opt in so we won't break the implicit impl Display for anyone. And imo this is really a documentation nicety (a very nice nicety) and so I think we can merge it as is because it has immediate benefit and if some sharp edges are eventually spotted we can address them then.

One note, it looks like your branch is out of date with main, I believe if you sync up with it again your build will succeed.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.08%) to 92.316% when pulling 5ddbb8e9425ce5c5961d0a8c843053df54dfd7a2 on mermaid into 8215f061433957712c5217917ab0aa3153d463f0 on main.