Most of the cases are pretty straight forward. The exception was KoFormulaTool
WIP since there are still some instance of QSignalMapper
Details
Details
- Reviewers
dfaure - Group Reviewers
Calligra: 3.0 KF6 - Commits
- R8:e29dc97256de: [WIP]: Port away from deprecated QSignalMapper
Change in KoFormulaTool were manually tested
Diff Detail
Diff Detail
- Repository
- R8 Calligra
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| libs/widgets/KoDialog.cpp | ||
|---|---|---|
| 161 | I bet this crashes at runtime. You're capturing the local variable key by reference, so it will have gone out of scope by the time the lambda calls the slot. Remove the & to make it a capture by value. | |
| plugins/formulashape/KoFormulaTool.cpp | ||
| 119–121 | qAsConst(m_templateActions) to avoid a detach | |
| 128 | same here | |
| 136–137 | completely unrelated : this those two lines could be combined into delete m_cursorList.takeAt(0);, and the comment removed. | |
| plugins/textediting/spellcheck/SpellCheckMenu.cpp | ||
| 83 | You're capturing suggestion by reference, and it's a reference itself. OK I did some googling and found that C++11 was unclear about that, C++14 clarified it to be OK. Still, I would just make a copy, so the next reader doesn't have to spend time googling for edge cases ;) | |
| plugins/textshape/TextTool.cpp | ||
| 192 | Remove & here too -- reference to local variable, used later = crash. | |
| plugins/textshape/dialogs/SimpleCitationBibliographyWidget.cpp | ||
| 80 | you know what I'm going to say ;) + add this as 3rd argument | |
| 110 | This is a bit dangerous, it also disconnects any other possible receiver. You should be able to pass this as 3rd arg instead of nullptr, if you pass this at connect time too. (this applies to all the disconnects in this change request) | |
| plugins/textshape/dialogs/SimpleTableOfContentsWidget.cpp | ||
| 78 | same Also, add this as third argument. | |
| words/plugins/scripting/Tool.h | ||
| 69–70 | Better capture a local variable called key by value. Iterators get invalidated when containers change. | |
Comment Actions
Thanks for the review David. I will now follow your advice and continue porting
away from QSignalMapper.
Comment Actions
Afaik we still support qt5.3, I don't think qAsConst is availabel.
Disregard if this has changed.
Comment Actions
Then write your own, it's easy:
// this adds const to non-const objects (like std::as_const)
template <typename T> Q_DECL_CONSTEXPR typename std::add_const<T>::type &koAsConst(T &t) noexcept { return t; }
// prevent rvalue arguments:
template <typename T> void koAsConst(const T &&) = delete;Comment Actions
Ok I will replace the qAsConst with koAsConst ;)
I suppose the reason why Calligra needs Qt5.3 is the Qt Quick 1 support?