Code review comment for lp://staging/~submarine/unity-scope-evolution/evolution-previews

A few style notes:

+ if line.endswith('\r'):
+ line = line.replace('\r', '')

[1]
If you want to split a string into lines where some may have DOS style line endings, use icalstring.splitlines() rather than icalstring.split('\n'). That way you won't have to check for '\r'.

Furthermore, there seems to be two places where you parse the text version of the iCalendar event, and you've only made this change in one place. Wouldn't it affect both cases?

Also, is it possible to use the existing methods on the CalComponent to get the dates, location and ID directly?

--

summary = ECalendar.CalComponentText()
try:
    component.get_summary(summary)
    summary = summary.value
except Exception as error:
- print(error)
+ #print('Summary Error: %s' % error)
    summary = ''

[2]
What exception are you catching here? It isn't clear from the C header file what exception could be raised here. Presumably it only occurs on get_summary() call too, so it would be cleaner to do something like:

    summary = ''
    try:
        value = ECalendar.CalComponentText()
        component.get_summary(value)
    except ???:
        pass
    else:
        summary = value.value

--

+ description = []
+ try:
+ description = component.get_description_list()
+ description = description[0].value
+ except IndexError:
+ description = ''
+ except AttributeError:
+ description = ''

[3]
Rather than checking for IndexError, it would be better to check that the length of the description list. And what AttributeError are you protecting against here?

--

+ dtend = int(datetime.datetime.timestamp(dtend))

[4]
datetime.timestamp() is a method of the datetime type, so "int(dtend.timestamp())" should give the same result.

[5]
You're assigning a different type to the same variable. While Python allows this, it does make your code harder to understand. Since both dtstart and dtend are only accessed once more after these conversions, why not just perform the conversion when creating the result dictionary?

--

+ preview.props.description_markup += _('<b>Description:</b>\n%s' % self.result.metadata['description'].get_string())

[6]
What happens if the description string contains special characters? (<, > or &) Presumably the description needs to be passed through an escape routine.

[7]
The i18n here will not work, since the user data is being substituted into the string prior to translation. Instead, you should use:

   _('<b>Description</b>:\n%s') % ...