Skip to content

Implementing peg2pegtl, based on abnf2pegtl. #377

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

redmercury
Copy link
Contributor

This PR implements peg2pegtl based on abnf2pegtl. The input grammar is from the original PEG paper.

@redmercury
Copy link
Contributor Author

I'm a little confused about the clang-tidy errors given that they're in files untouched by this PR. Any insights into that?

@ColinH ColinH self-assigned this Mar 23, 2025
@ColinH
Copy link
Member

ColinH commented Mar 23, 2025

I'm a little confused about the clang-tidy errors given that they're in files untouched by this PR. Any insights into that?

Sometimes this happens when the jobs switch to a newer version of clang-tidy, I'll look into this, and the rest of the PR as soon as I have some free time. Thanks for the initiative!

@redmercury
Copy link
Contributor Author

Thanks. Ideally I would have added a test that recompiled peg2pegtl using the generated output from generating a grammar from peg.peg, but I'm not quite sure how to achieve that in cmake (it's been a while since I used it). I did verify manually that replacing the grammar from peg.hpp with generated output generated the same output though.

@ColinH
Copy link
Member

ColinH commented Apr 6, 2025

This PR still appears to contain some Bazel related files?

@uilianries
Copy link
Member

@redmercury Thank you for your PR. Please, keep focused only in the proposed feature, do not add a new build system that's not supported in the project already, please, revert the bazel changes and keep only cmake for now. There is a separate discussion about including bazel in project.

@redmercury
Copy link
Contributor Author

Sorry about that, it wasn't my intention, just an oversight when experimenting. My apologies. should be good now.

@uilianries
Copy link
Member

@redmercury Thank you!! I see the clang tidy is failing again. I'll take a look over the weekend or this week. Sorry about that!

@ColinH ColinH requested a review from d-frey April 30, 2025 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants