D27503: [KCM Spellchecking] port to KPropertySkeletonItem

[Kevin Ottens]

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 calling toSet() on those? Not overly efficient but would compress a bit the resulting code. Alternatively, couldn't we ensure those lists are always kept sorted?

> spellchecking.cpp:61
> +    defaultValue.sort();
> +    if (currentValue != referenceValue) {
> +        unmanagedChangeState = true;

this is:

  unmanagedChangeState |= currentValue != referenceValue;

> spellchecking.cpp:64
> +    }
> +    if (currentValue != defaultValue) {
> +        unmanagedDefaultState = false;

this is:

  unmanagedDefaultState &= currentValue == defaultValue

> spellchecking.cpp:74
> +    defaultValue.sort();
> +    if (currentValue != referenceValue) {
> +        unmanagedChangeState = currentValue != referenceValue;

see above

> spellchecking.cpp:77
> +    }
> +    if (currentValue != defaultValue) {
> +        unmanagedDefaultState = false;

see above

> spellchecking.cpp:82
> +
> +    if (m_settings->defaultLanguage() != m_configWidget->language()) {
> +        unmanagedChangeState = true;

see above

> spellchecking.cpp:85
> +    }
> +    if (m_configWidget->language() != Sonnet::Settings::defaultDefaultLanguage()) {
> +        unmanagedDefaultState = false;

see above

> ervin wrote in spellchecking.h:56
> nitpick: we usually find methods before variables

This still applies

> spellcheckingskeleton.cpp:38
> +
> +void SpellCheckingSkeleton::usrRead()
> +{

I'd expect this to update m_preferredLanguages, m_ignoreList and m_defaultLanguage otherwise we could have situations where things get out of sync. Also I'd expect to see usrSetDefaults() to be overridden as well.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27503

To: bport, #plasma, meven, crossi, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200225/ae2db579/attachment-0001.html>