Better charset, unicode and image support for RTF files
ClosedPublic

Authored by pvuorela on Oct 25 2019, 1:44 PM.

Details

Summary

Combining work from Andrew den Exter and Mikhail Filippov.
Supports better different encodings, images and unicode literals.
Also added handling for \line.

Diff Detail

Repository
R8 Calligra
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
pvuorela created this revision.Oct 25 2019, 1:44 PM
Restricted Application added a project: Calligra: 3.0. · View Herald TranscriptOct 25 2019, 1:44 PM
Restricted Application added a subscriber: Calligra-Devel-list. · View Herald Transcript
pvuorela requested review of this revision.Oct 25 2019, 1:44 PM

I've made some suggestions and have a couple of queries, but they're all very minor.

filters/words/rtf/import/3rdparty/rtf-qt/src/DocumentDestination.cpp
125

A minor issue, but the spacing around the brackets here doesn't match the surrounding code.

filters/words/rtf/import/3rdparty/rtf-qt/src/PictDestination.cpp
46

The spec mentions \wbitmap is also a Windows device-dependent bitmap. Could that be sent through this branch too?

(see: http://latex2rtf.sourceforge.net/rtfspec_7.html#rtfspec_24)

54–55

The \picw and \pich keywords are now ignored; is this intentional? It looks like they're mandatory, whereas the \picwgoal, \pichgoal, \picscalex and \picscaley which seem to be used now instead, are optional.

(see: http://latex2rtf.sourceforge.net/rtfspec_7.html#rtfspec_24)

110

Lines 87-110 seem to use different spacing around the brackets than elsewhere.

filters/words/rtf/import/3rdparty/rtf-qt/src/TextDocumentRtfOutput.cpp
68

The tabs/spaces are all over the place in this file already, but it probably makes sense to stick to one or the other nevertheless (spaces by the looks of it).

This revision is now accepted and ready to land.Oct 28 2019, 2:25 PM
pvuorela added inline comments.Oct 28 2019, 3:04 PM
filters/words/rtf/import/3rdparty/rtf-qt/src/DocumentDestination.cpp
125

Sure.

filters/words/rtf/import/3rdparty/rtf-qt/src/PictDestination.cpp
46

I'd maybe add new keywords in separate commits later based on needs.

54–55

Was Andrew's part, but I think file itself should contain the geometry and goals are handled here as optional, i.e. if not set will be read from the file.

110

Yea, could change these too. Though have been pondering of just feeding the whole rtf-qt through astyle to get consistent formatting and whitespace.

filters/words/rtf/import/3rdparty/rtf-qt/src/TextDocumentRtfOutput.cpp
68

Yea, the whole module here is a mess regarding whitespace. Now using the same formatting as surrounding code.

pvuorela updated this revision to Diff 68888.Oct 28 2019, 3:10 PM

Whitespace adjustments

denexter added inline comments.
filters/words/rtf/import/3rdparty/rtf-qt/src/PictDestination.cpp
54–55

I think picw and pich are source sizes and somewhat meaningless since that information is encoded in the image. The goal size and scale combine to give the display size.

filters/words/rtf/import/3rdparty/rtf-qt/src/PictDestination.cpp
54–55

Thanks for the explanation, that makes sense. Probably these control words are more important for WMFs and the like.

101

The logic is confusing me here I'm afraid. If there's no \picwgoal control word, then m_goalWidth will default to 0 and the code above will be skipped. The image width will then be set to 0 on line 104: m_imageFormat.setWidth( 0 );. Similarly for height. Is this correct? It feels like it might make more sense for the conditions to be reversed.

110

I think it needs it, but I appreciate that's not really related to this change.

pvuorela marked an inline comment as done.Oct 29 2019, 12:46 PM
pvuorela added inline comments.
filters/words/rtf/import/3rdparty/rtf-qt/src/PictDestination.cpp
101

Hm, now that you mention it, looks peculiar indeed. Basically it's here overwriting whatever values got read with pic*goal.

pvuorela updated this revision to Diff 68972.Oct 29 2019, 12:53 PM

Update image size reading conditions

This looks like a nice and useful collection of changes to me.

This revision was automatically updated to reflect the committed changes.