D25715: Use RTF default color as default Qt format

[Damien Caliste]

dcaliste added a comment.


  Just a suggestion to avoid adding a variable which meaning may be redundant with the state of an already existing one. Otherwise LGTM.

INLINE COMMENTS

> ColorTableDestination.cpp:26
>      ColorTableDestination::ColorTableDestination( Reader *reader, AbstractRtfOutput *output, const QString &name ) :
> -      Destination( reader, output, name )
> +      Destination( reader, output, name ), m_currentColor(Qt::black), m_colorSet(false)
>      {

We let the initialisation by default m_currentColor(QColor())

> ColorTableDestination.cpp:37
>  	if ( controlWord == "red" ) {
>  	    m_currentColor.setRed( value );
>  	} else if (controlWord == "green" ) {

Before setting the red value, we add:

  if (! m_currentColor.isValid())
    m_currentColor = QColor::Black;

To avoid uninitialised values in other channels.

> ColorTableDestination.cpp:39
>  	} else if (controlWord == "green" ) {
>  	    m_currentColor.setGreen( value );
>  	} else if (controlWord == "blue" ) {

The same for blue and green.

> ColorTableDestination.cpp:54
>  	if ( plainText == ";" ) {
> -	    m_output->appendToColourTable( m_currentColor );
> +	    m_output->appendToColourTable( m_colorSet ?  m_currentColor : QColor() );
>  	    resetCurrentColor();

Here we pass m_currentColor, inconditionaly, since the invalid status is equivalent to the flag being false.

> ColorTableDestination.cpp:63
>      {
>  	m_currentColor = Qt::black;
> +	m_colorSet = false;

And finally, we reset to QColor(), instead of black.

> ColorTableDestination.h:45
>  	QColor m_currentColor;
> +	bool m_colorSet;
>        };

I'm wondering, if we could use the invalid status of QColor as this flag ?

See later…

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25715

To: pvuorela, davidllewellynjones, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20191203/d6a5b06a/attachment.htm>