search

arran4 / golang-ical

A ICS / ICal parser and serialiser for Golang.
Apache License 2.0
279 stars 72 forks source link

Unescape Value-Type: TEXT in Golang Model #91

Closed brenank closed 1 month ago

brenank commented 1 month ago

Previously, the property convenience methods Set...(someStrValue) expected someStrValue to be an unescaped string, getting the value would require calling ical.FromText(event.GetProperty(ical.ComponentProperty...).Value)

While adding convenience functions for Get...() was an option, the specification states that this should be applied to all property values with a value-type of "TEXT", which would require a lot of Set... & Get... convenience functions. It seemed cleaner to move the FromText/ToText calls into the de/serialization logic, which would clean up the public API to be consistent as entirely unescaped.

This also adds an additional loop & sort of all Property Parameters to keep the serialization stable. Mainly added for testing purposes, but if this is not desired, the compare serialized -> deserialized -> serialized tests could be removed.

Also corrected a small serialization bug, where VFREEBUSY properties were being serialized out as VBUSY.

See: https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.11

arran4 commented 1 month ago

@brenank Test failings are not yours it seems that the action is failing. I will try again in 24 hours, if it doesn't work then I will dig in and fix the tests.

brenank commented 1 month ago

@brenank Test failings are not yours it seems that the action is failing. I will try again in 24 hours, if it doesn't work then I will dig in and fix the tests.

Ah seems it failed the linting job. Created a fix.

brenank commented 1 month ago

Yeah it seems the GHAs for setup-go are failing. Might be worth pinning it to the version listed in go.mod: https://github.com/actions/setup-go?tab=readme-ov-file#getting-go-version-from-the-gomod-file And maybe rev'ing to the latest version.

arran4 commented 1 month ago

Happy for you to add that to the PR (or a separate if you want.) Otherwise I will dig in when I have time.

brenank commented 1 month ago

Happy for you to add that to the PR (or a separate if you want.) Otherwise I will dig in when I have time.

Created a separate PR https://github.com/arran4/golang-ical/pull/92. Rebased this PR on top of those changes.

arran4 commented 1 month ago

Let's sort out the actions PR first.

arran4 commented 1 month ago

I take it #92 is in this one too?

brenank commented 1 month ago

I take it #92 is in this one too?

Yeah It did, I just rebased on top of master, so it removed the commits from #92 for easier review.

brenank commented 1 month ago

All good! I'll make the suggested updates when I get some time.

arran4 commented 1 month ago

https://github.com/arran4/golang-ical/releases/tag/v0.3.0 Done