search

arran4 / golang-ical

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

Prefix mailto: for email in organizer property #88

Closed meain closed 9 months ago

meain commented 9 months ago

Organizer property should be similar to how attendee is handled.

https://github.com/arran4/golang-ical/blob/0fcebed54126830e648865ec05e459ad213986df/components.go#L260-L262

arran4 commented 9 months ago

Agreed. Consistency is king, also the spec is very clear on this too: https://www.kanzaki.com/docs/ical/calAddress.html I'm not sure if it can be something else.

This might break some existing deployments. I will get back to this, in a couple days.

arran4 commented 9 months ago

Need to delay, interview prep.

meain commented 9 months ago

I don't know if this is "clean", but if you have concerns about the user sending in email with "mailto:" prefix, I think we can probably consider only conditionally adding it by checking the prefix.

arran4 commented 9 months ago

It makes sense and if there is an issue someone can submit an issue and we can add a new code path. There is almost no need for duplication. : is not part of dot-atom https://datatracker.ietf.org/doc/html/rfc2822#section-3.2.4 so I think we are home free with that one.

meain commented 9 months ago

Sounds good, I've updated it to only conditionally add it. Also updated the attendee one to be similar. Let me know if I should revert how attendee works to previous version.

arran4 commented 9 months ago

But other than that looks good.

meain commented 9 months ago

Spent a bit more time looking into the spec. Looks like it is possible to have prefixes other than "mailto:" here. A CAL-ADDRESS can be a URI and if you see the doc, a URI can be more than just mailto. I think it might be incorrect to automatically add mailto: prefix after all 😅 . For my personal use-case, I'll always need the mailto: prefix and so I am fine either way. I'll let you decide how we proceed here.

mailto: is capitalized in the rfc might as well stick to that too.

I see the examples in the RFC to be using lowercase. I also see that there has been a previous discussion about this here.

If there is a string const use that?

There is none defined as of now, but I can add one if we are certain about adding the mailto: prefix.

arran4 commented 9 months ago

haha yep. I'm in a loop. Okay. Nothing more is necessary.

arran4 commented 9 months ago

Thanks for looking that up. I have a shocking memory it seems.

There you go:

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