D25741: Add natural sorting and case-insensitive sorting for all role-types

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> kfileitemmodel.cpp:1197
> +        }
> +        else if (stringRolesTrue(m_sortRole)) {
> +            auto lambdaLessThan = [&] (const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b)

No line return between `}` and `else if`

> kfileitemmodel.cpp:1681
> +{
> +    const QByteArray role = roleForType(m_sortRole);
> +    return a->values.value(role).toString() < b->values.value(role).toString();

I wonder if it woulde be better to move this code directly in the lambda code.

> kfileitemmodel.cpp:1855
> +    case OwnerRole:
> +    case GroupRole: {
> +        const QByteArray role = roleForType(m_sortRole);

Since this block is very close to the default case.

Maybe we could just have (and remove this added multicase) :

  default: {
      const QByteArray role = roleForType(m_sortRole);
      const QString roleValueA = a->values.value(role).toString();
      const QString roleValueB = b->values.value(role).toString();
      if (!roleValueA.isEmpty() && roleValueB.isEmpty()) {
          result = -1;
      } else if (roleValueA.isEmpty() && !roleValueB.isEmpty()) {
          result = +1;
      } else {
          result = stringCompare(roleValueA, roleValueB); // <- only line changed
      }
      break;
  }

REVISION DETAIL
  https://phabricator.kde.org/D25741

To: gvgeo, #dolphin, nicolasfella, meven
Cc: meven, kfm-devel, pberestov, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191218/5a6e3eb0/attachment.htm>