Remove crash macro from fmt and unexpand #5522
Closed
Conversation
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
|
GNU testsuite comparison: |
37a7755 to
c84dd50
Compare
cakebaker
approved these changes
Nov 12, 2023
tertsdiepraam
requested changes
Nov 12, 2023
src/uu/unexpand/src/unexpand.rs
Outdated
| @@ -209,16 +209,19 @@ pub fn uu_app() -> Command { | |||
| ) | |||
| } | |||
|
|
|||
| fn open(path: &str) -> BufReader<Box<dyn Read + 'static>> { | |||
| fn open(path: &str) -> std::io::Result<BufReader<Box<dyn Read + 'static>>> { | |||
There was a problem hiding this comment.
It looks like this should return UResult instead, because then we don't have to show! the error immediately. You can look at the map_err_context method too for io errors, that allows us to add a context string and turn them in a UError.
To do this, you could split of all the code that could have write errors into a separate function that returns std::io::Result and then call that with map_err_context in unexpand.
src/uu/fmt/src/linebreak.rs
Outdated
| Some(&s) => s, | ||
| None => { | ||
| show!(USimpleError::new( |
There was a problem hiding this comment.
Shouldn't we stop execution at this error? Either that, or if we can verify that it really never happens it should maybe be an unreachable! call.
c84dd50 to
239f155
Compare
|
GNU testsuite comparison: |
sylvestre
requested changes
Dec 25, 2023
|
no activity for a while, closing |
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.
This PR is related to #5487.
It removes the crash! macro from fmt and unexpand.