Skip to content

Commit

Permalink
Rustdoc: Fix natural ordering to look at all numbers.
Browse files Browse the repository at this point in the history
The old implementation only looks at numbers at the end, but not in
other places in a name: "u8" and "u16" got sorted properly, but "u8_bla"
and "u16_bla" did not.
  • Loading branch information
m-ou-se committed Aug 9, 2020
1 parent 543f03d commit 8c705f8
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 29 deletions.
54 changes: 36 additions & 18 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1903,23 +1903,41 @@ fn document_non_exhaustive(w: &mut Buffer, item: &clean::Item) {
}
}

fn name_key(name: &str) -> (&str, u64, usize) {
let end = name.bytes().rposition(|b| b.is_ascii_digit()).map_or(name.len(), |i| i + 1);

// find number at end
let split = name[0..end].bytes().rposition(|b| !b.is_ascii_digit()).map_or(0, |i| i + 1);

// count leading zeroes
let after_zeroes =
name[split..end].bytes().position(|b| b != b'0').map_or(name.len(), |extra| split + extra);

// sort leading zeroes last
let num_zeroes = after_zeroes - split;

match name[split..end].parse() {
Ok(n) => (&name[..split], n, num_zeroes),
Err(_) => (name, 0, num_zeroes),
/// Compare two strings treating multi-digit numbers as single units (i.e. natural sort order).
pub fn compare_names(mut lhs: &str, mut rhs: &str) -> Ordering {
/// Takes a non-numeric and a numeric part from the given &str.
fn take_parts<'a>(s: &mut &'a str) -> (&'a str, &'a str) {
let i = s.find(|c: char| c.is_ascii_digit());
let (a, b) = s.split_at(i.unwrap_or(s.len()));
let i = b.find(|c: char| !c.is_ascii_digit());
let (b, c) = b.split_at(i.unwrap_or(b.len()));
*s = c;
(a, b)
}

while !lhs.is_empty() || !rhs.is_empty() {
let (la, lb) = take_parts(&mut lhs);
let (ra, rb) = take_parts(&mut rhs);
// First process the non-numeric part.
match la.cmp(ra) {
Ordering::Equal => (),
x => return x,
}
// Then process the numeric part, if both sides have one (and they fit in a u64).
if let (Ok(ln), Ok(rn)) = (lb.parse::<u64>(), rb.parse::<u64>()) {
match ln.cmp(&rn) {
Ordering::Equal => (),
x => return x,
}
}
// Then process the numeric part again, but this time as strings.
match lb.cmp(rb) {
Ordering::Equal => (),
x => return x,
}
}

Ordering::Equal
}

fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean::Item]) {
Expand Down Expand Up @@ -1962,7 +1980,7 @@ fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean:
}
let lhs = i1.name.as_ref().map_or("", |s| &**s);
let rhs = i2.name.as_ref().map_or("", |s| &**s);
name_key(lhs).cmp(&name_key(rhs))
compare_names(lhs, rhs)
}

if cx.shared.sort_modules_alphabetically {
Expand Down Expand Up @@ -2395,7 +2413,7 @@ fn compare_impl<'a, 'b>(lhs: &'a &&Impl, rhs: &'b &&Impl) -> Ordering {
let rhs = format!("{}", rhs.inner_impl().print());

// lhs and rhs are formatted as HTML, which may be unnecessary
name_key(&lhs).cmp(&name_key(&rhs))
compare_names(&lhs, &rhs)
}

fn item_trait(w: &mut Buffer, cx: &Context, it: &clean::Item, t: &clean::Trait, cache: &Cache) {
Expand Down
38 changes: 27 additions & 11 deletions src/librustdoc/html/render/tests.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,40 @@
use super::*;

#[test]
fn test_name_key() {
assert_eq!(name_key("0"), ("", 0, 1));
assert_eq!(name_key("123"), ("", 123, 0));
assert_eq!(name_key("Fruit"), ("Fruit", 0, 0));
assert_eq!(name_key("Fruit0"), ("Fruit", 0, 1));
assert_eq!(name_key("Fruit0000"), ("Fruit", 0, 4));
assert_eq!(name_key("Fruit01"), ("Fruit", 1, 1));
assert_eq!(name_key("Fruit10"), ("Fruit", 10, 0));
assert_eq!(name_key("Fruit123"), ("Fruit", 123, 0));
fn test_compare_names() {
for &(a, b) in &[
("hello", "world"),
("", "world"),
("123", "hello"),
("123", ""),
("123test", "123"),
("hello", ""),
("hello", "hello"),
("hello123", "hello123"),
("hello123", "hello12"),
("hello12", "hello123"),
("hello01abc", "hello01xyz"),
("hello0abc", "hello0"),
("hello0", "hello0abc"),
("01", "1"),
] {
assert_eq!(compare_names(a, b), a.cmp(b), "{:?} - {:?}", a, b);
}
assert_eq!(compare_names("u8", "u16"), Ordering::Less);
assert_eq!(compare_names("u32", "u16"), Ordering::Greater);
assert_eq!(compare_names("u8_to_f64", "u16_to_f64"), Ordering::Less);
assert_eq!(compare_names("u32_to_f64", "u16_to_f64"), Ordering::Greater);
assert_eq!(compare_names("u16_to_f64", "u16_to_f64"), Ordering::Equal);
assert_eq!(compare_names("u16_to_f32", "u16_to_f64"), Ordering::Less);
}

#[test]
fn test_name_sorting() {
let names = [
"Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit1", "Fruit01", "Fruit2", "Fruit02",
"Apple", "Banana", "Fruit", "Fruit0", "Fruit00", "Fruit01", "Fruit1", "Fruit02", "Fruit2",
"Fruit20", "Fruit30x", "Fruit100", "Pear",
];
let mut sorted = names.to_owned();
sorted.sort_by_key(|&s| name_key(s));
sorted.sort_by(|&l, r| compare_names(l, r));
assert_eq!(names, sorted);
}

0 comments on commit 8c705f8

Please sign in to comment.