Conversation
Template files were validated with `str::from_utf8` and then copied into a new `String`, and rendered output was copied back into a `Vec<u8>` byte by byte. Use the consuming `String::from_utf8` and `String::into_bytes` conversions so both steps reuse the existing allocation. Binary files and text that fails to parse as a Liquid template still pass through byte-for-byte, since `FromUtf8Error::into_bytes` returns the original buffer and UTF-8 validation never alters it. Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
| None => Ok(TemplateContent::Binary(raw)), | ||
| Some(s) => { | ||
| match String::from_utf8(raw) { | ||
| Err(e) => Ok(TemplateContent::Binary(e.into_bytes())), // TODO: try other encodings! |
There was a problem hiding this comment.
This would benefit from a comment: my first reading was "wait is this rendering the error" and I had to look at the docs to understand "oh the error has methods to retrieve the blob that got consumed."
| Err(e) => Ok(TemplateContent::Binary(e.into_bytes())), // TODO: try other encodings! | |
| Err(e) => Ok(TemplateContent::Binary(e.into_bytes())), // retrieves the original blob // TODO: try other encodings! |
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
itowlson
left a comment
itowlson
left a comment
There was a problem hiding this comment.
Looks good - thanks!
As a minor nit, it may be better to do small fixups as amendments, rather than whole new commits. The second and third commit don't really have useful historical value - we could squash this into one commit and for me it would be clearer that way. (Unfortunately, our policies around verified commits mean we can't do that via a GitHub squash-merge, which is what I'd usually do.) But that's not a blocker, just something to consider for future PRs.
Ah got it, sorry about that. Will do them in a single commit next time. |
2 participants
Use the consuming
String::from_utf8andString::into_bytesconversions in the template renderer, so template text and rendered output reuse their buffers instead of being copied. Also inlines the single-usestring_from_byteshelper.