Fix escaping of chars in Debug for Wtf8 backed OsStr #27319
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { | ||
| fn write_str_escaped(f: &mut fmt::Formatter, s: &str) -> fmt::Result { | ||
| for c in s.chars().flat_map(|c| c.escape_default()) { | ||
| try!(write!(f, "{}", c)) |
There was a problem hiding this comment.
I think we want this to just be try!(f.write_char(c)), it saves some indirections.
There was a problem hiding this comment.
That bit is verbatim from core::fmt. I tried write_char but it didn't work (was missing Write in scope) so I didn't touch the code. Fixing now.
|
Testcompiling some code suggests that using write_char results in some real code size improvements, i.e. the write version results in irremovable overhead. So the other instances of using #![crate_type="lib"]
use std::fmt::{self, Write};
pub fn test_write(c: char, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", c)
}
pub fn test_write_char(c: char, f: &mut fmt::Formatter) -> fmt::Result {
f.write_char(c)
} |
|
Looks like there are some errors with |
|
I've got 2 instances of |
|
What do you mean by "fixed"? Also, could you squash the commits down? |
|
@alexcrichton Changed 2 instances of I'll try squashing ... it might all blow up. |
|
Ah ok, that's fine to throw in as well |
Fixes #27211 Fix Debug for {char, str} in core::fmt
|
Looks like it. Thanks for your patience ! |
I had to modify some tests : since `wtf8buf_show` and `wtf8_show` were doing the exact same thing, I repurposed `wtf8_show` to `wtf8buf_show_str` which ensures `Wtf8Buf` `Debug`-formats the same as `str`. `write_str_escaped` might also be shared amongst other `fmt` but I just left it there within `Wtf8::fmt` for review.
|
Thanks @diaphore! |
I had to modify some tests : since
wtf8buf_showandwtf8_showwere doing the exact same thing, I repurposedwtf8_showtowtf8buf_show_strwhich ensuresWtf8BufDebug-formats the same asstr.write_str_escapedmight also be shared amongst otherfmtbut I just left it there withinWtf8::fmtfor review.