-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Inaccurate {Path,OsStr}::to_string_lossy()
documentation
#129963
Inaccurate {Path,OsStr}::to_string_lossy()
documentation
#129963
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @workingjubilee (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Thanks! @bors r+ |
@bors rollup |
On Windows it's checking for a UTF-16 sequence not UTF-8. |
...hm. @bors r- |
@ollie27 I'm not sure whether this change isn't more correct though, since we're trying to reencode whatever the path is as UTF-8, and we only do this replacement for things that can't be otherwise encoded as UTF-8. |
Perhaps it's still not perfectly correct since it should reference the notion of the possible transcoding. But we're not actually checking for UTF-16, at least not as commonly understood, because we're rejecting invalid surrogate pairings, and everything that says it works with "UTF-16" actually also deals in such nonsense. Referencing Unicode ambiguously confuses this matter. |
There's no re-encoding going on in On Windows, |
Actually, yeah, #111544 exposed Sorry for the noise. |
@ollie27 No problem. :^)
Correct, I was talking somewhat imprecisely about the "total journey" of some bytes to an OsString to then Anyway, it sounds like we're back to a consensus that we should accept this even if it probably could use an even-better wording that is not leaping right into anyone's brains right now or we'd have suggested it. SO! @bors r+ |
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#129021 (Check WF of source type's signature on fn pointer cast) - rust-lang#129781 (Make `./x.py <cmd> compiler/<crate>` aware of the crate's features) - rust-lang#129963 (Inaccurate `{Path,OsStr}::to_string_lossy()` documentation) - rust-lang#129969 (Make `Ty::boxed_ty` return an `Option`) - rust-lang#129995 (Remove wasm32-wasip2's tier 2 status from release notes) - rust-lang#130013 (coverage: Count await when the Future is immediately ready ) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#129963 - rjooske:fix/inaccurate_to_string_lossy_doc, r=workingjubilee Inaccurate `{Path,OsStr}::to_string_lossy()` documentation The documentation of `Path::to_string_lossy()` and `OsStr::to_string_lossy()` says the following: > Any non-Unicode sequences are replaced with `U+FFFD REPLACEMENT CHARACTER` which didn't immediately make sense to me. ("non-Unicode sequences"?) Since both `to_string_lossy` functions eventually become just a call to `String::from_utf8_lossy`, I believe the documentation meant to say: > Any *non-UTF-8* sequences are replaced with `U+FFFD REPLACEMENT CHARACTER` This PR corrects this mistake in the documentation. For the record, a similar quote can be found in the documentation of `String::from_utf8_lossy`: > ... During this conversion, `from_utf8_lossy()` will replace any invalid UTF-8 sequences with `U+FFFD REPLACEMENT CHARACTER`, ...
The documentation of
Path::to_string_lossy()
andOsStr::to_string_lossy()
says the following:which didn't immediately make sense to me. ("non-Unicode sequences"?)
Since both
to_string_lossy
functions eventually become just a call toString::from_utf8_lossy
, I believe the documentation meant to say:This PR corrects this mistake in the documentation.
For the record, a similar quote can be found in the documentation of
String::from_utf8_lossy
: