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

Added map_with and try_map_with to replace existing mapper functions #537

Merged
merged 5 commits into from
Oct 8, 2023

Conversation

zesterer
Copy link
Owner

@zesterer zesterer commented Oct 8, 2023

There are big changes in here! For those reading this PR because they're not sure why their build has broken:

The problem

This PR is an attempt to remove all of the arbitrary map_<whatever> methods on Parser. Chumsky has managed to grow a lot of them, such as:

  • map_with_span
  • map_span
  • map_with_state
  • map_with_ctx

Additionally, we had the following 'same but with extra stuff' methods:

  • foldl_with_state
  • foldr_with_state
  • try_map_with_state

Despite all of these extra methods to remember, we still often hit annoying limitations. For example, it was really common for users to have to invent their own spans by combining existing spans in foldl/foldr functions, despite the fact that chumsky really should be able to produce a span on the fly. It was also common to see users needing to screw around with multiple mapping functions one after the other to get access to the information that they actually needed.

The solution

To solve all of this, we're replacing the zoo of methods above with the following four:

  • map_with
  • foldl_with
  • foldr_with
  • try_map_with

These functions are all nice and consistent. They work exactly like their easier equivalents (map, foldl, foldr, try_map), except that you get an extra argument, a MapExtra. This is a type with a few useful methods that you can call on it:

  • extra.span(): get the span of the parsed output
  • extra.slice(): get the slice covering the parsed output
  • extra.state(): get access to the parser state (ignore this if you've never needed parser state)
  • extra.ctx(): get access to the parser context (ignore this if you've never needed to do context-sensitive parsing)

This should massively improve the flexibility of the mapping functions and bring an end to the need to keep a dozen arbitrary mapping functions in your head!

Other changes

In addition, there have been a few other changes:

  • The second argument of validate's closure is a MapExtra (see above) instead of a Span (you can just do extra.span() in the closure to get the old behaviour back)
  • Parser::slice has been renamed Parser::to_slice so as to be consistent with Parser::to and Parser::to_span.

Migrating

In most cases, the changes involved should be trivial: just switch the old combinators to their _with counterpart. You might find the changes to the nano_rust.rs example in this PR to be instructive!

@zesterer zesterer merged commit 3142012 into main Oct 8, 2023
@zesterer zesterer mentioned this pull request Oct 10, 2023
35 tasks
@stefnotch
Copy link
Contributor

We still have two leftover map_with_spans

.map_with_span(|tt, span| (tt, span))
and
/// .map_with(|ident, e| Spanned(ident, e.span())) // Equivalent to `.map_with_span(|ident, span| Spanned(ident, span))`

@zesterer
Copy link
Owner Author

We still have two leftover map_with_spans

The former case is in commented code which might well be removed since it uses an old combinator, the latter is showing equivalence with the old API, so I think both are fine for now. Thanks for mentioning though!

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.

2 participants