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

Implement Display for Expr and OptimizedExpr #896

Merged
merged 2 commits into from Jul 20, 2023

Conversation

TheVeryDarkness
Copy link
Contributor

@TheVeryDarkness TheVeryDarkness commented Jul 20, 2023

Non-related changes included in previous pull request #895.

Improve the implementation added in #889.

Corresponding tests added.

Not related changes included in previous pull request pest-parser#895.
@TheVeryDarkness TheVeryDarkness requested a review from a team as a code owner July 20, 2023 06:11
@TheVeryDarkness TheVeryDarkness requested review from NoahTheDuke and removed request for a team July 20, 2023 06:11
meta/src/ast.rs Show resolved Hide resolved
meta/src/optimizer/mod.rs Show resolved Hide resolved
Comment on lines +279 to +280
nodes.push(lhs);
current = rhs;
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems this bit isn't covered by tests (also not covered in Choice); I'm fine to merge it as it is though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add several tests for that then. Sorry to forget that.

Copy link
Contributor

@tomtau tomtau Jul 20, 2023

Choose a reason for hiding this comment

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

No problem, it should be ok as it is

To cover more paths in Seq and Choice. Thanks for @tomtau to point it out in pest-parser#896.
@tomtau tomtau merged commit 02cffe6 into pest-parser:master Jul 20, 2023
9 checks passed
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

2 participants