D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ahmadsamir added a comment. Should be fixed by D28232 . REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27503 To: bport, #plasma, meven, crossi, ervin Cc: usta, ahmadsamir, ngraham, plasma-devel, Orage, LeGast00n,

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

usta added a comment. Not just include problem but also got a problem about new Qt version : [ 64%] Linking CXX shared module ../../bin/kcm_notifications.so /home/usta/kde/src/plasma-desktop/kcms/spellchecking/spellchecking.cpp: In member function ‘void

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ahmadsamir added a comment. This failed to build on the CI https://build.kde.org/job/Plasma/job/plasma-desktop/, I guess you'll need to bump the min required version of KF5 to 5.69.0. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27503 To: bport, #plasma,

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ngraham added a comment. Looks like this failed in the CI. [2020-03-23T17:18:47.106Z] /home/jenkins/workspace/Plasma/plasma-desktop/kf5-qt5 SUSEQt5.12/kcms/spellchecking/spellcheckingskeleton.cpp:24:10: fatal error: Sonnet/ConfigView: No such file or directory

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

This revision was automatically updated to reflect the committed changes. Closed by commit R119:38a874ab82aa: [KCM Spellchecking] port to KPropertySkeletonItem (authored by bport). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27503?vs=77267=78297

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport updated this revision to Diff 77267. bport added a comment. fix REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27503?vs=76516=77267 REVISION DETAIL https://phabricator.kde.org/D27503 AFFECTED FILES kcms/spellchecking/CMakeLists.txt

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin added inline comments. INLINE COMMENTS > bport wrote in spellchecking.cpp:88 > Yes, or I can probably also use item like before, and that will be handled > for me That would create other issues I think (not fully sure TBH). Well in any case that's an extra level of indirection we don't

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport added inline comments. INLINE COMMENTS > ervin wrote in spellchecking.cpp:88 > Sure but still, the skeleton is expected to be already be in a "loaded" state > at that point, that's why it works for the items (otherwise a findItem > wouldn't be enough, it'd have to call readConfig on them

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin added inline comments. INLINE COMMENTS > bport wrote in spellchecking.cpp:88 > no load only do findItem for managed widget on the skeleton Sure but still, the skeleton is expected to be already be in a "loaded" state at that point, that's why it works for the items (otherwise a findItem

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport added inline comments. INLINE COMMENTS > ervin wrote in spellchecking.cpp:88 > Are you sure this is necessary? I'd expect KCModule::load() to call load() on > m_skeleton no load only do findItem for managed widget on the skeleton > ervin wrote in spellcheckingskeleton.cpp:51 > I have a

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ervin wrote in spellchecking.cpp:55 > What about calling toSet() on those? Not overly efficient but would compress > a bit the resulting code. Alternatively,

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport updated this revision to Diff 76516. bport marked 19 inline comments as done. bport added a comment. cleanup REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27503?vs=76385=76516 REVISION DETAIL https://phabricator.kde.org/D27503 AFFECTED

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > spellchecking.cpp:55 > + > +auto referenceValue = m_settings->currentIgnoreList(); > +auto currentValue = m_configWidget->ignoreList(); What about

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport updated this revision to Diff 76385. bport added a comment. Take change from sonnet patch in consideration and other ervin feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27503?vs=75998=76385 REVISION DETAIL

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > spellchecking.cpp:30 > +#include > +#include > +#include Good opportunity to switch to the camel case includes? > spellchecking.cpp:40 >

D27503: [KCM Spellchecking] port to KPropertySkeletonItem

bport created this revision. bport added reviewers: Plasma, meven, crossi, ervin. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. bport requested review of this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D27503 AFFECTED