Skip to content

Add time parser #3738

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

Merged
merged 1 commit into from
Jan 12, 2024
Merged

Add time parser #3738

merged 1 commit into from
Jan 12, 2024

Conversation

eliaskosunen
Copy link
Contributor

@eliaskosunen eliaskosunen commented Dec 14, 2023

Closes https://github.com/tenzir/issues/issues/1051

Adds a new parser, time, based on POSIX strptime.

parse <field> time [--components] [--strict] <format>

<format> is a format string compatible with strptime. The output is a tenzir::time, except if the --components flag is passed, the output is a record. There's some additional logic for missing fields and what their default values are, which can be controlled with --strict.

Deals with timezones properly, which letmetellya is something one should not call trivial.

@eliaskosunen eliaskosunen added feature New functionality format Parser and printer labels Dec 14, 2023
@eliaskosunen eliaskosunen changed the title Add time parse Add time parser Dec 14, 2023
@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 5 times, most recently from ccc057e to aac7549 Compare December 18, 2023 10:19
@eliaskosunen eliaskosunen marked this pull request as ready for review December 18, 2023 10:20
@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 5 times, most recently from 710bff1 to 1bbd25e Compare December 19, 2023 13:43
@eliaskosunen eliaskosunen marked this pull request as draft December 21, 2023 13:27
@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 2 times, most recently from ece7294 to 5b69904 Compare December 21, 2023 14:53
dominiklohmann
dominiklohmann previously approved these changes Dec 21, 2023
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

The tests look good to me. I skimmed through the code and didn't find anything major, so assuming that the tests now pass on macOS this is also ready to go.

@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 2 times, most recently from 52e246e to 43a61c2 Compare December 22, 2023 10:12
@eliaskosunen
Copy link
Contributor Author

eliaskosunen commented Dec 22, 2023

It seems like libc++ (the stdlib on macOS) <chrono> is currently conflicting with https://github.com/HowardHinnant/date. This is definitely something I can't work around today, so this'll have to wait until January. See HowardHinnant/date#799

Copy link
Contributor

@jachris jachris left a comment

Choose a reason for hiding this comment

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

This looks nice!

@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 7 times, most recently from 9fda103 to 243977b Compare January 10, 2024 13:08
@eliaskosunen eliaskosunen changed the base branch from main to topic/diagnostic-for-unknown-msgpack-type January 10, 2024 13:08
@eliaskosunen eliaskosunen marked this pull request as ready for review January 10, 2024 13:09
@Dakostu Dakostu force-pushed the topic/diagnostic-for-unknown-msgpack-type branch 2 times, most recently from 9ac499f to 7b37d72 Compare January 10, 2024 15:13
Base automatically changed from topic/diagnostic-for-unknown-msgpack-type to main January 11, 2024 07:40
@eliaskosunen eliaskosunen force-pushed the topic/time-parser branch 2 times, most recently from 5484a32 to ccc38d1 Compare January 11, 2024 09:28
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

This works quite well. Thank you!

@eliaskosunen eliaskosunen merged commit 1daf7eb into main Jan 12, 2024
@eliaskosunen eliaskosunen deleted the topic/time-parser branch January 12, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality format Parser and printer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants