Skip to content

Conversation

@therealbnut
Copy link

I found this made it easier for me to read the scopes of each rule. I think it could be improved more, but I'm finding it useful so I thought I'd share it back.

Essentially it changes trace statements so they better reflect their hierarchy.

[PEG_TRACE] Attempting rule `atom_type` at 3:21 {
[PEG_TRACE]   Attempting rule `identifier` at 3:21, matched `identifier` from 3:21 to 3:32
[PEG_TRACE]   Attempting rule `ws` at 3:32, matched `ws` from 3:32 to 3:32
[PEG_TRACE]   Attempting rule `parse_char` at 3:21, failed `parse_char` at 3:21
[PEG_TRACE]   Attempting rule `parse_char` at 3:21, failed `parse_char` at 3:21
[PEG_TRACE]   Attempting rule `parse_string` at 3:21, failed `parse_string` at 3:21
[PEG_TRACE] } failed `atom_type` at 3:21

@therealbnut
Copy link
Author

Seems like the cache causes some mutability issues, this might need some revision.

@Mingun
Copy link

Mingun commented Feb 7, 2021

This will fix #236, but as @kevinmehall suggested in my issue, using https://github.com/fasterthanlime/pegviz is much better for debug rules.

@kevinmehall
Copy link
Owner

Nice!

As you've discovered, the existing approach of conditionally adding a bunch of code to the generated parser is kind of messy and tricky to get right especially in cases like caching. And while the trace format wasn't really intended to be a stable interface, things like pegviz are parsing it. So I'm reluctant to merge something like this as-is, but it's a good starting point for a discussion on what tracing in rust-peg would ideally look like.

If we're going to do a breaking change to tracing in 0.7, what might be better is providing a ParseTrace trait with methods for each trace event, and a way to pass a ParseTrace into the parser. That way, different implementations could provide nice text output like this, JSON output (#141), or one might possibly even be able to use it to build a concrete syntax tree directly rather than embedding Rust code in the grammar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants