Skip to content

Make a beginning of parsing GNU date items with nom #25

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Make a beginning of parsing GNU date items with nom #25

wants to merge 3 commits into from

Conversation

wanderinglethe
Copy link

@wanderinglethe wanderinglethe commented Jun 25, 2023 •
edited
Loading

GNU date understands multiple "items", currently uutils date only supports a limited amount of formats.

To support more formats I looked into using nom to parse these items. These items then need to be verified and combined to calculate a single date time.

Since this is my first using nom and I don't have much experience with Rust, please give feedback on style, design.

sylvestre
sylvestre reviewed Jun 26, 2023
View reviewed changes
sylvestre
sylvestre reviewed Jun 26, 2023
View reviewed changes
src/parse_items/gnu-items.md
@@ -0,0 +1,22 @@
## General date syntax
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to have examples

Copy link
Author

@wanderinglethe wanderinglethe Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Didn't knew I committed this file, not really sure if we should add this because it's straight out of the GNU docs.

Although I guess we can have some supported GNU features in the documentation.
And maybe some tracking issue for all GNU features.

@sylvestre
Copy link
Contributor

I guess you saw:


---- parse_items::items::tests::some_items stdout ----
thread 'parse_items::items::tests::some_items' panicked at 'assertion failed: `(left == right)`
  left: `Ok([TimeZoneRule(TzIdentifier("Europe/Amsterdam")), SecondsEpoch(SecondsEpoch { seconds: 123, nanoseconds: 456000000 }), CalendarDay(RawCalendarDay { day: 14, month: 11, year: 2022 }), MonthDay(RawMonthDay { day: 14, month: 11 }), TimeOfDay(RawTimeOfDay { hours: 12, minutes: 34, seconds: 45, nanoseconds: 789123456 }), TimeZoneCorrection(TimeZoneCorrection { hours: 1, minutes: 30 }), TimeOfDay(RawTimeOfDay { hours: 23, minutes: 0, seconds: 0, nanoseconds: 0 })])`,
 right: `Ok([TimeZoneRule(TzIdentifier("Europe/Amsterdam")), SecondsEpoch(SecondsEpoch { seconds: 123, nanoseconds: 456000000 }), CalendarDay(RawCalendarDay { day: 14, month: 11, year: 2022 }), MonthDay(RawMonthDay { day: 14, month: 11 }), TimeOfDay(RawTimeOfDay { hours: 12, minutes: 34, seconds: 56, nanoseconds: 789123456 }), TimeZoneCorrection(TimeZoneCorrection { hours: 1, minutes: 30 }), TimeOfDay(RawTimeOfDay { hours: 23, minutes: 0, seconds: 0, nanoseconds: 0 })])`', src/parse_items/items.rs:76:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

sylvestre
sylvestre reviewed Jul 2, 2023
View reviewed changes
Cargo.toml
@@ -10,3 +10,4 @@ readme = "README.md"
[dependencies]
regex = "1.8"
chrono = { version="0.4", default-features=false, features=["std", "alloc", "clock"] }
nom = "7.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
nom = "7.1"
nom = "7.1"

@sylvestre
Copy link
Contributor

Looks but could you please update the README?
Will it be possible to manage cases like?

 date -d@1687977240
date: invalid date '@1687977240'

@sylvestre
Copy link
Contributor

@wanderinglethe ping?

tertsdiepraam
tertsdiepraam reviewed Aug 20, 2023
View reviewed changes
src/parse_items/items/time_of_day.rs
Comment on lines +158 to +165
macro_rules! t12 {
($name:ident : $input:literal => $hours:literal:$minutes:literal:$seconds:literal:$nanoseconds:literal + $tail:literal) => {
ptest! { $name : time_of_day_12($input) => RawTimeOfDay { hours: $hours, minutes: $minutes, seconds: $seconds, nanoseconds: $nanoseconds }, $tail }
};
($name:ident : $input:literal => X) => {
ptest! { $name : time_of_day_12($input) => X }
};
}
Copy link
Member

Choose a reason for hiding this comment

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

I think there's too many macros that could be fairly simple functions. It's not really a problem if tests are a bit verbose :)

@tertsdiepraam
Copy link
Member

Will it be possible to manage cases like?

I think we should do this in another PR. I'd like to merge this relatively soon, so we can build further with it.

@tertsdiepraam tertsdiepraam mentioned this pull request Aug 23, 2023
Closed
@philolo1
Copy link
Contributor

@wanderinglethe are you still working on the pr or may i take over it?

@philolo1 philolo1 mentioned this pull request Aug 24, 2023
Merged
@wanderinglethe
Copy link
Author

@wanderinglethe are you still working on the pr or may i take over it?

I am sorry @philolo1, I missed your question and wasn't able to participate.

@tertsdiepraam tertsdiepraam mentioned this pull request Jan 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sylvestre sylvestre sylvestre left review comments

@tertsdiepraam tertsdiepraam tertsdiepraam left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wanderinglethe @sylvestre @tertsdiepraam @philolo1