-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Update to_uppercase docs to avoid ß->SS example #150902
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: main
Are you sure you want to change the base?
Update to_uppercase docs to avoid ß->SS example #150902
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
504d221 to
9156b54
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
library/core/src/char/methods.rs
Outdated
| /// | ||
| /// ``` | ||
| /// for c in 'ß'.to_uppercase() { | ||
| /// for c in 'ffi'.to_uppercase() { |
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.
Maybe it would make sense to leave a comment explaining that this is a single codepoint? It might be a bit confusing otherwise, as people are likely to not know that these exist (I didn't)
This comment has been minimized.
This comment has been minimized.
5e51443 to
c10f0dd
Compare
library/core/src/char/methods.rs
Outdated
| /// [Unicode Standard]: https://www.unicode.org/versions/latest/ | ||
| /// | ||
| /// # Examples | ||
| /// `'ffi'` (U+FB03) is a single Unicode code point (a ligature) that maps to "FFI" in uppercase. |
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.
Hh... hmm. Would it be better to move this comment to a Rust comment inside the example code block so that it's directly visible in-context and likely to be copied with it?
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.
Hh... hmm. Would it be better to move this comment to a Rust comment inside the example code block so that it's directly visible in-context and likely to be copied with it?
If we move the note into a Rust comment inside the first example, wouldn't it seem local to that specific snippet? Because here we have three snippets that demonstrate the same semantic fact ('ffi' to "FFI"), just through different APIs (iteration vs println! vs assert). Putting the explanation in only one of them wouldn't make the other appear to lack the key context?, and I feel like duplicating the same comment in all of them would introduce redundancy. (maybe i'm overthinking it, and you have a great point about being likely to be copied with it)
I think keeping it as an introduction note makes it clear that it applies to the entire Examples section. What do you think?
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.
Ah, my eyes glazed over and failed to fully process the rest of the diff, it seems!
...Yyyyes, I think it's either "once" or "repeated per-instance", and at that point I am less sure of either choice as I think there is still merit of keeping the note close but redundancy is, well, redundancy.
|
I was trying to think of a ligature that would be less "what? that's one codepoint?" to readers, because of being more commonly seen, but unfortunately the best example I can think of, |
|
( Also that's because it isn't a ligature exactly, except it kinda is, but it also is a Proper Letter, "æsh", retained like þ (thorn) and others. ) |
@workingjubilee What do you think? |
|
Also, it's slightly better than the previous ffi ligature. (But in font-rendering it looks like "ft" (FT)) |
|
oh, long S. sure! |
Hmmm.. Would the |
|
So, after some chatting with @workingjubilee on Zulip, I chose to go with the "ſt" ligature because I feel it's less confusing in font-rendering than the "ffi" ligature (which looks exactly like the string "ffi", here using Firefox and Github I can't even tell the difference). The introductory explanation note will be kept as well to make things clearer for readers |
for clarity: it would not, and my lamentation was basically "alas, any ligature that is used enough becomes considered a 'proper letter' which then means it is given a majuscule variant even if it didn't have one before". the Eszett is counted here, as it is also a ligature based on long S and Z. As I think this is the most distinctive variation we're getting: @bors r=Noratrieb,workingjubilee |
Fixes #150888
Updates the
to_uppercasedocumentation examples to avoid relying on the ß → "SS" mapping and instead uses a stable multi-character case-mapping example ('ffi' → "FFI").Note: the example uses U+FB03 (LATIN SMALL LIGATURE FFI), not the ASCII string "ffi".