more: use OS errors instead of hardcoded locales - #14085
Conversation
Merging this PR will degrade performance by 4.44%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | numfmt_stream_to_si_precision |
330.4 ms | 348 ms | -5.05% |
| ❌ | Simulation | du_wide_tree[(5000, 500)] |
19.4 ms | 20.3 ms | -4.19% |
| ❌ | Simulation | du_summarize_balanced_tree[(5, 4, 10)] |
16 ms | 16.7 ms | -4.07% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Devel08:more-use-os-error (6e38031) with main (9875296)
Footnotes
-
50 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. ↩
|
GNU testsuite comparison: |
| more-error-is-directory = {$path} is a directory. | ||
| more-error-cannot-open-no-such-file = cannot open {$path}: No such file or directory | ||
| more-error-cannot-open-io-error = cannot open {$path}: {$error} | ||
| more-error-is-directory = {$path}: Is a directory |
There was a problem hiding this comment.
Where is the place more-error-is-directory cannot be replaced by os error? (Removing it at another PR is fine).
There was a problem hiding this comment.
File::open() doesn't throw an error when a file is a directory, thus we have to check that manually
There was a problem hiding this comment.
EISDIR (or EPERM) is returned when read was called for directory. So the arm
coreutils/src/uu/more/src/more.rs
Lines 42 to 51 in 92cbd2f
There was a problem hiding this comment.
yeah it's returned on read, but the message will look something like "more: Is a directory", the path will be missing, if we rely on the message that read returns, should we let read's message or the custom locale?
There was a problem hiding this comment.
I'll fix this at different PR using format! or something else.
This PR replaces hardcoded error message with native OS error