D27057: Solid-device-automounter/kcm: Enable/Disable columns automount onLogin/onAttached depending on corresponding checkbox

[Kevin Ottens]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200131/ea739a96/attachment-0001.html>