ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> DeviceModel.cpp:185
> if (index.isValid()) {
> - if (index.parent().isValid()) {
> - if (index.column() > 0) {
> - return Qt::ItemIsSelectable | Qt::ItemIsEnabled |
> Qt::ItemIsUserCheckable;
> - } else if (index.column() == 0) {
> - return Qt::ItemIsSelectable | Qt::ItemIsEnabled;
> + if (index.parent().isValid() && index.column() > 0) {
> + if (index.column() == 1) {
This logic is becoming harder to read than necessary. I think I would go for
something structured along those lines:
if (!index.parent().isValid()) {
return Qt::ItemIsEnabled;
}
switch (index.column()) {
case 0:
...
case 1:
...
case 2:
...
default:
Q_UNREACHABLE
}
> DeviceModel.cpp:350
> +
> +void DeviceModel::setAutomaticMountOnLogin(bool automaticLogin) {
> + if (m_automaticLogin != automaticLogin) {
{ Should be on its own line
> DeviceModel.cpp:359
> +}
> +void DeviceModel::setAutomaticMountOnPlugin(bool automaticAttached) {
> + if (m_automaticAttached != automaticAttached) {
Ditto + needs an empty line before that one.
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27057
To: meven, #plasma, ervin, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
