Differential D25715 Diff 72487 filters/words/rtf/import/3rdparty/rtf-qt/src/ColorTableDestination.cpp
Changeset View
Changeset View
Standalone View
Standalone View
filters/words/rtf/import/3rdparty/rtf-qt/src/ColorTableDestination.cpp
| Show All 17 Lines | |||||
| 18 | #include "ColorTableDestination.h" | 18 | #include "ColorTableDestination.h" | ||
| 19 | 19 | | |||
| 20 | #include "rtfreader.h" | 20 | #include "rtfreader.h" | ||
| 21 | #include "rtfdebug.h" | 21 | #include "rtfdebug.h" | ||
| 22 | 22 | | |||
| 23 | namespace RtfReader | 23 | namespace RtfReader | ||
| 24 | { | 24 | { | ||
| 25 | ColorTableDestination::ColorTableDestination( Reader *reader, AbstractRtfOutput *output, const QString &name ) : | 25 | ColorTableDestination::ColorTableDestination( Reader *reader, AbstractRtfOutput *output, const QString &name ) : | ||
| 26 | Destination( reader, output, name ) | 26 | Destination( reader, output, name ), m_currentColor(Qt::black), m_colorSet(false) | ||
dcaliste: We let the initialisation by default m_currentColor(QColor()) | |||||
| 27 | { | 27 | { | ||
| 28 | m_currentColor = Qt::black; // this is our default color | | |||
| 29 | } | 28 | } | ||
| 30 | 29 | | |||
| 31 | ColorTableDestination::~ColorTableDestination() | 30 | ColorTableDestination::~ColorTableDestination() | ||
| 32 | {} | 31 | {} | ||
| 33 | 32 | | |||
| 34 | void ColorTableDestination::handleControlWord( const QByteArray &controlWord, bool hasValue, const int value ) | 33 | void ColorTableDestination::handleControlWord( const QByteArray &controlWord, bool hasValue, const int value ) | ||
| 35 | { | 34 | { | ||
| 35 | bool handled = true; | ||||
| 36 | if ( controlWord == "red" ) { | 36 | if ( controlWord == "red" ) { | ||
| 37 | m_currentColor.setRed( value ); | 37 | m_currentColor.setRed( value ); | ||
Before setting the red value, we add: if (! m_currentColor.isValid()) m_currentColor = QColor::Black; To avoid uninitialised values in other channels. dcaliste: Before setting the red value, we add:
```
if (! m_currentColor.isValid())
m_currentColor =… | |||||
| 38 | } else if (controlWord == "green" ) { | 38 | } else if (controlWord == "green" ) { | ||
| 39 | m_currentColor.setGreen( value ); | 39 | m_currentColor.setGreen( value ); | ||
dcaliste: The same for blue and green. | |||||
| 40 | } else if (controlWord == "blue" ) { | 40 | } else if (controlWord == "blue" ) { | ||
| 41 | m_currentColor.setBlue( value ); | 41 | m_currentColor.setBlue( value ); | ||
| 42 | } else { | 42 | } else { | ||
| 43 | handled = false; | ||||
| 43 | qCDebug(lcRtf) << "unexpected control word in colortbl:" << controlWord; | 44 | qCDebug(lcRtf) << "unexpected control word in colortbl:" << controlWord; | ||
It looks like there may be a distinction to be made between an empty control word and an unhandled control word. In the case of an unhandled control word, clearing the colour seems sensible, but then in the case of an empty control word, the debug output isn't needed. davidllewellynjones: It looks like there may be a distinction to be made between an empty control word and an… | |||||
pvuorela: Empty control word? Don't think that should be happening? | |||||
Or well, the code might return such, but think the file in that case is broken and as such ok to complain about on debug. Also a bit separate from the changes here. pvuorela: Or well, the code might return such, but think the file in that case is broken and as such ok… | |||||
Yeah, this was my misunderstanding of the spec. I don't see a problem with what you're doing here. davidllewellynjones: Yeah, this was my misunderstanding of the spec. I don't see a problem with what you're doing… | |||||
| 44 | } | 45 | } | ||
| 46 | if ( handled ) { | ||||
| 47 | m_colorSet = true; | ||||
| 48 | } | ||||
| 45 | } | 49 | } | ||
| 46 | 50 | | |||
| 47 | void ColorTableDestination::handlePlainText( const QByteArray &plainText ) | 51 | void ColorTableDestination::handlePlainText( const QByteArray &plainText ) | ||
| 48 | { | 52 | { | ||
| 49 | if ( plainText == ";" ) { | 53 | if ( plainText == ";" ) { | ||
| 50 | m_output->appendToColourTable( m_currentColor ); | 54 | m_output->appendToColourTable( m_colorSet ? m_currentColor : QColor() ); | ||
Here we pass m_currentColor, inconditionaly, since the invalid status is equivalent to the flag being false. dcaliste: Here we pass m_currentColor, inconditionaly, since the invalid status is equivalent to the flag… | |||||
| 51 | resetCurrentColor(); | 55 | resetCurrentColor(); | ||
| 52 | } else { | 56 | } else { | ||
| 53 | qCDebug(lcRtf) << "unexpected text in ColorTableDestination:" << plainText; | 57 | qCDebug(lcRtf) << "unexpected text in ColorTableDestination:" << plainText; | ||
| 54 | } | 58 | } | ||
| 55 | } | 59 | } | ||
| 56 | 60 | | |||
| 57 | void ColorTableDestination::resetCurrentColor() | 61 | void ColorTableDestination::resetCurrentColor() | ||
| 58 | { | 62 | { | ||
| 59 | m_currentColor = Qt::black; | 63 | m_currentColor = Qt::black; | ||
| 64 | m_colorSet = false; | ||||
dcaliste: And finally, we reset to QColor(), instead of black. | |||||
| 60 | } | 65 | } | ||
| 61 | } | 66 | } | ||
We let the initialisation by default m_currentColor(QColor())