Fix unicode handling in 'View HTML code as formated text'. Fixes #667 #679

2014-06-02T19:15:49+02:00

enikesha commented

2014-06-02 19:15:49 +02:00

(Migrated from github.com)

Implemented like in main view (def tableWidgetInboxItemClicked(self)) except for substitution logic.

Implemented like in main view (`def tableWidgetInboxItemClicked(self)`) except for substitution logic.

bmng-dev commented

2014-06-03 06:55:08 +02:00

(Migrated from github.com)

Seems fine to me as is, however there are some tiny performance improvements that can be made.

In the first for loop change the string literals to unicode literals. When operating on a string and unicode string the string is converted to unicode. Since the if tests are being performed on every line if there are 100 lines the string literals are being converted to unicode 100 times.

These other improvements don't really have anything to do with this fix but while we are talking about this section of code:

The second for loop should be replaced with the more efficient unicode join content = u''.join(lines)
The content.replace call is useless as all new lines were removed in the split operation to make the lines list. Since it has been useless for some time it can be safely removed. However if it is actually supposed to do something then maybe changing the join I suggested above to u'<br>'.join(lines) might achieve the desired outcome.

Seems fine to me as is, however there are some tiny performance improvements that can be made. In the first for loop change the string literals to unicode literals. When operating on a string and unicode string the string is converted to unicode. Since the if tests are being performed on every line if there are 100 lines the string literals are being converted to unicode 100 times. These other improvements don't really have anything to do with this fix but while we are talking about this section of code: - The second for loop should be replaced with the more efficient unicode join `content = u''.join(lines)` - The `content.replace` call is useless as all new lines were removed in the split operation to make the lines list. Since it has been useless for some time it can be safely removed. However if it is actually supposed to do something then maybe changing the join I suggested above to `u'<br>'.join(lines)` might achieve the desired outcome.

enikesha commented

2014-06-03 07:50:34 +02:00

(Migrated from github.com)

Thank you for your input! With regards to the performance I've decided that late unicode decoding would work better. And you're right, double newlines weren't handled properly, so I've tried to implement original logic.

bmng-dev commented

2014-06-03 08:26:39 +02:00

(Migrated from github.com)

A better solution than I suggested. Good work.

This repo is archived. You cannot comment on pull requests.

No reviewers