D21041: [Fstab] Use folder-decrypted icon for encrypting fuse mounts

[David Hallas]

hallas added inline comments.

INLINE COMMENTS

> fstabdevice.cpp:62
>          }
> +        if ((fstype == QLatin1String("fuse.encfs")) ||
> +            (fstype == QLatin1String("fuse.cryfs"))) {

Would it make sense to wrap this in a function with a more expressive name like: isEncryptedFilesystem or isValutFilesystem? Then we could use the same function in fstabhandling.cpp:126

> fstabdevice.cpp:91
>      if (m_iconName.isEmpty()) {
> -        if (m_isNetworkShare) {
> +        if (m_storageType == StorageType::NetworkShare) {
>              m_iconName = QLatin1String("network-server");

I think a switch here (without a default case) would be nice. This would allow us to get the compiler to tell us to update this list if new storage types are added.

> fstabdevice.cpp:172
> +               (interfaceType == Solid::DeviceInterface::NetworkShare)) {
>          return new FstabNetworkShare(this);
>      }

Why do we create a new FstabNetworkShare everytime this function is called with NetworkShare but we return the same object instance when it is a StorageAccess?

REPOSITORY
  R245 Solid

BRANCH
  fstab_generic

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

To: bruns, #frameworks, ngraham, hallas
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190506/4811a8d8/attachment-0001.html>