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

Move ISASCII defination to parse.y #8029

Merged
merged 1 commit into from Jul 8, 2023

Conversation

S-H-GAMELINKS
Copy link
Contributor

@S-H-GAMELINKS S-H-GAMELINKS commented Jul 5, 2023

Move to ISASCII defination to parse.y for universal parser.

@S-H-GAMELINKS S-H-GAMELINKS marked this pull request as draft July 5, 2023 15:28
@S-H-GAMELINKS S-H-GAMELINKS marked this pull request as ready for review July 5, 2023 22:46
@yui-knk
Copy link
Contributor

yui-knk commented Jul 8, 2023

Thanks for the PR. Can I ask you to update check these points?

  1. Remove int (*isascii)(int c); from struct rb_parser_config_struct (rubyparser.h) then remove config->isascii = parse_isascii; from rb_parser_config_initialize (ruby_parser.c). One of goals of the refactoring is to minimize the number of functions required by rb_parser_config_t.
  2. Define parse_isascii in parse.y not rubyparser.h. rubyparser.h is a public interface. I don't want to make parse_isascii public. We can put parse_isascii definition just before #ifdef ISASCII in parse.y

@S-H-GAMELINKS
Copy link
Contributor Author

Thanks for your review!
I fixed these point.

  1. Remove int (*isascii)(int c); from struct rb_parser_config_struct (rubyparser.h) then remove config->isascii = parse_isascii; from rb_parser_config_initialize (ruby_parser.c). One of goals of the refactoring is to minimize the number of functions required by rb_parser_config_t.
  2. Define parse_isascii in parse.y not rubyparser.h. rubyparser.h is a public interface. I don't want to make parse_isascii public. We can put parse_isascii definition just before #ifdef ISASCII in parse.y

@yui-knk yui-knk merged commit 8b2a0ec into ruby:master Jul 8, 2023
87 checks passed
@yui-knk
Copy link
Contributor

yui-knk commented Jul 8, 2023

Thank you!

alitaso345 added a commit to alitaso345/ruby that referenced this pull request Jul 8, 2023
The ISASCII definition was moved to parse.y( ruby#8029 ), but the old definition wasn't removed.
yui-knk pushed a commit that referenced this pull request Jul 8, 2023
The ISASCII definition was moved to parse.y( #8029 ), but the old definition wasn't removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants