make tidy-alphabetical use a natural sort

[folkertdev]

The idea here is that these lines should be correctly sorted, even though a naive string comparison would say they are not:

foo2
foo10

This is the "natural sort order".

There is more discussion in #t-compiler/help > tidy natural sort

Unfortunately, no standard sorting tools are smart enough to to this automatically (casting some doubt on whether we should make this change). Here are some sort outputs:

> cat foo.txt | sort
foo
foo1
foo10
foo2
mp
mp1e2
np", 
np1e2", 
> cat foo.txt | sort -n
foo
foo1
foo10
foo2
mp
mp1e2
np", 
np1e2", 
> cat foo.txt | sort -V
foo
foo1
foo2
foo10
mp
mp1e2
np1e2", 
np", 

Disappointingly, "numeric" sort does not actually have the behavior we want. It only sorts by numeric value if the line starts with a number. The "version" sort looks promising, but does something very unintuitive if you look at the final 4 values. None of the other options seem to have the desired behavior in all cases:

  -b, --ignore-leading-blanks  ignore leading blanks
  -d, --dictionary-order      consider only blanks and alphanumeric characters
  -f, --ignore-case           fold lower case to upper case characters
  -g, --general-numeric-sort  compare according to general numerical value
  -i, --ignore-nonprinting    consider only printable characters
  -M, --month-sort            compare (unknown) < 'JAN' < ... < 'DEC'
  -h, --human-numeric-sort    compare human readable numbers (e.g., 2K 1G)
  -n, --numeric-sort          compare according to string numerical value
  -R, --random-sort           shuffle, but group identical keys.  See shuf(1)
      --random-source=FILE    get random bytes from FILE
  -r, --reverse               reverse the result of comparisons
      --sort=WORD             sort according to WORD:
                                general-numeric -g, human-numeric -h, month -M,
                                numeric -n, random -R, version -V
  -V, --version-sort          natural sort of (version) numbers within text

r? @Noratrieb (it sounded like you know this code?)

[joshtriplett]

@folkertdev We have a lot of redundant versions of this sort. See the algorithm documented in the style guide; is that algorithm sufficient here? If so, that algorithm is implemented in rustfmt.

[folkertdev]

Oh, right, https://github.com/rust-lang/rustfmt/blob/master/src/sort.rs is probably more robust (e.g. it handles leading/repeated zeros, which my version does not). Can we achieve code sharing in a reasonable way between tidy and rustfmt? Or should I just copy-paste that implementation?

[Noratrieb]

Sharing the code is quite annoying due to how rustfmt is set up with its subtree, copying the code sounds better. It wouldn't be that bad if it diverges.

[folkertdev]

I think because this algorithm was designed for rustfmt, it has some weird behavior for things that are not rust identifiers. For instance it sorts these examples like so

foo_bar
foo-bar

Contrary to alphabetical sorting; _ is sorted before all non-whitespace characters. This also causes this:

        rustc_ast_lowering = { path = \"../rustc_ast_lowering\" }
        rustc_ast = { path = \"../rustc_ast\" }

So, idk, this algorithm has some nice features, but I think it needs some tweaking for this use case.

[rustbot]

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[folkertdev]

After looking at this a bit, I think for the sort of thing that tidy is used for, the segment-based sorting method is a poor fit. The method really overfits on sequences of rust identifiers, and in many cases produces an ordering that is different from standard string sort. Staying as close a possible to standard string sort is convenient (e.g. you can use your editor to sort lines).

So, I've updated the algorithm to 1) consider leading zeros and 2) perform a case-sensitive sort. In most cases that means your editor's sorting functionality will produce the correct result, except when there are numbers in the identifiers.

[saethlin]

Very nice improvement.

[saethlin]

FWIW I don't use my editor to sort these things, I just do a decent enough job by eye then comply when tidy tells me to change something. So any change that looks more sensible and/or facilitates eyeball-binary-search makes me happy.

[rustbot]

There are changes to the tidy tool.

cc @jieyouxu

[folkertdev]

Given that Nora seems busy and @jieyouxu gets pinged for changes to tidy

r? @jieyouxu

[rustbot]

jieyouxu is not on the review rotation at the moment.
They may take a while to respond.

[jieyouxu]

Thanks. I'm not sure if this is always better (e.g. the bootstrap dist changes), but looking at the identifiers involving numbers, at least those are definitely better IMO.

[jieyouxu]

@bors r+ rollup

[bors]

📌 Commit 1dfc840 has been approved by jieyouxu

It is now in the queue for this repository.