Skip to content
This repository was archived by the owner on Apr 2, 2026. It is now read-only.

feat: impl From<ExpectedNumber> for RichPattern#866

Open
seftontycho wants to merge 1 commit intozesterer:mainfrom
seftontycho:add_rich_error_support_for_numbers
Open

feat: impl From<ExpectedNumber> for RichPattern#866
seftontycho wants to merge 1 commit intozesterer:mainfrom
seftontycho:add_rich_error_support_for_numbers

Conversation

@seftontycho
Copy link
Copy Markdown

Currently it is not possible to use the number::Number parsers with the Rich error type as RichPattern does not implement From<ExpectedNumber>.

This PR adds the missing impl in the number module so it is appropriatley feature gated.

Not previously possible:

fn parse_u8<'a>() -> impl Parser<'a, &'a str, u8, extra::Err<Rich<'a, char>>> {
    let u8_num: Number<STANDARD, &'a str, u8, ErrRich<'a>> = number();
    u8_num
}

@seftontycho
Copy link
Copy Markdown
Author

I'm not exactly sure what variant of RichPattern to use here, Label seemed the most fitting but then I'm not sure on the label name.

Open to suggestions better than "number".

@zesterer
Copy link
Copy Markdown
Owner

Thanks! I think adding a new variant to RichPattern is fine also, and might be worth it for the ability to disambiguate at a type level (we don't want end users pattern-matching on strings, if we can help it). If you could, also added #[non_exhaustive] to the enum would also be great. It's a breaking change, but there are already several of those lined up anyway!

@seftontycho
Copy link
Copy Markdown
Author

seftontycho commented Aug 18, 2025

Thanks! I think adding a new variant to RichPattern is fine also...

How would you like that to work feature wise? If we added RichPattern::Number it wouldn't really make sense if a user wasn't using the lexical-numbers feature.

@zesterer
Copy link
Copy Markdown
Owner

Apologies, GitHub swallowed the notification for this.

I think the only change needed is to add #[non_exhaustive] to the RichPattern enum.

@zesterer zesterer force-pushed the main branch 2 times, most recently from 0f2b61a to e350fc6 Compare November 5, 2025 19:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants