Open shugo opened 1 month ago
We should wait until Ruby 3.1 gets EOL.
@shugo I added it to a new v0.6 milestone. 🙂
@gooroodev WDYT?
Thanks, @admsev! Happy to help!
The pull request makes the following changes:
String#index
with String#byteindex
.$~.end(0)
with $~.byteoffset(0)[1]
.The original code uses String#index
which returns the character index, while the new code uses String#byteindex
which returns the byte index. This change can affect the handling of multibyte characters. However, the code seems to handle byte offsets correctly by using $~.byteoffset(0)[1]
.
The comment "an 'accept' versiun of #text" contains a typo. The word "versiun" should be "version".
Proposed Fix:
# an "accept" version of #text
The method accept_spaces
uses byteindex
while other methods like next_token
still use index
in some places. For consistency, all instances should be updated to use byteindex
.
Proposed Fix:
Replace all instances of index
with byteindex
in the next_token
method.
byteindex
can improve performance in handling multibyte strings, which is crucial for IMAP responses that may contain multibyte characters.parse_error
calls to manage unexpected tokens.byteindex
Current Code:
@str.index(/\S*/n, @pos)
Improved Code:
@str.byteindex(/\S*/n, @pos)
Current Code:
# an "accept" versiun of #text
Improved Code:
# an "accept" version of #text
Ensure all instances of index
are replaced with byteindex
where necessary to handle multibyte characters correctly.
The pull request effectively updates the code to use String#byteindex
instead of String#index
, which is a positive change for handling multibyte characters. The proposed improvements should be implemented to maintain consistency and fix minor issues. Overall, the code quality and style are good, and the changes are well-justified.
Yours, Gooroo.dev. I'd love to hear your feedback! React or reply.
@admsev @gooroodev please don't. It's obnoxious and not very helpful.
Because String#index takes O(N), where N is the second argument, while String#byteindex takes O(1).