pr: report missing file as "pr: <file>: <message>"#13534
Merged
Conversation
std::fs::read errors were converted to PrError via to_string(), which dropped the file name and kept Rust's raw "(os error N)" suffix, so `pr nonexistent` printed "pr: No such file or directory (os error 2)". Attach the path and strip the errno with the existing uucore strip_errno() helper so the message matches GNU pr: "pr: nonexistent: No such file or directory".
Merging this PR will improve performance by 3.53%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
Contributor
|
Thanks for your PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13013
What
When
pris given a path it can't open, it currently prints:The file name is missing and Rust's raw
(os error N)suffix leaks. GNUprprints the file name and no errno:Change
read_to_endmappedstd::fs::readerrors intoPrErrorthrough the genericFrom<io::Error>impl, which only callsto_string()— so the path (still in scope at the call) was lost and the errno suffix kept.This adds a dedicated
PrError::ReadError { path, msg }variant and builds the message inread_to_end, reusing the existinguucore::error::strip_errno()helper (the same pattern already used inheadandsort).Only the read path is changed, so write errors (e.g.
pr file >/dev/full) keep their current behavior and are not prefixed with the file name, matching the discussion on the issue.After:
Test
Adds
test_missing_file_error_message. The exact OS message differs by platform (for example, "The system cannot find the file specified." on Windows), so the test asserts the portable behavior — the file name is present and the(os error N)suffix is stripped — rather than a hardcoded string.cargo test,cargo clippy --all-targets, andcargo fmt --checkall pass locally.Note for triage: #13231 also targets this issue but has been stalled; its regression test hardcodes the Unix error string and fails on Windows CI. This PR is a minimal alternative that keeps a platform-independent test.