davidllewellynjones accepted this revision.
davidllewellynjones added a comment.
This revision is now accepted and ready to land.
I've made some suggestions and have a couple of queries, but they're all very
minor.
INLINE COMMENTS
> DocumentDestination.cpp:125
> + m_charactersToSkip = m_unicodeSkip;
> + } else if (controlWord == "uc" && hasValue) {
> + m_unicodeSkip = value;
A minor issue, but the spacing around the brackets here doesn't match the
surrounding code.
> PictDestination.cpp:46
> + m_format = "png";
> + } else if ( controlWord == "dibitmap" ) {
> + qCDebug(lcRtf) << "BMP";
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)
> PictDestination.cpp:54
> } else if ( controlWord == "pich" ) {
> qCDebug(lcRtf) << "pict height: " << value;
> } else if ( controlWord == "picscalex" ) {
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)
> PictDestination.cpp:110
> + qCWarning(lcRtf) << "Embedded picture in unknown format";
> + }
> }
Lines 87-110 seem to use different spacing around the brackets than elsewhere.
> TextDocumentRtfOutput.cpp:68
> + {
> + m_cursor->insertText(str);
> }
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).
REPOSITORY
R8 Calligra
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D24943
To: pvuorela, davidllewellynjones
Cc: davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, vandenoever
