Add PairsBuilder to construct Pairs without a parser - #1194
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughAdds and publicly exports ChangesPairsBuilder construction
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This additive change introduces a builder for constructing parser-like pairs without changing existing behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tomtau
left a comment
There was a problem hiding this comment.
thanks! Could you rebase it on top of the latest master?
One other thing I'm thinking is that given 1. this is not used anywhere inside the pest crates, 2. it's meant for testing (without invoking the parser), would it make sense to feature-guard under a "test" feature flag? (I know fails_with and parses_to macros are kind of in the same category, but they are used within pest tests)
Testing code that consumes parser output (a function taking `Pair` or `Pairs`) currently requires running a real parse just to obtain tokens of the right shape, which couples such unit tests to the grammar. `PairsBuilder` lets the expected token tree be described directly: each pair is given a rule and a `[start, end)` span into the input, with inner pairs added through a closure and node tags attached via `tag`. `build` flattens the tree into the same `Start`/`End` queue the parser produces, so the resulting `Pairs`/`Pair` behave identically (`as_str`, `line_col`, `into_inner`, `tokens`, tag lookups). Spans are validated against the input on `build`. Closes pest-parser#469
3f5b700 to
c89d043
Compare
|
Rebased onto the latest master, just the one CI toolchain commit on top of the old base, no conflicts. On the feature flag, good question. A couple of thoughts. I'd steer away from A regular cargo feature that's off by default is the right mechanism if the goal is keeping it out of the default build. One caveat worth naming: cargo feature unification means that if a crate pulls pest in both as a normal dependency and as a dev-dependency with the feature on, the feature ends up enabled for the normal build too. So it isn't a hard guarantee, it just keeps the code out for anyone who never turns it on anywhere. My weak preference is to leave it ungated, for two reasons: |
Closes #469.
Problem
Code that consumes parser output usually takes a
Pair<'_, Rule>orPairs<'_, Rule>. Unit-testing such code today means running a real parse first, just to get tokens of the right shape — which couples the test to the grammar and makes it awkward to exercise edge cases (a specific nesting, a particular tag) in isolation. #469 asks for a simple way to hand-build aPairfor exactly this.What this adds
A
PairsBuilderinpest::iteratorsthat describes the expected token tree directly and turns it into realPairs:new(input)— start a builder over the input string.rule(rule, start, end)— append a leaf pair spanninginput[start..end](same[start, end)byte convention asSpan).rule_with(rule, start, end, |inner| ...)— append a pair whose inner pairs are built by the closure.tag(tag)— attach a node tag to the most recently appended pair (sofind_tagged/as_node_tagwork).build()— flatten into the sameStart/Endqueue the parser produces and returnPairs.Because it reuses the existing queue representation, the resulting pairs behave exactly like parsed ones —
as_str,as_span,line_col,into_inner,tokens, and the tag lookups all work unchanged.build()validates every span against the input (ascending range, on UTF-8 char boundaries), mirroring the invariantSpan::newenforces, and panics with a descriptive message otherwise.Notes
pest::iterators::PairsBuilder; nothing else changes.no_std-friendly (onlyalloc), consistent with the rest of the crate.line_col, multibyte spans, the empty builder, and the panic paths; docs include runnable examples.cargo fmt --checkandcargo clippyare clean.The method names / shape are of course open to bikeshedding — happy to adjust naming or the leaf-vs-
rule_withsplit if you'd prefer different ergonomics.Summary by CodeRabbit