Repository navigation
cmp: accept unit suffixes in --bytes - #309
jayhemnani9910 wants to merge 2 commits into
Conversation
-n/--bytes only took a plain number, so "cmp -n 1K a b" failed with "invalid --bytes value", while GNU cmp takes the same suffixes there as for --ignore-initial. Share the --ignore-initial parser, with the option name in its error message, and turn the test that expected 1K to fail into cases for -n 1K, --bytes=1kB and --bytes=2M.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
|
||
| Ok(num) | ||
| }; | ||
| let parse_skip = |param: &str, skip_desc: &str| -> Result<SkipU64, String> { |
There was a problem hiding this comment.
do we need this wrapper? could call parse_size(.., "ignore-initial", ..) directly at the two callers, no?
There was a problem hiding this comment.
Done in f89e83b. The wrapper is gone and all seven callers call parse_size(.., "ignore-initial", ..) directly.
| ) | ||
| ); | ||
|
|
||
| // Same unit suffixes as --ignore-initial |
There was a problem hiding this comment.
please also add a -n 1K case in cmp_max_bytes in tests/integration.rs
There was a problem hiding this comment.
Added in f89e83b: -n 1K in cmp_max_bytes. It fails on main with invalid --bytes value '1K'. cargo test passes and clippy is clean.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
-n/--bytesonly takes a plain number, so anything with a unit fails before comparing:GNU cmp takes the same suffixes for
--bytesas for--ignore-initial, and the code already knew it (the TODO on the1Ktest, and the comment onBytesLimitU64about limits like1Y).The
--ignore-initialparser becomesparse_size, which takes the option name for its error message.--bytesuses it, andparse_skipis now a thin wrapper, so its callers don't change. The test that expected--bytes=1Kto fail now checks-n 1K,--bytes=1kBand--bytes=2M, and keeps a failure case for1x.Tests: the new cases fail on main.
cargo test: 285 passed, same as main.cargo fmt --checkis clean, andcargo clippy --all-targetsgives the same warnings as on main. Against GNU cmp 3.12 with those two files,-nwith1K,1kB,2K,1M,1500K,1T,1Y,1500and1499gives the same result and exit code.1b,1x,1Kiand-1give the same error. GNU also takes1k,1KB,1KiBand hex, and reads a leading 0 as octal, for both options; this parser does neither (it reads010as ten), and I left that for a separate change.