Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade from Winnow 0.4 to 0.5 #214

Merged
merged 89 commits into from
Dec 20, 2024
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Oct 28, 2024

The biggest change in this release is switching from Fn(I) -> Result<(I, O, E> to Fn(&mut I) -> Result<O, E>. This offers better ergonomics and performance. "Checkpoints" were introduced as a way to save off a specific location and return to it later, replacing the input juggling done before.

To make this major shift more doable, compatibility APis were added. parse_peek is like the new parse_next except it has the old signature. unpeek turns the old signature into the new one. These are terrible names but meant to be short-lived in a code base.

@epage epage marked this pull request as draft October 28, 2024 15:37
@GuillaumeGomez GuillaumeGomez changed the title Upgrade from Winnow 0.3 to 0.4 Upgrade from Winnow 0.4 to 0.5 Oct 28, 2024
@epage epage force-pushed the winnow-05 branch 3 times, most recently from 3247bba to f8ce339 Compare November 4, 2024 20:44
@epage epage force-pushed the winnow-05 branch 4 times, most recently from b017d2c to 3fd7521 Compare November 7, 2024 19:48
@epage epage force-pushed the winnow-05 branch 2 times, most recently from a1ac0fa to e229746 Compare November 11, 2024 15:28
@epage epage force-pushed the winnow-05 branch 4 times, most recently from 739839b to 36601b1 Compare November 25, 2024 22:24
@epage epage force-pushed the winnow-05 branch 2 times, most recently from f54b2fa to a0614ce Compare December 19, 2024 20:46
This leaves room for a new `ParseResult` that doesn't have the input as
we transition to the Winnow 0.5 API.
In 0.4, `parse_next` and `parse_peek` are the same.
In 0.5, `parse_next`s signature will change.
By switching to `parse_peek` now, we can make it so we gradually adopt
the new `parse_next` signature.
In 0.4, `unpeek` is a no-op.
In 0.5, the `FnMut` signature impled for `Parser` changes and `unpeek`
will take the old signature and adapt it to work.
This allows us to more gradually migrate to the new `FnMut` signature.

Yes, the name `unpeek` isn't ideal but its only transitional.
Note: `tests::fuzzed_target_recursion` fails on debug builds because of
the extra overhead from the deprecated APIs adapting to their
non-deprecated forms
@epage epage marked this pull request as ready for review December 20, 2024 15:46
@epage epage force-pushed the winnow-05 branch 2 times, most recently from 2a92482 to 341ce9a Compare December 20, 2024 15:53
@GuillaumeGomez
Copy link
Contributor

Looks good to me, thanks a lot!

@GuillaumeGomez GuillaumeGomez merged commit 47a7334 into rinja-rs:master Dec 20, 2024
28 checks passed
@epage epage deleted the winnow-05 branch December 20, 2024 16:37
@Kijewski
Copy link
Collaborator

Thank you very much for your work! This PR was huge.

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