KConfigDialogManager::changedMap is deprecated since KF 5.32. This commit mimics the replacement commit from Okteta:
https://cgit.kde.org/okteta.git/commit/?id=1ed966965b656bac22e1fe99e1b7ede00da68263
Details
Details
- Reviewers
aacid - Group Reviewers
KDE Edu - Commits
- R334:4da724769b1f: Replace deprecated KConfigDialogManager::changedMap with property in the class…
Compiles. It seems that Equation editor behavior does not change.
Diff Detail
Diff Detail
- Repository
- R334 KmPlot
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 18366 Build 18384: arc lint + arc unit
Comment Actions
Given that almost everything ships KF5 > 5.32
https://repology.org/project/kconfig/badges
Maybe we can just require
KF 5.32 in CMakeLists.txt and remove the ifdef?
| kmplot/equationeditwidget.h | ||
|---|---|---|
| 39 | Q_PROPERTY are ignored if the class doesn't have a Q_OBJECT which this one doesn't have. Once you add the Q_OBJECT marked, it stops compiling because document() doesn't return a QString. Which brings me the question of, what was that KConfigDialogManager line doing? Did we even need it? | |
Comment Actions
EquationEdit != EquationEditWidget
You rather want to add a NOTIFY textEdited to the property definition of text for class EquationEdit, at https://phabricator.kde.org/source/kmplot/browse/master/kmplot/equationedit.h$52 :)
Re: "Did we even need it?": EquationEdit is used with ui files for kcfgx-based settings UI, so making KConfigDialogManager aware of the property change signal is needed to update config editing status (e.g. to enable Apply/Ok buttons).
See e.g. https://phabricator.kde.org/source/kmplot/browse/master/kmplot/editcoords.ui
Comment Actions
Unfortunately the code for that EquationEdit seems to be a bit terrible, but that's a different story :D
Open the View -> Coordinate system dialog and see how apply is enabled (when it should not)
Now click apply and give the focus to the first edit and see how apply becomes reenabled :(
Anyhow, i guess we can commit this, since that's not a regression.