Conversation
src/uu/fmt/src/linebreak.rs
Outdated
| None => { | ||
| return Err(USimpleError::new( | ||
| 1, | ||
| "Failed to find a k-p linebreak solution. This should never happen.", |
There was a problem hiding this comment.
I think this text is right actually, this can, as far as I can tell, never happen. This is because the min_by_key call only returns None if the slice is empty, but the active slice is initialized with at least one value. Maybe an unreachable! call is more appropriate here. Although, I would also love to see the code refactored so that this case is eliminated. For example, we could also choose to just unwrap_or(0) on this min_by_key call, because active is initialized with 0. I'll leave that decision up to you :)
There was a problem hiding this comment.
This makes sense. Will mark the arm unreachable! as unwrap_or can give the impression that the default value can possibly be picked.
There was a problem hiding this comment.
Refactored to remove it completely.
|
GNU testsuite comparison: |
|
@tertsdiepraam I've made the requested changes. I see another related PR on this issue, please let me know if these changes are now redundant. Also, on another note, I see that some unrelated tests are failing, ....is this supposed to happen? |
|
Ah I missed that there were two PRs. I don't think we should discard these changes necessarily. The other PR simply changes |
|
Thank you for your response @tertsdiepraam ! |
src/uu/fmt/src/linebreak.rs
Outdated
| .iter() | ||
| .flat_map(|&&(mut best_idx)| { |
There was a problem hiding this comment.
| .iter() | |
| .flat_map(|&&(mut best_idx)| { | |
| .map(|&(mut best_idx)| { |
src/uu/fmt/src/linebreak.rs
Outdated
| } | ||
| } | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
| .collect() | |
| .unwrap_or_default() |
|
GNU testsuite comparison: |
tertsdiepraam
left a comment
There was a problem hiding this comment.
Great! Many thanks!
Related to issue: #5487
This removes
crash!usage from fmt.