Skip to content

Replace unmaintained human-sort with numeric-sort#1759

Merged
mre merged 3 commits intolycheeverse:masterfrom
tosky1125:replace-human-sort-with-numeric-sort
Jul 4, 2025
Merged

Replace unmaintained human-sort with numeric-sort#1759
mre merged 3 commits intolycheeverse:masterfrom
tosky1125:replace-human-sort-with-numeric-sort

Conversation

@tosky1125
Copy link
Contributor

Description

Fixes #1758

Replaces the unmaintained human-sort crate with numeric-sort to fix panic issues in recent Rust versions.

Problem

  • human-sort is unmaintained
  • Causes "user-provided comparison function does not correctly implement a total order" panic in recent Rust versions

Solution

  • Replace human-sort (0.2.2) with numeric-sort (0.1.5)
  • Update all usages of human_sort::compare to numeric_sort::cmp
  • The new crate provides the same natural/human sorting behavior

Changes

  • Updated Cargo.toml dependency
  • Modified sorting logic in:
    • lychee-bin/src/formatters/stats/mod.rs
    • lychee-bin/src/formatters/stats/compact.rs
    • lychee-bin/src/formatters/stats/detailed.rs

Testing

  • cargo build passes without errors
  • cargo test passes
  • No more panic when running lychee
  • Sorting behavior remains consistent with natural ordering

tosky1125 added 2 commits July 4, 2025 20:57
human-sort is unmaintained and causes panic in recent Rust versions.
This commit replaces all usages of human_sort::compare with
numeric_sort::cmp to fix the 'total order' panic issue.

Fixes lycheeverse#1758
sorted_suggestions.sort_by(|a, b| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
cmp(&a, &b)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is std::cmp. Can we use the fully qualified name here to avoid the naming confusion?

Suggested change
cmp(&a, &b)
numeric_sort::cmp(&a, &b)

sorted_suggestions.sort_by(|a, b| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
cmp(&a, &b)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here and in the other examples

@mre
Copy link
Member

mre commented Jul 4, 2025

Thanks for the PR. Added some comments.

@shawneverest
Copy link

@mre
Thanks for the review! I've updated all three files to use the fully qualified name:

  • Removed use numeric_sort::cmp; imports
  • Changed all usages to numeric_sort::cmp(&a, &b)

This should make it clear that we're using the numeric_sort function rather than the std module.

@mre
Copy link
Member

mre commented Jul 4, 2025

Yup, that makes perfect sense. Thanks a lot!

@mre mre merged commit 830ec2d into lycheeverse:master Jul 4, 2025
6 checks passed
@mre mre mentioned this pull request Jul 4, 2025
@mre mre mentioned this pull request Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch to numeric-sort

3 participants