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 for all day events (#55) #61

Closed
wants to merge 2 commits into from
Closed

Fix for all day events (#55) #61

wants to merge 2 commits into from

Conversation

cuvar
Copy link

@cuvar cuvar commented Jan 23, 2023

Hopefully, this implementation should fix #55. At least, this solutions seems to have worked for me. I took @kevinmorio's approach mentioned in the issue here.

@arran4
Copy link
Owner

arran4 commented Jan 23, 2023

@cuvar Thanks. I will get back to you in a couple days. I will have to check the RFC(s) for what it says it should be too.

However;

	icalDateFormatLocal      = "20060102"
	icalDateFormat      		 = "20060102"

They are the same value.

@cuvar
Copy link
Author

cuvar commented Jan 24, 2023

cool, thanks! I'll add another commit regarding the icalDateFormatLocal and icalDateFormat

@arran4
Copy link
Owner

arran4 commented Jan 29, 2023

Thanks. @cuvar What I'm thinking is that this changes behaviour, so how about we just duplicate the functions and append Local (Edit; actually it should really be UTC as it's not in the local timezone, we would have to replace the UTC() and ask for the timezone they want to use; and if we do that it should be appended with InLocation instead of Local or UTC) to them so we don't impact existing users. We also haven't considered deserialization here but I believe the function is more flexible.

Another approach would be to make it do a better job at detecting the format of the date/time being used.

@cuvar
Copy link
Author

cuvar commented Feb 4, 2023

I'm sorry, but unfortunately I won't have the time to dive into the code in the next time. Therefore, I just created this shallow fix. If there's more time, I'll have a look into it again.

@arran4
Copy link
Owner

arran4 commented Feb 5, 2023

@cuvar Thanks though. Perhaps someone else will come and pick up this PR. Did you get your problem solved at all? If not I can figure something out. What timezone are you?

@arran4 arran4 added the help wanted Extra attention is needed label Feb 13, 2023
@cuvar
Copy link
Author

cuvar commented Feb 17, 2023

Thanks! Yes, it seems to have solved my issue. My timezone is UTC+1

@cuvar cuvar closed this by deleting the head repository Aug 15, 2023
@arran4
Copy link
Owner

arran4 commented Aug 15, 2023

I will still look into this when I get some time to work on this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All Day Events
2 participants