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

date: Adds support for datetime for parameter -d #5181

Closed
wants to merge 1 commit into from

Conversation

philolo1
Copy link

@philolo1 philolo1 commented Aug 21, 2023

This resolves issue #5177.

The PR adds a new enum HumanDuration(Duration), HumanDateTime(DateTime) and uses parse_datetime::parse_datetime::from_str to support datetime if relative time cannot be parsed.

For example we can now parse date -d "@0", date -d "1992-02-12" and date -d "2012-01-01 14:00".

Furthermore tests are added for tests/by-util/test_date.rs.

@tertsdiepraam
Copy link
Member

Cool! I don't think we need 2 variants. Basically parse_datetime should always return a DateTime. If it parses a relative time, it should add that to the current time.

@philolo1
Copy link
Author

@tertsdiepraam thank you for your comment, would you recommend to change parse_datetime so it accepts relative time? I can do that, just want to confirm first.

@tertsdiepraam
Copy link
Member

In theory yes, but there's a lot happening there already. Let's start here to get the issue fixed and look at getting that change to parse_datetime later. Does that sound good?

@philolo1
Copy link
Author

@tertsdiepraam okay, so you want me to fix the comments by the bot for now only?

@tertsdiepraam
Copy link
Member

Hmmm, actually you're right. I didn't think this through. Yes, parse_datetime should do both absolute and relative in one function. Sorry for the confusion :)

@philolo1
Copy link
Author

@tertsdiepraam okay no problem. Thanks for clarification.

@philolo1 philolo1 marked this pull request as draft August 22, 2023 15:00
@philolo1 philolo1 force-pushed the fix/support-datetime-to-date branch 3 times, most recently from bfcd05e to bbf95cf Compare September 19, 2023 09:37
This resolved issue uutils#5177.

The PR adds a new enum  HumanDuration(Duration), HumanDateTime(DateTime<FixedOffset>) and uses  parse_datetime::parse_datetime::from_str  to support datetime if relative time cannot be parsed.

Furthermore tests are added for tests/by-util/test_date.rs.
@sylvestre sylvestre force-pushed the fix/support-datetime-to-date branch from bbf95cf to f95e42a Compare September 23, 2023 07:36
@tertsdiepraam
Copy link
Member

Looks like touch is failing because the new parse_datetime version. I think we could make touch depend on a separate version and then fix that in another PR. cargo deny will complain, but we can temporarily add parse_datetime to the deny.toml list of ignored crates to fix that.

@sylvestre
Copy link
Contributor

No activity for a while, closing!

Feel free to reopen when ready

@sylvestre sylvestre closed this Dec 25, 2023
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.

3 participants