Unescape Value-Type: TEXT in Golang Model #91
Conversation
brenank
force-pushed
the
master
branch
3 times, most recently
from
0a707b2 to
dc3ed8d
Compare
|
@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. |
|
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 |
|
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
force-pushed
the
master
branch
4 times, most recently
from
1c9036c to
ec40c25
Compare
Created a separate PR #92. Rebased this PR on top of those changes. |
|
Let's sort out the actions PR first. |
brenank
force-pushed
the
master
branch
3 times, most recently
from
aa56c89 to
bd0b9a7
Compare
|
I take it #92 is in this one too? |
This is a breaking change. Property.Value of value-type: TEXT is now unescaped in the deserialized model. Previously, this was escaped.
.gitignore
Outdated
| @@ -1 +1,2 @@ | |||
| /.idea/ | |||
| actual | |||
There was a problem hiding this comment.
I don't mind extra entries but this one seems like it's your side only, perhaps add it to your $PROJECT/.git/info/exclude list rather than $PROJECT/.gitignore ?
There was a problem hiding this comment.
The test outputs this folder if it fails for easy comparison and updating of the expected values. It should never be checked in so I figured it should be added to project level gitignore.
I can scope it down to be more specific if desired.
There was a problem hiding this comment.
I have no issue with the line being there, however it should be
/actual
so that it isn't applied throughout the entire directory structure. Just we aren't generating any runtime data for actual as far as I can tell so it might have been better in the .git/info/exclude file (which is per repo and local only.)
There was a problem hiding this comment.
scoped this more narrowly, since this does create files during runtime (only when running serialization test & only if there are failures)
|
All good! I'll make the suggested updates when I get some time. |
certain os methods were introduced in go1.16
Previously, the property convenience methods
Set...(someStrValue)expectedsomeStrValueto be an unescaped string, getting the value would require callingical.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 ofSet...&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 -> serializedtests 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