Replace deprecated KConfigDialogManager::changedMap with property in the class definition
ClosedPublic

Authored by yurchor on Oct 30 2019, 3:34 PM.

Details

Summary

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

Test Plan

Compiles. It seems that Equation editor behavior does not change.

Diff Detail

Repository
R334 KmPlot
Branch
arcpatch-D25078
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19206
Build 19224: arc lint + arc unit
yurchor created this revision.Oct 30 2019, 3:34 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptOct 30 2019, 3:34 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
yurchor requested review of this revision.Oct 30 2019, 3:34 PM
aacid added a subscriber: aacid.Oct 30 2019, 8:43 PM

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?

yurchor updated this revision to Diff 69085.Oct 31 2019, 7:27 AM

Rise the dependence on KF5 to avoid #ifdef

aacid added inline comments.Nov 26 2019, 11:33 PM
kmplot/equationeditwidget.h
39 ↗(On Diff #69085)

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?

kossebau added a subscriber: kossebau.EditedNov 27 2019, 12:08 AM

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

yurchor updated this revision to Diff 70403.Nov 27 2019, 9:51 AM

Add a NOTIFY textEdited to the property definition of text for class EquationEdit

aacid accepted this revision.Nov 27 2019, 9:27 PM

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.

This revision is now accepted and ready to land.Nov 27 2019, 9:27 PM
This revision was automatically updated to reflect the committed changes.