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

Fix date creation #82

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Fix date creation #82

merged 1 commit into from
Apr 26, 2021

Conversation

valtlai
Copy link
Contributor

@valtlai valtlai commented Apr 26, 2021

The Date constructor does not support undefined as an argument, so we must specify the default values ourselves.

Also

  • allow a lowercase t as the date-time separator
  • shorten the regexes by using \d\d instead of \d{2}
  • reorder the comparison operators in comments to match the ones in switchcase.

The `Date` constructor does not support `undefined` as an argument, so we must specify the default values ourselves.

Also
* allow a lowercase `t` as the date-time separator
* shorten the regexes by using `\d\d` instead of `\d{2}`
* reorder the comparison operators in comments to match the ones in `switch`–`case`.
@oscarotero
Copy link
Member

The idea was to use the current time values for missing (or undefined) arguments. For example, new Date(year, month, day) creates a date with the current time (to hours, minutes and seconds). Your change forces the time to 0:00:00. Maybe it makes more sense because using the current time is a bit random...

@valtlai
Copy link
Contributor Author

valtlai commented Apr 26, 2021

Yeah, I think it’s random. If I write date>=2021-04, I want a page with date: 2021-04-01T00:00:00 to always match, regardless of the current date or time.

Hmm, the Date constructor doesn’t seem to use the current date time for the missing values, but replaces them with default values instead, like 1 for the day of the month or 0 for hours. (I tested with Chrome, which uses the same V8 JavaScript engine as Deno. See also MDN.) Using undefined (as we’re doing with hour && parseInt(hour)) results Invalid Date.

@oscarotero
Copy link
Member

Yes, you're absolutely right. I don't know why I've assumed that it would take the current value.

@oscarotero oscarotero merged commit 5e0243b into lumeland:master Apr 26, 2021
@valtlai valtlai deleted the date-fix branch April 26, 2021 21:50
@oscarotero
Copy link
Member

Btw, as you are doing so many valuable PR to this project, are you interested in joining to lumeland organization as a contributor? I can invite you if you like :)

@valtlai
Copy link
Contributor Author

valtlai commented Apr 26, 2021

That would be nice 😄

@valtlai
Copy link
Contributor Author

valtlai commented Apr 26, 2021

Thanks for inviting :)

@oscarotero
Copy link
Member

You're welcome!!

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.

2 participants