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

GNU coreutils date-like time zone formatting #759

Merged
merged 10 commits into from
Aug 17, 2022
Merged

Conversation

jarkonik
Copy link
Contributor

@jarkonik jarkonik commented Aug 6, 2022

Pull request adds additional string formatting options for time zones using multiple colons as syntax, which are identical to formatting options in GNU coreutils date ( https://man7.org/linux/man-pages/man1/date.1.html )

This would fix uutils/coreutils#3780

The pull request adds additional format specifiers for chrono::format::strftime.

In addition to currently implemented %z and %:z PR adds 2 new time zone formatting identifiers:

  1. %::z -Offset from the local time to UTC with seconds.
    Example output: +09:30:00
  2. %:::z - Offset from the local time to UTC without minutes.
    Example output: +09

@djc
Copy link
Member

djc commented Aug 6, 2022

Would you mind documenting in the PR description the intended behavior?

@sylvestre
Copy link

@jarkonik ping ? :)

@jarkonik
Copy link
Contributor Author

I've expanded the PR description a bit. If more information is needed please let me know what should I change/add.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Some minor suggestions below.

src/format/mod.rs Outdated Show resolved Hide resolved
src/format/mod.rs Outdated Show resolved Hide resolved
@djc
Copy link
Member

djc commented Aug 16, 2022

Thanks, this looks good to me.

@djc
Copy link
Member

djc commented Aug 17, 2022

@esheppa any opinions?

@esheppa
Copy link
Collaborator

esheppa commented Aug 17, 2022

Looks good, just one thing I'd noticed is that potentially it could loop forever if the input was like %:::::::::: ...etc. I've set up a PR to @jarkonik which proposes two alternative implementations (one is commented out) - jarkonik#1

My concern with the looping forever is that the strftime items can be used to validate user input, so it could be a DOS attack

Copy link
Collaborator

@esheppa esheppa left a comment

Choose a reason for hiding this comment

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

Thanks @jarkonik, if possible, do you mind making a couple of fixes in the docs?

src/format/strftime.rs Outdated Show resolved Hide resolved
src/format/mod.rs Outdated Show resolved Hide resolved
src/format/mod.rs Outdated Show resolved Hide resolved
@djc djc merged commit 5f54101 into chronotope:main Aug 17, 2022
@djc
Copy link
Member

djc commented Aug 17, 2022

Thanks!

@sylvestre
Copy link

Well done!
Sorry for the question but how often do you release ? :)

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.

Some input in date are panicking the binary
4 participants