unexpand: accept a blank as a tab-list separator like GNU - #14057
unexpand: accept a blank as a tab-list separator like GNU#14057AlejandroCoronadoN wants to merge 2 commits into
Conversation
Merging this PR will improve performance by 3.44%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | du_wide_tree[(5000, 500)] |
20.2 ms | 19.6 ms | +3.44% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AlejandroCoronadoN:unexpand-space-tab-separator (148fd58) with main (a9b64fc)2
Footnotes
-
229 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(71ebb92) during the generation of this report, so a9b64fc was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
|
GNU testsuite comparison: |
| } | ||
|
|
||
| /// Decide whether the character is either a space or a comma. | ||
| fn is_space_or_comma(c: char) -> bool { |
There was a problem hiding this comment.
Not a useful function. Please remove it and write code directly.
2a16155 to
80248df
Compare
|
Good point, removed the helper and inlined it as |
|
GNU splits on blank, so a literal TAB is a valid separator too - s.split([' ', '\t', ',']). expand's is_space_or_comma has the same gap; worth fixing both here. |
80248df to
7c49a86
Compare
|
Good catch, added the tab so the split is now |
Unable to generate the performance reportThere was an internal error while processing the run's data. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists. |
parse_tab_stops accepts a comma or any blank as a separator, but the tests only ever used a comma or a space, so a tab separator was untested. unexpand had no coverage of multi-stop lists at all. Missing case identified here: uutils/coreutils#14057 * tests/expand/expand.pl (t4a): New test, using tabs as separators. * tests/unexpand/unexpand.pl (tabs-3, tabs-4, tabs-5, tabs-6): New tests, for comma-, space- and tab-separated lists and for a leading separator. Link: #332
GNU
unexpandaccepts both a comma and a blank between tab stops in-t/--tabs, sounexpand -t '2 3'behaves likeunexpand -t '2,3'. uutils only splits on a comma, so the space form is rejected:uutils' own
expandalready accepts both separators via anis_space_or_commahelper; onlyunexpandwas inconsistent. This splits the tab list on a space or a comma too, mirroringexpand. Verified byte for byte against GNUunexpandin the C locale for'2 3',' 2 3','2 3','2, 3'and'1 4 7'; invalid values like-t xstill error as before.