string.c: a binary string is inspected byte by byte - #7083
Merged
Conversation
`str_escape()` serves both `inspect` and `dump`, and for `inspect` it passes a whole UTF-8 character through unescaped so the result stays readable. A string marked by `String#b` is read as bytes everywhere else, and holds no characters to keep readable, but the walk here never asked. ```ruby "る".inspect # "る" "る".b.inspect # was "る", CRuby: "\xE3\x82\x8B" "る".b.dump # "\xe3\x82\x8b", escaped byte by byte all along ``` So the same bytes were shown two ways by two methods whose only difference is meant to be whether a printable character is escaped. Read the flag and escape byte by byte, which is the answer `dump` already gave for that string.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUnder UTF-8 support, ChangesBinary string inspection
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
str_escape()serves bothinspectanddump, and forinspectit passes awhole UTF-8 character through unescaped so the result stays readable. A string
marked by
String#bis read as bytes everywhere else and holds no charactersto keep readable, but the walk here never asked about the flag.
The same bytes were shown two ways by two methods whose only intended
difference is whether a printable character is escaped.
The change
str_escape()readsRSTR_BINARY_P()and takes thedumppath for a binarystring. One condition, and the answer it gives is the one
dumpalready gavefor the same string.
Related, not required
String#bmarks the string, and several other places insrc/string.creadsuch a string as UTF-8 for reasons of their own: #7081 for the length,
#7080 for a copy losing the flag, and
String#chop!walking characters. Eachis its own fix; none of them depends on this one and this one does not depend
on them.
Testing
rake testis green onfull-corewith gcc onx86_64-linux: 2215 asserts,no failures. The new test fails without the change:
Checked against CRuby 4.0.6. The escapes differ in letter case (
\xe3here,\xE3there), which is whatdumpin this tree has always produced and is nottouched by this change.
Summary by CodeRabbit