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

Set datetime time zone if TZID propery present #53

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

silverfoxdoc
Copy link
Contributor

I worked on this a while ago and came back to this in the last few days with newed enthusiasm.

I noticed that calendars which have TZID property (apple calendar and I think outlook as well) were not being allocated to the stated TZID property time zone.

Needed to do some more reading to improve my base R code knowledge but think I've sorted it now to allocate TZID property time zone and then convert to local time zone.

I have extracted each TZID property timezone individually for each column and then mapped it to same columns; I don't think most calendar software will allow different timezones to be set for different events within same calendar but I noticed when testing that if you do that, you get multiple DTSTART/DTEND columns with the different TZID properties so this would hopefully future proof it so to speak. It wouldn't merge them but into single column but user could probably do that themselves if desired.

See what you think and if it should be included.

Copy link
Member

@Robinlovelace Robinlovelace left a comment

Choose a reason for hiding this comment

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

Great to see this, many thanks. Very glad to see the tests also.

@Robinlovelace
Copy link
Member

Can you re-document with

devtools::document()

so checks pass?

@Robinlovelace
Copy link
Member

You can check locally with

devtools::check()

Looking great btw.

@Robinlovelace
Copy link
Member

We're getting an error message in the examples now @silverfoxdoc, any ideas?

https://github.com/ATFutures/calendar/actions/runs/8902573330/job/24448742581#step:6:170

@silverfoxdoc
Copy link
Contributor Author

looks like trying to add this feature has broken the ability to write an ical object out to .ics file; I don't have a lot of time spare at present but hopefully might be able to come back to this in a about a month or so

@Robinlovelace
Copy link
Member

OK, thanks @silverfoxdoc and no worries. I'm confident this is a + for the package so will also aim to take a look when time allows 🤞

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