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

stipulate sep parser must consume #1723

Merged
merged 1 commit into from
Apr 21, 2024
Merged

stipulate sep parser must consume #1723

merged 1 commit into from
Apr 21, 2024

Conversation

gdennie
Copy link
Contributor

@gdennie gdennie commented Jan 27, 2024

separated_list1 by multipspace0 sep failed #1691

the sep parser in the separated_list parser combinators cannot be "any" parser but must be a consuming parser, contrary to documentation. Ideally, we should have a trait to assert this constraint as well but better clarity in the documentation helps.

the separator parser cannot be any parser but must be a consuming parser, contrary to documentation. Ideally, we should have a trait to assert this constraint as well.
@gdennie gdennie requested a review from Geal as a code owner January 27, 2024 12:20
@@ -337,7 +337,7 @@ where
/// [`cut`][crate::combinator::cut].
///
/// # Arguments
/// * `sep` Parses the separator between list elements.
/// * `sep` Parses the separator between list elements. Must be consuming.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many0 has a similar note that is separate from the Arguments list

Note: if the parser passed in accepts empty inputs (like alpha0 or digit0), many0 will return an error, to prevent going into an infinite loop

I'm assuming it'd be good to be consistent.

Copy link
Contributor Author

@gdennie gdennie Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether a parser consumes some input is clearly an important characteristic. As such, it would be good if that characteristic could be put into the type system.

It seems at least all of the multi combinators require that their subject parsers consume some input.

I wonder if this consuming requirement can be expressed in the type system through a ownership and return pattern and whether its worth the effort.

// Consuming Parsers...
fn(OldInput) -> NewInput {..}

// Optionally Consuming Parsers
fn(OldInput) -> Either<OldInput, NewInput> {..}

// Some machinery and ergonomics
impl From<NewInput> for OldInput {..}
impl From<Either<OldInput, NewInput>> for OldInput {..}

impl OldInput {
    fn next(&self) -> NewInput {..}
}
impl NewInput {
   fn next(&self) -> NewInput {..}
} 

@Geal Geal merged commit 19e8bb8 into rust-bakery:main Apr 21, 2024
@Geal
Copy link
Collaborator

Geal commented Apr 21, 2024

thank you, it's a good idea to be explicit about it

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.

None yet

3 participants