Conversation
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
Merging this PR will degrade performance by 3.06%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | cmd_cmp_gnu_equal[1000] |
23.2 µs | 24.9 µs | -6.67% |
| ❌ | cmd_cmp_release_equal[25000] |
24.8 µs | 26.2 µs | -5.32% |
| ❌ | cmd_cmp_release_equal[100] |
25.2 µs | 26.5 µs | -4.86% |
| ❌ | cmd_cmp_release_equal[10000] |
24.8 µs | 25.6 µs | -3.28% |
| ⚡ | cmd_diff_release_equal[1000] |
25.8 µs | 24.5 µs | +5.26% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing SAY-5:fix-width-non-utf8-panic (8c9a0b3) with main (860f6f4)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
===========================
===========================
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| .cloned() | ||
| .peekable() | ||
| ) | ||
| ); |
There was a problem hiding this comment.
please also add a case for xyz--width=5 being an operand, since the PR changes that behavior
| #[test] | ||
| fn width_non_utf8_is_not_an_option() { | ||
| use std::os::unix::ffi::OsStringExt; | ||
| // A non-UTF-8 argument whose lossy form ends in `--width=N` must be |
There was a problem hiding this comment.
comment could be shorter, the test name already says most of it :)
| // treated as an operand, not parsed as the width option (which used to | ||
| // panic in `into_string().unwrap()`). | ||
| let bad = OsString::from_vec(b"\xff--width=5".to_vec()); | ||
| assert!(parse_params( |
There was a problem hiding this comment.
is_err() only works here because of the extra operand.
could you please pass bad and foo only and check that from == bad? that shows it is really treated as an operand
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
all three done in 8c9a0b3: added the |
diffpanicked with exit 101 on a non-UTF-8 argument whose lossy form ends in--width=N, e.g.diff $'\xff--width=5' A B. The--widthregex was missing the leading^anchor that--tabsizealready has, so the lossy (U+FFFD) form matched and the followinginto_string().unwrap()then failed on the real bytes.Anchoring the regex (
^--width=...$, matchingtabsize_re) makes such an argument fall through to the operand path instead of being parsed as a width option, so it no longer panics. As a side effectdiff xyz--width=5is now also treated as a filename rather than silently accepting a width.Added a
widthtest and a#[cfg(unix)]regression test for the non-UTF-8 case.Fixes #247.