sortcheck: review-driven cleanup, add --numeric/--natural, allocation-free streaming loop#3756
Merged
Merged
Conversation
…-free streaming loop Bring sortcheck in line with the recent review-driven cleanups for sort (#3755), dedup (#3754), datefmt (#3753), and util (#3752), and close a real feature gap: sortcheck previously only supported lex / --ignore-case, so users could not pre-verify a CSV before piping into `sort --numeric`, `sort --natural`, or `dedup --numeric --sorted`. Changes - Add --numeric (-N) and --natural flags, reusing iter_cmp_num, iter_cmp_natural, and iter_cmp_natural_ignore_case already exported from cmd/sort.rs (no new comparators). - Resolve flags into a ComparisonMode enum once before the loop, with the same precedence as sort/dedup: --natural > --numeric > --ignore-case. Dispatch is a single match per row, monomorphizing to one comparator. - Replace record.clone_from(&next_record) with std::mem::swap on both Less and Greater arms — zero-copy buffer rotation, matching dedup #3754. - args.flag_json | args.flag_pretty_json -> || (idiomatic logical OR). - dupe_count as i64 -> i64::try_from(...).unwrap_or(i64::MAX) with a comment on why -1 signals "not applicable". - Drop stale #[allow(dead_code)] on Args; add a comment on the do_json-forces-full-scan invariant that makes scan_ctr equivalent to record_count in the JSON path. - USAGE: documented the new flags + their precedence, added an Examples block, tightened the extsort paragraph (sort itself is multi-threaded). - Regenerated docs/help/sortcheck.md via `qsv --generate-help-md`. Tests - 10 new tests in tests/test_sortcheck.rs covering --ignore-case (including a real lex-vs-ignore-case discriminator), empty input, single-row input, --numeric sorted/unsorted, --natural sorted/unsorted, --natural + --ignore-case composition, and a precedence test confirming --natural overrides --numeric. - 19/19 sortcheck tests pass; `cargo clippy -F all_features -- -D warnings` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 15 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades sortcheck to support the same comparison modes as sort/dedup (notably --numeric and --natural), while also tightening the streaming loop to reduce allocations and updating tests/docs accordingly.
Changes:
- Add
--numeric(-N) and--naturalcomparison modes (with the same precedence rules assort/dedup). - Replace
clone_fromwithstd::mem::swapin the streaming loop to avoid per-row buffer copies. - Expand
sortchecktests and regeneratedocs/help/sortcheck.mdto cover the new flags and examples.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/cmd/sortcheck.rs |
Adds numeric/natural comparison dispatch and allocation-free record rotation; updates USAGE/docs text. |
tests/test_sortcheck.rs |
Adds new cases for ignore-case, empty/single-row inputs, numeric/natural checks, and precedence. |
docs/help/sortcheck.md |
Regenerated help markdown to reflect new flags, precedence, and examples. |
…omparisonMode Copy, test rename - Fix empty-input bug: a header-only CSV used to report `record_count: 1` because the loop incremented scan_ctr once even when the initial read_byte_record returned false. Guard the loop and simplify the JSON path to always use scan_ctr (do_json forces a full scan, so it's authoritative). - Reword the --ignore-case USAGE block so no continuation line begins with `--natural` (docopt scans for option-like prefixes; lines starting with `--` could be misinterpreted as a second option definition). - Also clarify the wording: --ignore-case is only ignored under PURE numeric comparison (--numeric without --natural), since the documented precedence allows --natural --numeric --ignore-case where ignore-case does apply. - Derive Clone/Copy on ComparisonMode to mirror SortMode in cmd/sort.rs. Functionally a no-op (matching unit variants doesn't move), but defensive and consistent with sort.rs. - Rename test sortcheck_ignore_case_sorted -> sortcheck_ignore_case_notsorted (the test asserts an error; the prior name was misleading — the truly-sorted-under-ignore-case case is covered by sortcheck_ignore_case_actually_sorted). - Update sortcheck_empty test to expect record_count: 0 (was 1, codifying the off-by-one). - Regenerated docs/help/sortcheck.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings
sortcheckin line with the recent review-driven cleanups forsort(#3755),dedup(#3754),datefmt(#3753), andutil(#3752), and closes a real feature gap.sortcheckpreviously only supported lex /--ignore-case, so users could not pre-verify a CSV before piping intosort --numeric,sort --natural, ordedup --numeric --sorted. Adds--numeric(-N) and--natural, reusingiter_cmp_num,iter_cmp_natural, anditer_cmp_natural_ignore_casealready exported fromcmd/sort.rs— no new comparators.ComparisonModeenum once before the loop, with the same precedence assort/dedup:--natural>--numeric>--ignore-case. Dispatch is a singlematchper row that monomorphizes to one comparator.record.clone_from(&next_record)→std::mem::swap(...)on bothLessandGreaterarms — zero-copy buffer rotation, matching dedup dedup: review-driven cleanup, allocation-free ignore-case, edge-case fixes #3754.args.flag_json | args.flag_pretty_json→||(idiomatic logical OR).dupe_count as i64→i64::try_from(...).unwrap_or(i64::MAX)with a comment on why-1signals "not applicable".#[allow(dead_code)]onArgs.do_json-forces-full-scan invariant that makesscan_ctrequivalent torecord_countin the JSON path.sortitself is now multi-threaded).docs/help/sortcheck.mdregenerated viaqsv --generate-help-md.Test plan
cargo test sortcheck -F all_features— 19/19 pass (10 new + 9 existing).cargo clippy --bin qsv -F all_features -- -D warnings— clean.cargo build --bin qsv -F all_features— clean.docs/help/sortcheck.mdreflects the new flags.--ignore-casesorted/unsorted (incl. a real lex-vs-ignore-case discriminator), empty input, single-row input,--numericsorted/unsorted,--naturalsorted/unsorted,--natural+--ignore-casecomposition, and a precedence test confirming--naturaloverrides--numeric.printf 'n\n10\n2\n30\n' | qsv sortcheck -n→ exit 1 (lex unsorted)printf 'n\n2\n10\n30\n' | qsv sortcheck -n --numeric→ exit 0printf 'n\nitem1\nitem2\nitem10\n' | qsv sortcheck -n --natural --json→ sorted JSON, exit 0🤖 Generated with Claude Code