D25715: Use RTF default color as default Qt format

This revision was automatically updated to reflect the committed changes. Closed by commit R8:959027525581: Use RTF default color as default Qt format (authored by pvuorela). REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25715?vs=70828&id=72487 REVISION DETAI

D25715: Use RTF default color as default Qt format

davidllewellynjones accepted this revision. davidllewellynjones added a comment. This revision is now accepted and ready to land. This looks good to me, and certainly gave better results for me using files with the 'auto' colour. INLINE COMMENTS > pvuorela wrote in ColorTableDestination.cpp:

D25715: Use RTF default color as default Qt format

pvuorela added inline comments. INLINE COMMENTS > pvuorela wrote in ColorTableDestination.cpp:44 > 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 separ

D25715: Use RTF default color as default Qt format

pvuorela added inline comments. INLINE COMMENTS > davidllewellynjones wrote in ColorTableDestination.cpp:44 > 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

D25715: Use RTF default color as default Qt format

davidllewellynjones added inline comments. INLINE COMMENTS > ColorTableDestination.cpp:44 > + handled = false; > qCDebug(lcRtf) << "unexpected control word in colortbl:" << > controlWord; > } It looks like there may be a distinction to be made between an empty control

D25715: Use RTF default color as default Qt format

dcaliste added a comment. @pvuorela as you prefer, anyway, it's a question of taste, I find it myself easier to follow if the information is on one variable only. But your solution is good also, the file is small and is kept small so easy to follow. REPOSITORY R8 Calligra REVISION DETAIL

D25715: Use RTF default color as default Qt format

pvuorela added inline comments. INLINE COMMENTS > dcaliste wrote in ColorTableDestination.h:45 > I'm wondering, if we could use the invalid status of QColor as this flag ? > > See later… Should work, but I'm not sure would it be much better. Saves one boolean instance during loading a file, bu

D25715: Use RTF default color as default Qt format

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, > AbstractRtfO

D25715: Use RTF default color as default Qt format

pvuorela added reviewers: davidllewellynjones, dcaliste. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D25715 To: pvuorela, davidllewellynjones, dcaliste Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever

D25715: Use RTF default color as default Qt format

pvuorela created this revision. Herald added a project: Calligra: 3.0. Herald added a subscriber: Calligra-Devel-list. pvuorela requested review of this revision. REVISION SUMMARY RTF spec says a color entry without anything defined is a "default color", but doesn't specify more what that is.