[KCM/Component] Terminal port to KConfigXT, make isDefault work
ClosedPublic

Authored by meven on Dec 31 2019, 10:00 AM.

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20450
Build 20468: arc lint + arc unit
meven created this revision.Dec 31 2019, 10:00 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 31 2019, 10:00 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
meven requested review of this revision.Dec 31 2019, 10:00 AM
ervin accepted this revision.Dec 31 2019, 10:24 AM

One nitpick to address before pushing.

Also I mark those accepted assuming there will be another phase where the settings objects will actually be properly matched to the GUI. Currently it's all fixing the isDefaults by hand based on GUI state but not on config state, also I don't think it currently honors the mutability state of those keys. This will need being addressed in further commits.

kcms/componentchooser/componentchooserterminal.cpp
67

I'd use the opportunity to fix that opening curly brace to have it on its own line

This revision is now accepted and ready to land.Dec 31 2019, 10:24 AM
meven updated this revision to Diff 72468.Dec 31 2019, 10:36 AM

Make default button work, fix a carriage return missing

meven marked an inline comment as done.Dec 31 2019, 10:45 AM

One nitpick to address before pushing.

Also I mark those accepted assuming there will be another phase where the settings objects will actually be properly matched to the GUI. Currently it's all fixing the isDefaults by hand based on GUI state but not on config state.

Given the current GUI entanglement and the CfpPlugin split, I wonder about the opportunity given how much rewrite/refactoring will be necessary to use the KConfig state.

I don't think it currently honors the mutability state of those keys. This will need being addressed in further commits.

Indeed, it will follow.

One nitpick to address before pushing.

Also I mark those accepted assuming there will be another phase where the settings objects will actually be properly matched to the GUI. Currently it's all fixing the isDefaults by hand based on GUI state but not on config state.

Given the current GUI entanglement and the CfpPlugin split, I wonder about the opportunity given how much rewrite/refactoring will be necessary to use the KConfig state.

I think it's worth it in any case, otherwise we fix symptoms but not the root cause and issues will creep up again later on when someone touches the module and forgets one of the error prone construct (like checking for mutability and enabling/disabling corresponding widgets, removing keys which are back to defaults from the config file, etc.).

I don't think it currently honors the mutability state of those keys. This will need being addressed in further commits.

Indeed, it will follow.

ervin accepted this revision.Dec 31 2019, 11:03 AM
This revision was automatically updated to reflect the committed changes.