Details
- Reviewers
ervin - Group Reviewers
Plasma - Commits
- R119:fd4fd031eb6b: [KCM/Component] filemanager: make dolphin the default filemananger
Diff Detail
- Repository
- R119 Plasma Desktop
- Branch
- arcpatch-D26324
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20455 Build 20473: arc lint + arc unit
Note that I think std::any_of wrapped in a helper to get an iterator on the dolphin radio button would make that code simpler. To be checked maybe? Not mandatory at all, feel free to ignore me there.
Damn, sorry mate... I meant find_if... dunno why I spluttered any_of... :-/
Indeed if you make a tiny wrapper function for find_if on the "storageId == org.kde.dolphin.desktop" condition, you can use it both in defaults and isDefaults.
In one case it's just about calling setChecked(true) on the found radio is any, in the other case it's about returning true or isChecked() if something was found (and then the check on mDynamicRadioButtons emptiness can disappear, indeed if it's empty dolphin can't be present anyway).
| kcms/componentchooser/componentchooserfilemanager.cpp | ||
|---|---|---|
| 57 | Turn it into a free function (taking the list as parameter), move it toward the top of the file and in the anonymous namespace (this is typically before the first definition of a method in CfgFileManager). Also: space before * or & not after. | |
| 61 | This if is not necessary anymore (if it's empty dolphinRadio will be a nullptr | |
| kcms/componentchooser/componentchooserfilemanager.h | ||
| 46 ↗ | (On Diff #72489) | No need to expose it in the .h at all, could take the list as a const& parameter. Besides the space should be before the star not after. |
Almost there.
| kcms/componentchooser/componentchooserfilemanager.cpp | ||
|---|---|---|
| 42 | Please move it before the ctor an declare it in the anonymous namespace ("namespace { ... }"). Also remove the space after the * and receive the radio buttons list as a const reference not by value. Nitpick: I think I'd rename the parameter to something like "radioButtons" or "radioButtonList". | |
| kcms/componentchooser/componentchooserfilemanager.h | ||
| 46 ↗ | (On Diff #72491) | This change should go away now. |
Correct namespace declaration, better variable naming, take list of QRadioButton by const ref
Note the :: prefix is likely unnecessary in this context (doesn't hurt either).
| kcms/componentchooser/componentchooserfilemanager.h | ||
|---|---|---|
| 46 ↗ | (On Diff #72493) | Really, this change should go away. ;-) |
| kcms/componentchooser/componentchooserfilemanager.cpp | ||
|---|---|---|
| 32 | And of course as I was about to hit "accept" I notice this opening curly brace is misplaced, please put it on the next line. | |