Fix for #39596: sort Trait2 before Trait10. #40567
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
src/librustdoc/html/render.rs
Outdated
| } | ||
|
|
||
| fn name_key(name: &str) -> (&str, u64) { | ||
| let split = s.bytes().rposition(|b| b < b'0' || b'9' < b).unwrap_or(0); |
There was a problem hiding this comment.
Shouldn't this be unwrap_or(name.len())?
There was a problem hiding this comment.
You are correct.
There was a problem hiding this comment.
So we were actually both wrong on this one and I figured this out. The correct answer is map_or(0, |s| s + 1).
Because if the position of the last non-digit is nothing, then this should be 0. Otherwise, it should be the position + 1 to be the beginning of the digits.
src/librustdoc/html/render.rs
Outdated
| // get rid of leading zeroes | ||
| let split = split + s[split..].bytes().position(|b| b != b'0').unwrap_or(0); | ||
| let strkey = &s[..split]; | ||
| let intkey = s[split..].parse().unwrap_or(0); |
There was a problem hiding this comment.
If parsing fails, shouldn't the ordering fall back to the full name for strkey?
There was a problem hiding this comment.
The main thing I was thinking is that the only case where this could fail is if split == name.len(), in which case returning 0 for intkey is what's desired anyway. But I'll change it regardless for clarity.
There was a problem hiding this comment.
Actually, you're right, overflowing a u64 can theoretically (but unlikely) happen. Fixed.
clarfonthey
force-pushed
the
rustdoc-sort
branch
2 times, most recently
from
7af0c0f
to
15d35cd
Compare
|
r? @rust-lang/tools |
|
@steveklabnik this seems like it's much more in the doc team's wheelhouse? (e.g. the presentation of the documentation, not the implementation) |
|
Fine by me if that's fine by you :) /cc @rust-lang/docs then |
clarfonthey
force-pushed
the
rustdoc-sort
branch
from
15d35cd
to
9ff18aa
Compare
|
Could provide screenshots of this (before and after would be very useful) please? |
|
So I just picked out the errors in this, added a few tests, and I'll show a few screenshots once it's done compiling. |
clarfonthey
force-pushed
the
rustdoc-sort
branch
7 times, most recently
from
5af9cd9
to
555223f
Compare
clarfonthey
force-pushed
the
rustdoc-sort
branch
from
555223f
to
c09083c
Compare
|
Examples of generated documentation: https://docs.charr.xyz/before-doc/test_sorting/ Screenshots: |
|
This is ready to merge whenever so let me know if the docs team is okay with this! |
clarfonthey
changed the title
|
LGTM, though going to double check it's alright with @GuillaumeGomez |
|
Fine by me. Thanks @clarcharr! @bors: r=frewsxcv rollup |
|
📌 Commit c09083c has been approved by |
Fix for rust-lang#39596: sort Trait2 before Trait10. This is a change discussed in rust-lang#39596. Essentially, item names will be sorted as if they're (&str, u64) pairs instead of just `&str`, meaning that `"Apple" < "Banana"` and also `"Fruit10" > "Fruit2"`. Sample sorting: 1. Apple 2. Banana 3. Fruit 4. Fruit0 5. Fruit00 6. Fruit1 7. Fruit01 8. Fruit2 9. Fruit02 10. Fruit20 11. Fruit100 12. Pear Examples of generated documentation: https://docs.charr.xyz/before-doc/test_sorting/ https://docs.charr.xyz/after-doc/test_sorting/ Screenshots of generated documentation: Before: http://imgur.com/Ktb10ti After: http://imgur.com/CZJjqIN
This is a change discussed in #39596. Essentially, item names will be sorted as if they're (&str, u64) pairs instead of just
&str, meaning that"Apple" < "Banana"and also"Fruit10" > "Fruit2".Sample sorting:
Examples of generated documentation:
https://docs.charr.xyz/before-doc/test_sorting/
https://docs.charr.xyz/after-doc/test_sorting/
Screenshots of generated documentation:
Before: http://imgur.com/Ktb10ti
After: http://imgur.com/CZJjqIN