Add legacy +POS/-POS handling in sort to pass GNU sort-field-limit test#9501
Conversation
Support GNU’s obsolescent +POS1 [-POS2] syntax by translating it to -k before clap parses args, gated by _POSIX2_VERSION. Adds tests for accept and reject cases to ensure sort-field-limit GNU test passes.
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
| let mut idx = 0; | ||
| let bytes = spec.as_bytes(); | ||
| while idx < bytes.len() && bytes[idx].is_ascii_digit() { | ||
| idx += 1; | ||
| } |
There was a problem hiding this comment.
I think a functional approach is cleaner. Something like:
| let mut idx = 0; | |
| let bytes = spec.as_bytes(); | |
| while idx < bytes.len() && bytes[idx].is_ascii_digit() { | |
| idx += 1; | |
| } | |
| let idx = spec.chars().take_while(|c| c.is_ascii_digit()).count(); |
| let mut char_idx = 0; | ||
| let stripped_bytes = stripped.as_bytes(); | ||
| while char_idx < stripped_bytes.len() && stripped_bytes[char_idx].is_ascii_digit() { | ||
| char_idx += 1; | ||
| } |
There was a problem hiding this comment.
Same here.
| let mut char_idx = 0; | |
| let stripped_bytes = stripped.as_bytes(); | |
| while char_idx < stripped_bytes.len() && stripped_bytes[char_idx].is_ascii_digit() { | |
| char_idx += 1; | |
| } | |
| let char_idx = stripped.chars().take_while(|c| c.is_ascii_digit()).count(); |
| #[derive(Debug, Clone)] | ||
| struct LegacyKeyPart { | ||
| field: usize, | ||
| char: usize, |
There was a problem hiding this comment.
The name char is a bit unlucky.
| to.field.saturating_add(1) | ||
| }; | ||
|
|
||
| let mut end_part = end_field.to_string(); |
There was a problem hiding this comment.
What's the reason for introducing end_part? Couldn't you reorder things a bit and push directly to keydef?
| } | ||
| } | ||
|
|
||
| fn parse_legacy_part(spec: &str, default_char: usize) -> Option<LegacyKeyPart> { |
There was a problem hiding this comment.
Is the default_char param necessary? It's 0 in both places this function is called.
| if stripped | ||
| .as_bytes() | ||
| .first() | ||
| .is_some_and(|c| c.is_ascii_digit()) | ||
| { |
There was a problem hiding this comment.
I think you can simplify it with starts_with:
| if stripped | |
| .as_bytes() | |
| .first() | |
| .is_some_and(|c| c.is_ascii_digit()) | |
| { | |
| if stripped.starts_with(|c: char| c.is_ascii_digit()) { |
CodSpeed Performance ReportMerging #9501 will degrade performances by 4.51%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
@cakebaker Thanks for the review! I've addressed all the comments. |
Kudos, thanks! |
2 participants
This change translates GNU’s legacy +POS1 [-POS2] sort key syntax into -k form before clap parsing, gating it by _POSIX2_VERSION to match GNU behavior, and adds by-util tests to ensure acceptance/rejection behavior matches GNU and that the sort-field-limit compatibility test passes.
#9127