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

Add Parser implementation for nom::combinator::value #1829

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

althonos
Copy link

@althonos althonos commented Feb 24, 2025

Salut Geoffroy!

This PR adds a Parser implementation for nom::combinator::value, so that the underlying parser is always run in Check mode and the parser value is never emitted, as it would be replaced by the user-provided value anyway.

The old code is doing:

pub fn value<I, O1: Clone, E: ParseError<I>, F>(
 where
   F: Parser<I, Error = E>,
 {
    parser.map(move |_| val.clone())
 }

which basically delegates to using the Parser::map implementation. Parser::map maps the closure to the output of the parser, which means that the parser output must always be emitted. But since in the case of value we are gonna ignore the result anyway, we can actually always run the underlying parser in Check mode.

I added a dedicated Value<F, O1> struct with he following implementation to avoid emitting the inner parser value (similarly to preceded, etc.):

impl<I, O1, E, F> Parser<I> for Value<F, O1>
where
  F: Parser<I, Error = E>,
  O1: Clone,
  E: ParseError<I>,
{
  type Output = O1;
  type Error = E;
  fn process<OM: OutputMode>(
      &mut self,
      input: I,
    ) -> PResult<OM, I, Self::Output, Self::Error> {
      self.parser.process::<OutputM<Check, OM::Error, OM::Incomplete>>(input)
        .map(|(i, _)| ( i, OM::Output::bind(|| self.val.clone()) ))
    }
}

I don't think the value combinator is tested anywhere outside of the doctests, let me know if I should add some tests as well.

@althonos althonos requested a review from Geal as a code owner February 24, 2025 16:49
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.

1 participant