D25335: [Details mode] Allow to fill the column size of directories with actual size

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  I think we should default to "number of items", because we can't know whether the user runs an SSD or an old HDD. Better safe than sorry.

INLINE COMMENTS

> kfileitemlistwidget.cpp:69-70
>          if (values.value("isDir").toBool()) {
>              // The item represents a directory. Show the number of sub directories
>              // instead of the file size of the directory.
>              if (!roleValue.isNull()) {

Please fix this comment (or drop it).

> kfileitemmodelrolesupdater.cpp:770-774
> -            disconnect(m_model, &KFileItemModel::itemsChanged,
> -                       this,    &KFileItemModelRolesUpdater::slotItemsChanged);
>              m_model->setData(index, data);
> -            connect(m_model, &KFileItemModel::itemsChanged,
> -                    this,    &KFileItemModelRolesUpdater::slotItemsChanged);

Why remove these?

> kdirectorycontentscounter.cpp:70
>  {
> -    --m_workersCount;
> -
> -    if (m_workersCount > 0) {
> +    if (m_workerThread->isRunning()) {
>          // The worker thread will continue running. It could even be running

Why this change? What's wrong with `m_workersCount` ?

> kdirectorycontentscounterworker.cpp:47
> +    long size = 0;
> +    auto dir = QT_OPENDIR(dirPath.toLocal8Bit());
>      if (dir) {

Why `toLocal8Bit()` ?

> kdirectorycontentscounterworker.cpp:50-52
> +        QT_STATBUF buf;
> +
> +        bool linkFound = false;

These can be declared below, where they are actually used.

> kdirectorycontentscounterworker.cpp:77
> +            if (allowedRecursiveLevel > 0) {
> +                 QString nameBuf = QString("%1/%2").arg(dirPath, dirEntry->d_name);
> +

`QStringLiteral`

> kdirectorycontentscounterworker.cpp:119
> +    const uint maxRecursiveLevel = DetailsModeSettings::recursiveDirectorySizeLimit();
> +    QByteArray dirPath = QFile::encodeName(path);
> +

Why call `QFile::encodeName()` here and not inside `walkDir()` ?

> viewsettingstab.cpp:134
> +
> +        topLayout->addRow(i18nc("@title:group", "Folder size display:"), m_numberOfItems);
> +        topLayout->addRow(QString(), contentsSizeLayout);

Typo: "Folder size displays"

REPOSITORY
  R318 Dolphin

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

To: meven, elvisangelaccio, ngraham, #dolphin
Cc: feverfew, anthonyfieroni, iasensio, kfm-devel, azyx, nikolaik, pberestov, aprcela, fprice, fbampaloukas, alexde, Codezela, meven, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200426/6273e259/attachment.htm>