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

Could date filter support Instant? #308

Closed
seancorfield opened this issue May 30, 2024 · 5 comments
Closed

Could date filter support Instant? #308

seancorfield opened this issue May 30, 2024 · 5 comments
Assignees

Comments

@seancorfield
Copy link
Collaborator

seancorfield commented May 30, 2024

The code for java.util.Date arguments goes through Instant to get to a local date/time so it seems like java.time.Instant could be supported this way too:

        (instance? java.util.Date d)
        (-> (.toInstant ^java.util.Date d)
            (.atZone (ZoneId/systemDefault))
            (.toLocalDateTime))

as:

        (instance? java.time.Instant d)
        (-> (.atZone ^java.time.Instant d (ZoneId/systemDefault))
            (.toLocalDateTime))

I was a bit surprised this wasn't already supported when I got "Execution error (IllegalArgumentException) at selmer.filters/fix-date (filters.clj:75).
2024-05-30T18:54:50.768632311Z is not a valid date format."

I also noticed that the comment on the |date filter talks about Joda Time... which isn't supported by Selmer any more, right?

@yogthos
Copy link
Owner

yogthos commented May 30, 2024

Oh yeah, code got updated to use Instant, but looks like the doc string never got updated. I agree with supporting Instant along with Date.

@seancorfield seancorfield self-assigned this May 30, 2024
@seancorfield
Copy link
Collaborator Author

OK, will work on a PR...

@yogthos
Copy link
Owner

yogthos commented May 30, 2024

Oh, I just pushed up 1.12.60 with the fix. :)

@yogthos
Copy link
Owner

yogthos commented May 30, 2024

But if there's anything else you'd like to add PR is welcome as well.

@yogthos
Copy link
Owner

yogthos commented May 30, 2024

and pushed up 1.12.61 with your changes added in, thanks for the quick PR

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

No branches or pull requests

2 participants