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

feat: add sepby1 combinator #165

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Conversation

steve-chavez
Copy link
Contributor

sepby1 is a common reusable combinator in Haskell Parsec.

This adds mpc_sepby1(mpc_fold_t f, mpc_parser_t *sep, mpc_parser_t *a) according to Haskell's implementation:

https://hackage.haskell.org/package/parsec-3.1.16.1/docs/src/Text.Parsec.Combinator.html#sepBy1

Reuses existing mpc_and, mpc_many, and mpcf_snd_free.

`sepby1` is a common reusable combinator in Haskell Parsec.

This adds `mpc_sepby1(mpc_fold_t f, mpc_parser_t *sep, mpc_parser_t *a)` according to Haskell's implementation:

https://hackage.haskell.org/package/parsec-3.1.16.1/docs/src/Text.Parsec.Combinator.html#sepBy1

Reuses existing `mpc_and`, `mpc_many`, and `mpcf_snd_free`.
@steve-chavez
Copy link
Contributor Author

@HalosGhost @orangeduck How can I make it easier to get this reviewed?

(It's a pretty short function and I've included tests/docs)

@orangeduck orangeduck merged commit 55f68ef into orangeduck:master Aug 9, 2023
1 check passed
@orangeduck
Copy link
Owner

Looks great, thanks!

Comment on lines +2123 to +2127
mpc_parser_t *mpc_sepby1(mpc_fold_t f, mpc_parser_t *sep, mpc_parser_t *a) {
return mpc_and(2, f,
a, mpc_many(f, mpc_and(2, mpcf_snd_free, sep, mpc_copy(a), free)),
free);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orangeduck I've realized that the fold function is applied twice. This makes it hard (or maybe impossible) to build an array of structures with a custom fold function.

Do you have any advice on how to fix this?

Seems like we need a new repeat parser, but I'm lost on how to add those.

Copy link
Owner

Choose a reason for hiding this comment

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

I guess you mean that the fold function when used in mpc_many will get a list of entries, while when used in mpc_and it will just get the first item and the rest of the elements?

Yeah I see the problem.

The easiest thing would be to let people give two different fold functions to use in the mpc_and and the mpc_many.

There also might be a way to implement this in terms of mpc_many1 - in particular if you allow a trailing separator...

Otherwise I am not totally sure, and yes perhaps the only way is to implement a custom repeat parser type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orangeduck Many thanks for the reply. I'm more familiar with the repeat parsers code now. I'm working on adding a new one for sepby1.

Preliminary work for that on #166.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks a lot for working on this addition 👍

steve-chavez added a commit to steve-chavez/mpc that referenced this pull request Aug 20, 2023
Deduplicates the results allocation logic used in MPC_TYPE_MANY
and MPC_TYPE_MANY1 on a `grow_results` function.

This will aid in implementing new repeat parser types. A new one is
neeeded for `sepby1` as discussed on
orangeduck#165 (comment).
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