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
