collective / icalendar

icalendar parser library for Python
https://icalendar.readthedocs.io/en/latest/
Other
976 stars 167 forks source link

string conversion of summary field converts "," to "\," #127

Open schaefer0 opened 10 years ago

schaefer0 commented 10 years ago

when the calendar event summary field contains a string with a comma, for example "key=value1,value2" the to_ical() function converts the above string to: "key=value1\,value2"

Is this a bug or a feature?

thanks - bob s.

untitaker commented 10 years ago

Skimming over the iCalendar RFC it seems to be a feature, at least for value type TEXT. But it is entirely dependent on the key you're having!

pdav commented 8 years ago

In src/icalendar/prop.py, the 'categories' entry should not have type 'text' (in types_map array) since (quoting @geier in https://github.com/pimutils/khal/pull/414):

in the RFC the example for multiple categories (CATEGORIES:APPOINTMENT,EDUCATION) is comma separated, while the current implementation here escapes the comma with a backslash (example: CATEGORIES:CatA\,CatB)

More specifically, method vText.to_ical() (in src/icalendar/prop.py) calls escape_char (in src/icalendar/parser.py) which escapes ",".

I guess a new type (list ?) should be introduced.

stlaz commented 8 years ago

You can read from the quoted RFC that the 'text' type in 'categories' entry is actually right. There's no type "list" in iCalendar, there's just multiple values, but that's not applicable to TEXT values. As for escaping the commas, it seems like a bug to me. If the from_ical method gets text with unescaped "," it should not try to escape it.

pdav commented 8 years ago

The quoted RFC is ambiguous about the type TEXT since this type may be both a single value or a list.

Quoting the TEXT specification: If the property permits, multiple TEXT values are specified by a COMMA-separated list of values.

Thus, some properties (e.g. SUMMARY) are single TEXT with the requirement to escape the comma, and some properties are multiple TEXT values (e.g. CATEGORIES) which need comma-quoting on single values and not on the comma used as a separator.

From my point of view, icalendar should either:

stlaz commented 8 years ago

Ah, sorry, you're right, I did not notice that and it actually makes sense why the comma should be escaped in TEXT, then. The value type of CATEGORIES is still TEXT, though. I believe that with https://github.com/collective/icalendar/pull/192, the vDDDLists class may actually be able to handle vText lists although some changes in the TypesFactory will probably be required. The parsing of TEXT values would still need to be fixed first as "\," != "," on the input of from_ical methods.