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