-
Notifications
You must be signed in to change notification settings - Fork 244
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
Enforce escape functions to operate over UTF-8 #423
Conversation
@@ -901,11 +897,13 @@ impl<'a> BytesCData<'a> { | |||
/// | `&` | `&` | |||
/// | `'` | `'` | |||
/// | `"` | `"` | |||
pub fn escape(self) -> BytesText<'a> { | |||
BytesText::from_escaped(match escape(&self.content) { | |||
pub fn escape(self, decoder: Decoder) -> Result<BytesText<'a>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because BytesText::escape
and BytesText::partial_escape
introduced only in new 0.24 release, the changes in their signatures are not reflected in the changelog. Moreover, the next PRs will change that signature back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
==========================================
- Coverage 49.53% 49.50% -0.04%
==========================================
Files 22 22
Lines 13856 13861 +5
==========================================
- Hits 6864 6862 -2
- Misses 6992 6999 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
FYI, pretty sure that 1a1f4bf doesn't work, as equality comparison on Cow ignores the variant. You probably need to use |
Yes, you're right. As far as I remember, there are a few more places in tests where we rely on such behavior |
They are always operate over UTF-8 input, but that was not reflected in types.
This set of changes is more isolated that one in #422, and it also does not enforce owning of data in some cases where that is not required.