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

Introduce encoding check macro #6700

Merged
merged 2 commits into from Dec 1, 2022

Conversation

S-H-GAMELINKS
Copy link
Contributor

Some code has encoding check that using rb_usascii_encoding, rb_ascii8bit_encoding and rb_locale_encoding functions.
But, bit longer I thought.

So, I thought better to introduce encoding check macro like a is_ascii_string function to more shorter and simpler.

Comment on lines 1058 to 1060
#define is_usascii_enc(enc) (enc) == rb_usascii_encoding()
#define is_ascii8bit_enc(enc) (enc) == rb_ascii8bit_encoding()
#define is_locale_enc(enc) (enc) == rb_locale_encoding()
Copy link
Member

Choose a reason for hiding this comment

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

Since these macros seem not used under ext or include, isn't internal/encoding.h better?
Or, should be prefixed at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!
Added rb_ prefix and move to internal/encoding.h in f29a25a

@S-H-GAMELINKS S-H-GAMELINKS force-pushed the introduce/encoding_check_macro branch 4 times, most recently from 002d767 to 3e15b42 Compare November 15, 2022 04:22
@S-H-GAMELINKS S-H-GAMELINKS force-pushed the introduce/encoding_check_macro branch 2 times, most recently from ea45c3a to ba67de9 Compare November 22, 2022 13:46
@nobu
Copy link
Member

nobu commented Nov 24, 2022

In general, it is safer to parenthesize macro expressions.

@nobu nobu merged commit 914cf26 into ruby:master Dec 1, 2022
@S-H-GAMELINKS S-H-GAMELINKS deleted the introduce/encoding_check_macro branch December 1, 2022 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants