Skip to content
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

Sort: Implement stable sort, ignore non-printing, month sort dedup, auto parallel sort through rayon, zero terminated sort, check silent #2008

Merged
merged 115 commits into from
Apr 8, 2021

Conversation

kimono-koans
Copy link
Contributor

I'd ask the reviewer squash these many, many commits, if they can, because I can't figure it out!

Implemented GNU’s version of stable sort (last resort comparison), ignore non-printing, month sort dedup, auto parallel sort through rayon, zero terminated sort, check silent. Specifically, last resort comparison fixes all sorts of ordering issues when given mixed type inputs.

Created a general numeric compare option. Right now, numeric compare is mostly an emulation mode for what GNU accomplishes through strnumcmp/strncmp. Thus GNU's numeric is on CPU much less. If anyone would like to help implement that, it would be appreciated.

Switched to FNV hash for random shuffle because it has better small input characteristics. See: https://github.com/Gankra/hash-rs

Removed my tests within sort.rs, but created lots of new tests in test_sort.rs.

Fixed month sort - panic on unwrap for longer matches, ie JUNNNN=JUN, and lots of other weird bugs.

AFAIK this is most of the major functionality of GNU sort except key and separator sort which is what I will try next.

sylvestre and others added 30 commits March 24, 2021 20:28
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
…on't know about how general numeric is implemented. No way to do a random test?
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

Some nits

src/uu/sort/src/sort.rs Outdated Show resolved Hide resolved
src/uu/sort/src/sort.rs Show resolved Hide resolved
src/uu/sort/src/sort.rs Show resolved Hide resolved
src/uu/sort/src/sort.rs Show resolved Hide resolved
@kimono-koans
Copy link
Contributor Author

I'm supposing you're waiting on -k and -T to resolve conflicts, but I just discovered an issue with some of my tests. I should resolve before you commit. Thanks.

Arg::with_name(OPT_CHECK_SILENT)
.short("C")
.long(OPT_CHECK_SILENT)
.help("exit successfully if the given file is already sorted, and exit with status 1 otherwise. "),
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove the trailing space (new PR is fine)

@sylvestre sylvestre merged commit 8474249 into uutils:master Apr 8, 2021
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.

3 participants