Skip to content

Included timezone shift #7

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

Open
RideLikeTheWind wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Included timezone shift #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,14 @@ module.exports.parseString = function(st, max) {
events[event_count][k].value = getDate(params["VALUE"], value);
} else {
if(params["TZID"] !== undefined) {
//Allow to parse as per GMT
events[event_count][k].value = getDate(undefined, value.concat('Z'));
//If a locale exists, strip preceeding /
if(params["TZID"].search(/\//) != -1){
Copy link
Owner

@Christop406 Christop406 Mar 1, 2023

Choose a reason for hiding this comment

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

I am not sure this does exactly what you want it to do, it seems that this check will find any / in the TZID then take a substring, so, for example, wouldn't this:

America/Los_Angeles

match for your .search(...) call? Which would then end up with merica/Los_Angeles?

I think you would just want to check if the string starts with /, which simplifies the call to if(params["TZID"].startsWith('/'))

Copy link
Author

@RideLikeTheWind RideLikeTheWind Mar 1, 2023

Choose a reason for hiding this comment

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

Yes agree re the search. I'll make the change to the code and commit the change. I'll also write a test case for it and submit. Thanks for the advice.

Copy link
Owner

@Christop406 Christop406 Mar 1, 2023

Choose a reason for hiding this comment

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

Sounds good - I'm about to head on a trip for a bit over a week tomorrow so I may go quiet here, but I will try my best to come back to this promptly after my trip. I hope you have a workaround for the time being.

params["TZID"] = params["TZID"].slice(1);
}
//Pass to toLocaleString to reduce to GMT based on timezone
events[event_count][k].value.toLocaleString(undefined, {timeZone: params["TZID"]});
Copy link
Owner

@Christop406 Christop406 Mar 1, 2023

Choose a reason for hiding this comment

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

What does this line do? toLocaleString doesn't set anything, so I think the return of this function is unused.

Copy link
Author

@RideLikeTheWind RideLikeTheWind Mar 1, 2023

Choose a reason for hiding this comment

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

My understanding is it returns a correctly coded datetime using the correct locale.

} else {
events[event_count][k].value = getDate(undefined, value);
}
Expand Down