Strong types + no longer ignore the VALUE parameter

[stlaz]

Hi,

In this maybe a bit controversial patch, I introduce strong types to the python-icalendar library. Strong types are required in the RFC 5545 as discussed in the issue #187.

This implementation allows the parser not to ignore the VALUE parameter of properties anymore. As a positive side effect of that, it also allows the VALUE parameter to be recognized even for unknown properties thus representing their content properly.

Contrary to my original thought, I am keeping vDDDTypes as default type for vDDDList should none other be set for it. Therefore, this patch makes use of #189 pull request and should it be accepted the other pull request may be closed.

Also, even though the tests pass, there might be bugs so I beg for a careful review.

[stlaz]

For some reason, the tests fail on https://github.com/collective/icalendar/blob/master/src/icalendar/tests/test_timezoned.py#L138, which I believe is not my doing.

[geier]

I'm unsure if this should be the only behaviour or if we should introduce a lax- or strict- mode parsing. As this is breaks backwards compatibility (at least in the current form or with strict-mode parsing as the default) we should probably discuss if we want to break compatibility in master or create a new branch for that.

I might to some actual code review later.

[stlaz]

Well, it might be difficult to separate lazy and strict modes just here as you would get different behaviors for unknown properties based on strictness of parsing the input.

However, it might be helpful to further improve error handling so that when exception is raised during parsing of a property (and ignore_exceptions is true) this property would still be assigned the original value in the representation although as a text so that the user knows there was an error (from the component .errors) and is able to handle it appropriately.

Also, thanks for the review in advance :)

[geier]

I haven't got anything of meaning to add to this, looks good.

So instead, I'll start bikeshedding on line breaks:
I find lines like those rather ugly:

vals = factory(factory.from_ical(vals,
                                 params['TZID'])

and personally prefer:

vals = factory(factory.from_ical(
     vals,  params['TZID'])

or

vals = factory(
    factory.from_ical(vals, params['TZID']))

[stlaz]

Thanks, that's actually pretty valid point, I had trouble tetrissing it in. I haven't had much time recently to fix it but will hopefully get to it soon.

[stlaz]

http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html states that the agreement does not cover github/collective/ code.
Also, the Changelog verifier seems broken.

[thet]

@stlaz @geier @untitaker sorry guys for not being part of the discussion. I can't really help out in the moment. I trust your mutual reviews and try to follow what happens.

[thet]

Something else: @stlaz I added you as a collaborator to this repository with read/clone/push rights. If you dont mind. That way, you're able to merge others pull requests.

[stlaz]

Thank you. It's quite a responsibility and I'll still keep the non-trivial pushes to those of you who maintain the repo but may merge the simpler things when necessary :)

[gforcada]

@stlaz both checks (changelog entry and contributors agreement) should be fixed. If you make a change on the commits we will see it.

Thanks for noticing and reporting it!

[thet]

@stlaz please rebase your PR with master.

does this introduce a backwards incompatibility, so that this should go into a 4.0 release? if not, i'd go for a 3.11 release.

[stlaz]

Rebased.
There are some changes in the TypesFactory class that might be considered breaking bacward compatibility maybe.
Also, some strings with wrong VALUE type at a property will not be accepted now even though they would have been before these changes (note that the property's VALUE parameter would have been completely ignored before). This may require even better error handling that would not remove the original value of a property on parse fail but rather force it as a vText/string.

[stlaz]

I added all the changes to #196 which is now rebased on the branch which is designated for 4.0 development.