nimbleparse: add -d flag to dump state graph #474
Conversation
nimbleparse/src/main.rs
Outdated
|
|
||
| if dump_state_graph { | ||
| println!("Stategraph:\n{}\n", sgraph.pp_core_states(&grm)); | ||
| process::exit(0); |
There was a problem hiding this comment.
My suggestion is that we don't exit here: we print out the stategraph and continue. That simplifies the argument parsing in the sense that users always have to provide .l, .y, and input files. We could be more clever, but documenting that in a digestible way becomes a pain, and I think the simple approach is adequate.
There was a problem hiding this comment.
Yeah the problem with my testing was that I didn't actually have valid input for any of the github provided parsers I was testing, while it is likely that /dev/null would be a valid for most perhaps even all of them, it was easier to just avoid complications with input/AST production.
Perhaps that is a special case that is no longer necessary, and I suppose it is easy to add the exit call if I need something like that again?
There was a problem hiding this comment.
That particular use case would probably best be covered by an lrtable binary: it would naturally only take in the grammar (and I suppose optionally a lexer). There is precedence here: we do build an lrlex binary for this sort of testing.
There was a problem hiding this comment.
That could be good, there were a couple of actually the most heavily used libraries depending upon grmtools
which I couldn't test with this approach because they used a manually implemented lexer.
There was a problem hiding this comment.
I think this PR is useful (without the exit) and worth keeping, but an additional lrtable binary PR would be cool too!
There was a problem hiding this comment.
Should be fixed in 2a13f24
There was a little more to it, also needed to avoid duplicate printing of the state graph when there are conflicts.
nimbleparse/src/main.rs
Outdated
| eprintln!("{}", msg); | ||
| } | ||
| eprintln!("Usage: {} [-r <cpctplus|none>] [-y <eco|grmtools|original>] [-q] <lexer.l> <parser.y> <input file>", leaf); | ||
| eprintln!("Usage: {} [-r <cpctplus|none>] [-y <eco|grmtools|original>] [-q] [-d] <lexer.l> <parser.y> <input file>", leaf); |
There was a problem hiding this comment.
In ye olde unix convention, this would be [-dq].
|
Please squash. |
ratmice
force-pushed
the
dump_state_graph
branch
from
2a13f24 to
b68a9be
Compare
|
Squashed. |
2 participants
This is a patch I used for testing softdevteam/sparsevec#25
It adds a
-dflag for dumping the state graph from nimbleparse, then exiting.It also fixes an issue where the pretty printing function produces non-deterministic output.
I'm not certain it is quite up to par, because of the argument parsing an input file is required even though it is never used.
but making the input file 'required unless the -d flag is specified' is also weird. Passing in an unused filename like /dev/null was sufficient for my testing purposes though.