feat: add customizable uptime display with multiple time units#102
feat: add customizable uptime display with multiple time units#102joncrangle merged 1 commit intojoncrangle:mainfrom
Conversation
b14a8f2 to
3b1a010
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a customizable uptime display system that allows users to specify which time units to show (week, day, hour, min, sec) and moves uptime functionality from system stats to a dedicated --uptime flag.
- Created a new uptime module with flexible time unit support and intelligent carry-over logic
- Added
--uptimeCLI flag with proper validation and removed uptime from system stats - Updated documentation to reflect the new uptime usage patterns
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stats/uptime.rs | New module implementing customizable uptime display with multiple time units |
| src/stats/system.rs | Removed uptime handling from system stats |
| src/stats/mod.rs | Added uptime module export |
| src/main.rs | Integrated new uptime flag handling and removed old uptime logic |
| src/cli.rs | Added --uptime flag and removed uptime from system flags |
| README.md | Updated documentation with new uptime usage examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/cli.rs
Outdated
| } | ||
|
|
||
| pub fn all_uptime_flags() -> Vec<&'static str> { | ||
| vec!["day", "hour", "min", "sec", "week"] |
There was a problem hiding this comment.
The uptime flags are not sorted in the same order as TIME_UNITS in uptime.rs (week, day, hour, min, sec). This inconsistency could be confusing. Consider ordering them consistently, either largest-to-smallest or alphabetically.
| vec!["day", "hour", "min", "sec", "week"] | |
| vec!["week", "day", "hour", "min", "sec"] |
src/stats/uptime.rs
Outdated
| let mut flags_vec: Vec<&str> = flags.iter().copied().collect(); | ||
| flags_vec.sort_by_key(|&flag| { | ||
| TIME_UNITS | ||
| .iter() | ||
| .position(|u| u.name == flag) | ||
| .unwrap_or(usize::MAX) | ||
| }); |
There was a problem hiding this comment.
Using unwrap_or(usize::MAX) could place invalid flags at the end of the sorted list instead of handling them properly. Invalid flags should be filtered out or cause an error, as they won't match any TIME_UNITS entry and will be silently ignored in the following loop.
There was a problem hiding this comment.
Could use the following to filter out invalid flags:
let sorted_flags: Vec<&str> = if flags.is_empty() {
TIME_UNITS.iter().map(|u| u.name).collect()
} else {
let mut flags_vec: Vec<&str> = flags
.iter()
.copied()
.filter(|&flag| TIME_UNITS.iter().any(|u| u.name == flag))
.collect();
flags_vec.sort_by_key(|&flag| TIME_UNITS.iter().position(|u| u.name == flag).unwrap());
flags_vec
};|
Thanks for the PR. I've tested and works great. Please address the two review comments and I'll merge. Please run |
|
Apologies for the delayed response. I’ve been quite busy the past couple of days and unfortunately missed replying to this PR. I’ll now make the necessary changes as per your suggestions and get them updated shortly. |
- Add support for customizable uptime display units (week, day, hour, min, sec) - Allow users to specify which time units to display via CLI flags - Implement intelligent time unit conversion and formatting - Update README with correct usage examples for --uptime flag
3b1a010 to
5df5670
Compare
Summary
Changes
src/stats/uptime.rsmodule with customizable time unit display--uptimeflag with proper validationTest plan
--uptimeflag works with various unit combinations