-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Implement Debug
for EncodeWide
#140153
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
base: master
Are you sure you want to change the base?
Implement Debug
for EncodeWide
#140153
Conversation
Even if rust doesn't accept \u for surrogate halves, we can still format them as \u. |
Here are the options, as I see them. I think formatting them as chars when not surrogates is the most readable for most texts. Rust style seems to lean towards uppercase hex.
|
d211853
to
827bb27
Compare
I've updated formatting to be 4-wide zero-padded hex (style 4) For another opinion, @ChrisDenton wrote:
(in favor of style 4) |
Since `std::os::windows::ffi::EncodeWide` is reexported from `std::sys_common::wtf8::EncodeWide`, which has `#![allow(missing_debug_implementations)]` in the parent module, it does not implement `Debug`.
827bb27
to
a339db4
Compare
What's needed next to move this along? |
I've nominated for libs-api. I don't know if it needs a full libs API discussion but I believe at least someone from the team will need to sign off on it. |
Since trait impls are insta-stable, if we want to do this it would require a libs-api FCP. Nominating seems like the right next step. |
Ah, right. To clarify my stream of consciousness that was quoted earlier, I don't think we should use I do think it's nice to use hex here as that's what Unicode uses. Zero padded hex makes it look even more like Unicode's U+xxxx format but is valid rust. |
We discussed this in the @rust-lang/libs-api meeting. We definitely agree that @rfcbot merge |
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
To expand on that, we are free to change the implementation in the future because of the lack of guarantee’s on the output of |
Since
std::os::windows::ffi::EncodeWide
is reexported fromstd::sys_common::wtf8::EncodeWide
, which has#![allow(missing_debug_implementations)]
in the parent module, it does not implementDebug
.Implement it like
core::str::Chars
. Format the WTF-16 code units as hex, zero-padded to 4 wide, since it would not be semantically correct to render them likechar
.This becomes insta-stable.
r? libs-api