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

Support user define parameterizing rules #285

Merged
merged 7 commits into from
Dec 11, 2023
Merged

Support user define parameterizing rules #285

merged 7 commits into from
Dec 11, 2023

Conversation

ydah
Copy link
Collaborator

@ydah ydah commented Dec 7, 2023

No description provided.

@ydah ydah mentioned this pull request Dec 7, 2023
Closed
@ydah ydah force-pushed the support-user-define branch 4 times, most recently from cded23b to f4baa7f Compare December 8, 2023 16:29
@ydah ydah changed the title [WIP] Support user define parameterizing rules Support user define parameterizing rules Dec 8, 2023
@ydah ydah marked this pull request as ready for review December 8, 2023 16:30
@ydah ydah force-pushed the support-user-define branch 2 times, most recently from 7a08775 to 5550ec3 Compare December 8, 2023 16:40
@ydah ydah force-pushed the support-user-define branch from 5550ec3 to c0849c3 Compare December 8, 2023 16:51
yui-knk
yui-knk reviewed Dec 9, 2023
View reviewed changes
parser.y
{
builder = val[2]
result = val[0].append(builder)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Note] Trailing ; is covered by https://github.com/ruby/lrama/pull/285/files#diff-359cd0950b35838d2e90b9e88f5b91a34db81d3125073d530b3b4f4adbc3d0f2R72

yui-knk
yui-knk reviewed Dec 9, 2023
View reviewed changes
parser.y
result = val[0].append(builder)
}

rule_rhs: /* empty */
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Note] Rule for nested parameterizing rules like rhs IDENTIFIER parameterizing_suffix tag_opt or rhs IDENTIFIER "(" parameterizing_args ")" tag_opt are not defined now. It will be supported by another PR.

@ydah ydah force-pushed the support-user-define branch from c932a71 to b07ce82 Compare December 9, 2023 02:56
yui-knk
yui-knk reviewed Dec 9, 2023
View reviewed changes
lib/lrama/grammar.rb Outdated Show resolved Hide resolved
@yui-knk
Copy link
Collaborator

yui-knk commented Dec 9, 2023

Thanks for the PR. Direction is good.
I left comments, this functionality is big one, then we need to take care of a lot things. If you have any questions, ask me freely.
[Note] is just note for me, you don't need to care about in this PR.

@ydah ydah requested a review from yui-knk December 10, 2023 08:33
@ydah
Copy link
Collaborator Author

ydah commented Dec 10, 2023

I updated this PR. Your detailed comments on the review allowed me to make corrections without question. Thank you!

@yui-knk
Copy link
Collaborator

yui-knk commented Dec 11, 2023

Thanks for the update, let's introduce user defined parameterizing rules into Lrama 🎉

@yui-knk yui-knk merged commit 7b93130 into ruby:master Dec 11, 2023
15 checks passed
@ydah ydah deleted the support-user-define branch December 11, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yui-knk yui-knk Awaiting requested review from yui-knk

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

Successfully merging this pull request may close these issues.
2 participants
@ydah @yui-knk