Skip to content
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

Add leading "0:" to output involving sub-minute times? #19

Closed
flexibeast opened this issue Jan 9, 2022 · 4 comments
Closed

Add leading "0:" to output involving sub-minute times? #19

flexibeast opened this issue Jan 9, 2022 · 4 comments

Comments

@flexibeast
Copy link
Contributor

Firstly, thanks for emlop!

This just a small thing: i find times like a bare "45" for "45 seconds" to be disorienting, as from other contexts i'm used to sub-minute times being represented as e.g. "0:45" or "0:0:45". (And in those contexts, a bare "45" is used to mean "45 minutes" or "45 hours".)

Might you be willing to change the output format accordingly, or make this an option? (Unless this option already available, and i've missed it ....)

@vincentdephily
Copy link
Owner

Thanks for suggestion. I prefer the shorter format, but this is subjective and it's easy to add another format (there's already an option to display durations as seconds).

Would you like to try writing a PR for this ? It's an easy one, even if you never wrote any Rust before. There are 3 steps:

  • Add a possible_value to the duration arg in cli.rs
  • Add the corresponding branch to the DurationStyle enum, DurationStyle.from_str fn, and fmt_duration fn in main.rs
  • Add cases to the duration unittest in main.rs

@flexibeast
Copy link
Contributor Author

Thanks for being open to this! Sure, happy to try to put together a PR. :-)

i've cloned the repo, and tried running cargo test, but two tests failed:

failures:

---- commands::tests::timespan_next_ stdout ----
thread 'commands::tests::timespan_next_' panicked at 'assertion failed: `(left == right)`
  left: `2020-01-01T00:00:00+00:00`,
 right: `2019-12-31T13:00:00Z`: year 2019-01-01T00:00:00+00:00', src/commands.rs:852:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- proces::tests::start_time stdout ----
thread 'proces::tests::start_time' panicked at 'Couldn't parse   PID                  STARTED', src/proces.rs:130:29

Is this something that should be addressed before proceeding further? Would you like the backtrace?

@vincentdephily
Copy link
Owner

I've extracted those test failures into their own issues, let's handle them individually. Bugfixes are more important than new features, but feel free to focus on the issues that interest you.

@flexibeast
Copy link
Contributor Author

All tests now passing, thanks! Will start working on the PR. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants