Skip to content

Conversation

@fuad1502
Copy link
Contributor

@fuad1502 fuad1502 commented Nov 24, 2024

Continuation of PR #152 for issue #14 : support —time-format option.

  • notime
  • raw
  • ctime
  • iso
  • delta
  • reltime

@fuad1502
Copy link
Contributor Author

@cakebaker can I support --time-format delta and --time-format reltime directly in this PR or should I put it on a separate PR? I am worried this PR is already too big. delta and reltime will be similar in implementation but differs from the other option values implementation.

I'll delete reltime and delta from test_dmesg.rs if I should put it on a separate PR.

@cakebaker
Copy link
Contributor

It's ok to keep them in this PR, they are part of the --time-format feature.

@fuad1502 fuad1502 marked this pull request as ready for review November 26, 2024 18:00
Co-authored-by: Daniel Hofstetter <[email protected]>
let sub_seconds = i64::abs(delta_us % 1000000);
let sign = if delta_us >= 0 { '+' } else { '-' };
let mut res = format!("{}.{:0>6}", seconds, sub_seconds);
res.insert(0, sign);
Copy link
Contributor

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 insert to add the sign instead of doing it on line 77?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I could've added it on line 77 for this case. I changed it in commit e965074.

Just curious, your comment regarding this is concerning readability, correct? Or are you also concerned about the string processing performance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is about readability. I don't know whether there is much of a difference in string processing performance.

@cakebaker cakebaker merged commit 9e087e6 into uutils:main Nov 30, 2024
13 checks passed
@cakebaker
Copy link
Contributor

Thanks, good job :)

@fuad1502 fuad1502 deleted the dmesg-time-format branch December 2, 2024 03:12
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

Successfully merging this pull request may close these issues.

2 participants