-
Notifications
You must be signed in to change notification settings - Fork 34
top, w: fix uptime format #401
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
Conversation
46c9e49 to
a5588fe
Compare
src/uu/top/src/header.rs
Outdated
| ByteSize::b(memory_b).0 as f64 / unit as f64 | ||
| } | ||
|
|
||
| fn format_uptime_procps(up_secs: i64) -> UResult<String> { |
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.
What's the reason for using a procps suffix in the function name (and in get_formated_uptime_procps)?
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.
to distinguish it from that of uucore
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.
should I keep 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.
Yes, I would keep it. Currently I don't have a better idea. And we can still change it later, if necessary.
src/uu/top/src/header.rs
Outdated
| } | ||
|
|
||
| #[inline] | ||
| fn get_formated_uptime_procps() -> UResult<String> { |
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.
A typo:
| fn get_formated_uptime_procps() -> UResult<String> { | |
| fn get_formatted_uptime_procps() -> UResult<String> { |
src/uu/top/src/header.rs
Outdated
| let test_times = [ | ||
| 1, | ||
| 10, | ||
| 60, | ||
| 100, | ||
| 200, | ||
| 3600, | ||
| 5000, | ||
| 3600 * 24, | ||
| 3600 * 24 + 60, | ||
| 3600 * 24 + 3600, | ||
| ]; | ||
| let re = regex::Regex::new(r"(\d+ days?,)?\s*\d+( min|:\d+)").unwrap(); |
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 wouldn't use a regex for this test as it also matches invalid results. I think it is simpler to use (input, expected) pairs.
|
Are we planning a common crate for uutils/procps, which is similar to uucore? I think the uptime formatter are widely used in procps. As the alternative option, I would place it in |
e7f078e to
377c182
Compare
377c182 to
f15a629
Compare
Yes, I think it would make sense to have such a crate. |
|
all fixed. |
|
Thanks! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #401 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
procps uptime format from procps differs from coreutils'.
for example:
coreutils:
procps v4.0.5:
also see coreutils/7757