-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
sleep
: Replace uucore::parse_time::from_str with fundu
#4448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sleep
: Replace uucore::parse_time::from_str with fundu
#4448
Conversation
Marking this as draft because of the dependency between the two PRs. You can mark it ready again when the other PR is merged :) |
ddd3069
to
ff70ca5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I'll update the branch to get rid of the clippy warnings.
src/uu/sleep/src/sleep.rs
Outdated
ParseError::Syntax(pos, description) | ||
| ParseError::TimeUnit(pos, description) => format!( | ||
"{description} at position {}", | ||
pos.checked_add(1).unwrap_or(pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This is just a saturating_add
isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yes, I'll change it to saturating_add
!
Nice! I'll update the branch to get rid of the clippy warnings.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased onto main after this change, so the merge commit is gone again.
7614cc2
to
5fb091a
Compare
GNU testsuite comparison:
|
Nice! |
See also https://github.com/Joining7943/fundu. This pr also improves the error messages.
This pr depends on #4439