Skip to content

UTF-8 flag cleanup, and str_repeat #10490

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

Merged
merged 2 commits into from
Feb 5, 2023
Merged

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Feb 2, 2023

This abstracts away, and cleans up, the flag handling for properties of
strings that hold when concatenating two strings if they both hold that
property. (These macros also work with simply copies of strings because
a copy of a string can be considered a concatenation with the empty
string.) This gets rid of some branches and some repetitive code, and
leaves room for adding more flags like these in the future.

This also adds the UTF-8 flag for str_repeat (if the input is UTF-8).

cc @Girgias @alexdowad (because you both worked on this before)

@alexdowad
Copy link
Contributor

Nice!

I would just suggest adding a code comment to ZSTR_COPYABLE_CONCAT_PROPERTIES to explain the idea behind this macro. (If I was reading this code without context, I would find it mysterious at first.)

@nielsdos
Copy link
Member Author

nielsdos commented Feb 2, 2023

Thanks, I added some comments.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Oh that's a smarter way of doing it, I always forget that one can achieve this by using a bitwise AND.

…nating two strings

This abstracts away, and cleans up, the flag handling for properties of
strings that hold when concatenating two strings if they both hold that
property. (These macros also work with simply copies of strings because
a copy of a string can be considered a concatenation with the empty
string.) This gets rid of some branches and some repetitive code, and
leaves room for adding more flags like these in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants