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 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 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, but then feels a bit more complex. 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
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 ) :
> - 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
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. In practice seems to work nicer now when such a default color is e.g. used to clear a previously set text background color. Earlier version resulted in black background and black text. REPOSITORY R8 Calligra BRANCH rtf_default_color REVISION DETAIL https://phabricator.kde.org/D25715 AFFECTED FILES filters/words/rtf/import/3rdparty/rtf-qt/src/ColorTableDestination.cpp filters/words/rtf/import/3rdparty/rtf-qt/src/ColorTableDestination.h filters/words/rtf/import/3rdparty/rtf-qt/src/TextDocumentRtfOutput.cpp To: pvuorela Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
Re: Qt version and new website
On dinsdag 3 december 2019 15:54:53 CET Carl Schwan wrote: > Hello, > > I wanted to know if we could change the Qt dependency from Qt 5.3 to Qt 5.9 > at least. > > I think some parts already depends on Qt 5.7 at least. For example in > Calligra Gemini, there is > a dependency to Kirigami, that depends on Qt Quick Control 2 introduced in Qt > 5.7. > > I suppose there are two possible reasons, we still depend on Qt 5.3: > > + Nobody bothered to change the CMakeLists.txt I think it was because Jolla, which uses Calligra as the Sailfish documents application still is on Qt < 5.9: "We (Jolla) are looking reasonably into the summer or even 2nd half of 2019 for the Qt 5.9 roll-out. " See https://together.jolla.com/question/194079/qt-512-and-sailfish-os-3/ > + Some parts of Calligra need a version of Qt <= 5.3 > > I hope this is the first reason. > > In other news, I created a task for Season of KDE to rewrite the Calligra > website using > KDE branding and a Jekyll theme > (https://invent.kde.org/websites/jekyll-kde-theme). So that > we have a website looking like kontact.kde.org and cantor.kde.org. > > Cheers, > > Carl Schwan > https://carlschwan.eu > > -- Boudewijn Rempt | https://www.valdyas.org | https://www.krita.org signature.asc Description: This is a digitally signed message part.
Qt version and new website
Hello, I wanted to know if we could change the Qt dependency from Qt 5.3 to Qt 5.9 at least. I think some parts already depends on Qt 5.7 at least. For example in Calligra Gemini, there is a dependency to Kirigami, that depends on Qt Quick Control 2 introduced in Qt 5.7. I suppose there are two possible reasons, we still depend on Qt 5.3: + Nobody bothered to change the CMakeLists.txt + Some parts of Calligra need a version of Qt <= 5.3 I hope this is the first reason. In other news, I created a task for Season of KDE to rewrite the Calligra website using KDE branding and a Jekyll theme (https://invent.kde.org/websites/jekyll-kde-theme). So that we have a website looking like kontact.kde.org and cantor.kde.org. Cheers, Carl Schwan https://carlschwan.eu publickey - carl@carlschwan.eu - 0x7F564CB5.asc Description: application/pgp-keys signature.asc Description: OpenPGP digital signature
D25714: Port away from deprecated KHtml
ognarb added a reviewer: Calligra: 3.0. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D25714 To: ognarb, #calligra:_3.0 Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25714: Port away from deprecated KHtml
ognarb edited the summary of this revision. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D25714 To: ognarb Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25714: Port away from deprecated KHtml
ognarb added a task: T11542: Remove KHTML. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D25714 To: ognarb Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25714: Port away from deprecated KHtml
ognarb created this revision. Herald added a project: Calligra: 3.0. Herald added a subscriber: Calligra-Devel-list. ognarb requested review of this revision. TEST PLAN Compile, but I need to figure out why before and after this change the filter doesn't work. REPOSITORY R8 Calligra BRANCH master REVISION DETAIL https://phabricator.kde.org/D25714 AFFECTED FILES CMakeLists.txt filters/sheets/html/CMakeLists.txt filters/sheets/html/htmlimport.cc filters/sheets/html/htmlimport.h To: ognarb Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25664: [WIP]: Port away from deprecated QSignalMapper
This revision was automatically updated to reflect the committed changes. Closed by commit R8:e29dc97256de: [WIP]: Port away from deprecated QSignalMapper (authored by ognarb). REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25664?vs=70790=70818 REVISION DETAIL https://phabricator.kde.org/D25664 AFFECTED FILES libs/text/styles/KoParagraphStyle.h libs/widgets/KoDialog.cpp libs/widgets/KoDialog_p.h plugins/formulashape/KoFormulaTool.cpp plugins/formulashape/KoFormulaTool.h plugins/textediting/spellcheck/SpellCheckMenu.cpp plugins/textediting/spellcheck/SpellCheckMenu.h plugins/textshape/TextTool.cpp plugins/textshape/dialogs/SimpleCitationBibliographyWidget.cpp plugins/textshape/dialogs/SimpleCitationBibliographyWidget.h plugins/textshape/dialogs/SimpleParagraphWidget.cpp plugins/textshape/dialogs/SimpleParagraphWidget.h plugins/textshape/dialogs/SimpleTableOfContentsWidget.cpp plugins/textshape/dialogs/SimpleTableOfContentsWidget.h plugins/textshape/dialogs/StylesModel.cpp plugins/textshape/dialogs/StylesModel.h words/plugins/scripting/Tool.h To: ognarb, #calligra:_3.0, #kf6, dfaure Cc: danders, dfaure, Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25664: [WIP]: Port away from deprecated QSignalMapper
dfaure accepted this revision as: dfaure. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D25664 To: ognarb, #calligra:_3.0, #kf6, dfaure Cc: danders, dfaure, Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
D25664: [WIP]: Port away from deprecated QSignalMapper
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R8 Calligra BRANCH arcpatch-D25664 REVISION DETAIL https://phabricator.kde.org/D25664 To: ognarb, #calligra:_3.0, #kf6, dfaure Cc: danders, dfaure, Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, vandenoever
