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 newDate() and option for reverted back Now() date format #1534

Merged
merged 6 commits into from
Jun 10, 2023

Conversation

alsundukov
Copy link
Contributor

newDate() for Safari (et. al.)
Supported formats:
"2022.01.10 04:10",
"2022.01.10 04:10:11",
"2022.01.10 04:10:11.123",
"2022-01-10 04:10",
"2022-01-10 04:10:11",
"2022-01-10 04:10:11.123",
"2022.1.10",
"2022-1-1"
...

Fix for NOW() in Firefox: option for reverted back the date format: 2022-11-01 -> 2022.11.01

@mathiasrw
Copy link
Member

Please correct file formatting using yarn format and try again.

@alsundukov alsundukov force-pushed the fix_newDate branch 2 times, most recently from 50aadda to 70c5061 Compare October 28, 2022 18:21
@mathiasrw
Copy link
Member

@alsundukov Looks really good. I have a few questions. Please check the comments in the code.

newDate() for Safari (et. al.)
Supported formats:
"2022.01.10 04:10",
"2022.01.10 04:10:11",
"2022.01.10 04:10:11.123",
"2022-01-10 04:10",
"2022-01-10 04:10:11",
"2022-01-10 04:10:11.123",
"2022.1.10",
"2022-1-1"
...

Fix for NOW() in Firefox: reverted back the date format: 2022-11-01 -> 2022.11.01
@alsundukov
Copy link
Contributor Author

@mathiasrw I have shortened the comment.

src/17alasql.js Outdated Show resolved Hide resolved
src/61date.js Outdated Show resolved Hide resolved
src/61date.js Outdated Show resolved Hide resolved
src/61date.js Outdated Show resolved Hide resolved
test/test845.js Show resolved Hide resolved
src/61date.js Outdated Show resolved Hide resolved
@mathiasrw
Copy link
Member

mathiasrw commented Jun 10, 2023

I really like the work you did here. Sorry that it took me so long to submerge myself into the details. I felt we had to get timezones included into this. On the way I also got around to understand the details in https://stackoverflow.com/questions/2587345/why-does-date-parse-give-incorrect-results/20463521#20463521 and its clear that we should rely mainly on our own parsing to combat inconsistencies.

I see there is an error now in the cicd that I did not get locally. Looking into this tomorrow and hope its not a "timezone of the computer where we are testing" thing.

On a side note: I now see why #1627 is happening and will include that in the next release so we can bundle two breaking changes.

@mathiasrw mathiasrw changed the base branch from develop to pr-1534 June 10, 2023 13:30
@mathiasrw mathiasrw changed the base branch from pr-1534 to develop June 10, 2023 13:32
@mathiasrw mathiasrw merged commit 6a4241a into AlaSQL:develop Jun 10, 2023
mathiasrw added a commit that referenced this pull request Jun 12, 2023
mathiasrw pushed a commit that referenced this pull request Jun 12, 2023
* Better date support for Safari
* Config separators for now() output
* Support timezone timestamps
@mathiasrw
Copy link
Member

This commit was moved to the v5 branch so we can prepare a few breaking changes in one go and release together.

akhaneev added a commit to akhaneev/alasql that referenced this pull request Jul 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.

2 participants