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.

Sign up for GitHub

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

Jump to bottom

Add list folding to parser #127

Merged
merged 4 commits into from
Feb 1, 2023
Merged

Add list folding to parser #127

merged 4 commits into from
Feb 1, 2023

Conversation

tohrnii
Copy link
Contributor

@tohrnii tohrnii commented Jan 26, 2023 •
edited
Loading

Partly addressing #87. This PR adds support for list folding to the parser. This PR should be merged after #120 and #119.

@tohrnii tohrnii force-pushed the tohrnii-list-folding branch from 7593ee9 to 3a504f9 Compare January 26, 2023 18:04
@tohrnii tohrnii changed the base branch from next to tohrnii-list-comprehension January 26, 2023 18:04
@tohrnii tohrnii marked this pull request as ready for review January 26, 2023 18:05
@tohrnii tohrnii mentioned this pull request Jan 26, 2023
Merged
14 tasks
grjte
grjte reviewed Jan 27, 2023
View reviewed changes
Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

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

preliminarily I think this looks pretty good, but maybe we should wait until the other 2 are merged before reviewing

parser/src/parser/grammar.lalrpop Outdated Show resolved Hide resolved
parser/src/parser/tests/list_comprehension.rs Outdated Show resolved Hide resolved
@grjte
Copy link
Contributor

grjte commented Jan 27, 2023

I think maybe we should put it in draft mode for the moment. wdyt?

@tohrnii tohrnii marked this pull request as draft January 27, 2023 09:27
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 3 times, most recently from dc8fa06 to 8db9bf4 Compare January 30, 2023 09:09
@tohrnii tohrnii force-pushed the tohrnii-list-folding branch 2 times, most recently from 073478e to 842d575 Compare January 30, 2023 10:23
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 6 times, most recently from 5fd2d8d to d358bec Compare January 31, 2023 01:21
@tohrnii tohrnii force-pushed the tohrnii-list-folding branch 4 times, most recently from 90fdfb2 to b2ac361 Compare January 31, 2023 06:54
@tohrnii tohrnii marked this pull request as ready for review January 31, 2023 06:55
@tohrnii tohrnii requested a review from grjte January 31, 2023 06:56
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from d358bec to 5d30017 Compare January 31, 2023 06:57
@tohrnii tohrnii force-pushed the tohrnii-list-folding branch 2 times, most recently from 6234de0 to 0342b0e Compare January 31, 2023 12:07
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from 5d30017 to 580e060 Compare January 31, 2023 14:27
@tohrnii tohrnii force-pushed the tohrnii-list-folding branch from 0342b0e to 53da6e0 Compare January 31, 2023 14:28
@tohrnii tohrnii force-pushed the tohrnii-list-folding branch from 53da6e0 to 0d3b94e Compare January 31, 2023 14:36
Base automatically changed from tohrnii-list-comprehension to next January 31, 2023 14:36
Fumuran
Fumuran approved these changes Feb 1, 2023
View reviewed changes
Copy link
Contributor

@Fumuran Fumuran left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you @tohrnii!

grjte
grjte approved these changes Feb 1, 2023
View reviewed changes
Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

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

This looks great @tohrnii, nice work!

@tohrnii tohrnii merged commit 5844e8d into next Feb 1, 2023
@tohrnii tohrnii deleted the tohrnii-list-folding branch February 1, 2023 13:57
This was referenced Mar 7, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grjte grjte grjte approved these changes

@Fumuran Fumuran Fumuran approved these changes

@Al-Kindi-0 Al-Kindi-0 Awaiting requested review from Al-Kindi-0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tohrnii @grjte @Fumuran