Code formatting is improved. "save" function is improved by preventing unnecessary file open and file write. Functionality is same. Everything is tested, works fine.
Details
- Reviewers
ngraham romangg mart - Group Reviewers
Plasma - Commits
- R119:14e585415226: Workspace KCM Code Improvement
R119:de742972bf31: Workspace KCM Code Improvement
Diff Detail
- Repository
- R119 Plasma Desktop
- Branch
- kcmworkspace-CodeFormatting
- Lint
No Linters Available - Unit
No Unit Test Coverage
While we're doing some formatting and style cleanup work, how about renaming some variables? For example m_ostateToolTip and m_ostateVisualFeedback aren't very descriptive IMHO. Maybe instead, they could be m_currentToolTipState and m_currentVisualFeedbackState.
Also, about coding style. Why are you using /* for one line comments?
kcms/workspaceoptions/workspaceoptions.cpp | ||
---|---|---|
28–29 | ||
35–37 | I think you could do something like this in the header file class ... { ... m_stateToolTip = true; m_stateVisualFeedback = true; m_stateSingleClick = true; ... }; | |
111–112 | I would remove it. It doesn't add any useful information. I don't see any word about commets in the kdelibs coding style(I assume Plasma follows it) but as a rule of thumb: comment things that are not obvious. For example, see https://google.github.io/styleguide/cppguide.html#Implementation_Comments |
kcms/workspaceoptions/workspaceoptions.cpp | ||
---|---|---|
62 | We want to batch our syncs to plasmarc, which the old code did better. I wouldn't bother trying to be clever with checking if it's the original state or not here, as KConfig will do all of that for us anyway at a more correct lower level. |
kcms/workspaceoptions/workspaceoptions.cpp | ||
---|---|---|
62 | For the sync yes. For checking state: the old code wrote always all values to the config file, with or without the user changing them at all. It might make sense to only write config values which have been explicitly changed by the user, because this would mean that we can later on change the default of values of this user which have never been explicitly changed by the user before with an update of just the KCM code. |
kcms/workspaceoptions/workspaceoptions.cpp | ||
---|---|---|
62 | Nice point. I tested the situation for the old code. When I just change single/double click option, other option files (plasmarc) are not opened. That means KConfig handles the situation. Thank you. Correction will be done now. |
resources.qrc is creator for better Qt Creator integration and explicit definition of resource files. workspaceoptions.cpp/.h are reformatted according to phab feedback. Save and load function are seperated to different functions. It is easier to manipulate now and the code is more clear. Comments are reformatted. Tested and everything works fine.
kcms/workspaceoptions/resources.qrc | ||
---|---|---|
1 ↗ | (On Diff #34652) | I don't want this kcm done differently from all the others, i want all modules using the same structure, on disk |